[PATCH] D65486: [clangd] Ignore semicolons, whitespace, and comments in SelectionTree.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:50
+      S.StartOffset = SM.getFileOffset(Tok.location());
+      S.EndOffset = S.StartOffset + Tok.length();
+      if (S.StartOffset >= SelBegin && S.EndOffset <= SelEnd)
----------------
SureYeaah wrote:
> Would this work correctly for nested templates? Or do we need to use the specialized token length function that we use for toHalfOpenFileRange?
This uses the token list as an intermediary for matching selected chars with AST nodes. TokenBuffer will indeed by default lex `>>` as a right shift. So we're buggy here, but I think it *mostly* doesn't matter.

If it's a double template:
 - the innermost template will claim it first, if the template range touches the selection. Problem: if only the first > is selected, the inner template will only be partially selected.
 - the outermost template will not get to claim it at all (if the inner template range touches the selection). Problem: if this is the *only* part of the outer template that's selected, it will be marked unselected. (This should be rare)
 - If the inner template doesn't touch the selection, then the outer template will be selected but only partially, which is actually correct.

Examples:
```
a<b<c>>
     ~  b=partial (correct)
  ~~~~  b=partial (incorrect: b=complete) <-- this is the worst case
~~~~~~~ a=complete,b=complete (correct)
      ~ a=partial (correct)
     ~~ b=partial (incorrect: a=partial,b=partial)
```

I'll send a followup to fix this case tomorrow (I think we can just always split the token in half) but I don't think it's critical.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65486/new/

https://reviews.llvm.org/D65486