[PATCH] D73610: [clangd] Replace raw lexer code with token buffer in prepare rename.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 29 03:40:23 PST 2020


sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:322
+    if (!TouchingIdentifier)
+      return CB(llvm::None); // no rename on non-identifiers.
+
----------------
hokein wrote:
> sammccall wrote:
> > Agree that this behavior is OK per the spec, but do we actually prefer it/why the change?
> > 
> > Seems like clients are marginally more likely to handle null incorrectly/silently, and we give up control over the message.
> > Agree that this behavior is OK per the spec, but do we actually prefer it/why the change?
> 
> I checked with VSCode, it seems fine (`the element can't be renamed`), and YCM (but it doesn't support prepare rename yet). Are you suggesting that we return our message here?
> 
Doh, I had this confused with rename where the spec is more ambiguous (and e.g. eglot doesn't handle null well). Nevermind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73610





More information about the cfe-commits mailing list