[PATCH] D63126: [clangd] Implement "prepareRename"

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 23 06:56:30 PDT 2019


hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:393
         }}}};
+  if (Params.capabilities.RenamePrepareSupport)
+    Result["renameProvider"] = llvm::json::Object{{"prepareProvider", true}};
----------------
sammccall wrote:
> why set this only if the client advertised support?
The LSP says
```     
 /**
	 * The server provides rename support. RenameOptions may only be
	 * specified if the client states that it supports
	 * `prepareSupport` in its initial `initialize` request.
	 */
	renameProvider?: boolean | RenameOptions;
```

so we only send `RenameOptions` when the client declares it supports prepareRename.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:298
+    if (!Changes) {
+      llvm::consumeError(Changes.takeError());
+      return CB(llvm::None);
----------------
sammccall wrote:
> I thought the point of supporting this was to get errors to the user sooner. That can't happen if we throw away the error message...
use our own error message instead, luckily, VSCode prompt this error message to users.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63126





More information about the cfe-commits mailing list