hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:242
+
+enum TokenKind { Identifier, Operator, Whitespace, Other };
+
----------------
ilya-biryukov wrote:
> `TokenKind` has the same name as `tok::TokenKind`. Could we use a different
> name here to avoid possible confusions?
> E.g. `TokenGroup` or `TokenFlavor`.
renamed to `TokenFlavor`.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:258
+
+TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
+ const LangOptions &LangOpts) {
----------------
ilya-biryukov wrote:
> NIT: add `static` for consistency with the rest of the function.
I think the static is redundant here, as the function is in the anonymous
namespace, I removed the `static` on the function above.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:326
+
+ // Handle case 1 and 3. Note that the cursor could be at the token boundary,
+ // e.g. "Before^Current", we prefer identifiers to other tokens.
----------------
ilya-biryukov wrote:
> `could be at the token boundary` should be `is at the token boundary`, right?
> Whenever it's not the case we'll bail out on `BeforeTokBeginning ==
> CurrentTokBeginning`.
yes, you are right. I was thinking of the single token case `int ^foo;`, which
should be counted into the boundary case as we consider whitespace as a token
kind.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67695/new/
https://reviews.llvm.org/D67695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
