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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
