kadircet added a comment.

> Note that we use this information to *animate* the matching tokens, i.e. when 
> the cursor is on one of them, both it and its counterpart get a special 
> highlighting. That's why it's so important that the language server 
> guarantees they always come in pairs.

Oh I see the argument here (somehow missed comment, probably because i had a 
stale window lying around), this totally makes sense. but I am still feeling 
uneasy due to implementation complexity (it's embarrassing but dealing with 
`>>` is quite problematic, still, with little value :/) + the UX we'll have in 
other editors by default.
Especially as this comes as two different `HighlightingKind`s and they're 
likely to get colored differently, and having your matching brackets in 
different colors is quite annoying.

I feel like editors can still have a quite good behaviour with existing 
operator highlights, as one can build an extra layer on top to provide 
"matching bracket" support, and whenever the data is broken (e.g. non-matching 
brackets) just keep using the results from last "successful" run.



================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:408
+      Position End = Begin;
+      ++End.character;
+      addToken(*LRange, HighlightingKind::AngleBracketOpen);
----------------
there can be weird cases like (same goes for -1 below):
```foo>\
>```

can you add tests to make sure we're handling these properly (or not producing 
braces for them if it complicates the logic too much, i don't think there'll be 
many cases where they pop-up).


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:930
+          $Class[[B]] $LocalVariable_def[[b]];
+          int $LocalVariable_def[[i]] = 
static_cast$AngleBracketOpen[[<]]int$AngleBracketClose[[>]](3.5);
+          void *$LocalVariable_def[[p]] = 
reinterpret_cast$AngleBracketOpen[[<]]void *$AngleBracketClose[[>]](0);
----------------
could you also add tests for `>>` and `>>>` to make sure they're handled 
correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to