[PATCH] D65263: [clangd] a prototype for triggering rename after extracting.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 25 05:19:18 PDT 2019


hokein marked 4 inline comments as done.
hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/Protocol.h:777
+  // each command has a certain llvm::Optional structure for its arguments.
+  llvm::Optional<Position> renamePosition; // for LSP_RENAME
+};
----------------
sammccall wrote:
> couldn't we send a range here?
> That would allow the client to skip prepareRename and still use its native UI.
> (In principle. no need to actually implement this in vscode)
`rename` just uses the `position` as its parameter, not `range`.


================
Comment at: clang-tools-extra/clangd/Protocol.h:807
+
+  /// A command that will be executed in the LSP client.
+  llvm::Optional<ClientCommand> clientCommand;
----------------
sammccall wrote:
> (mark extension here too).
ah, this is not used, remove.


================
Comment at: clang-tools-extra/clangd/Protocol.h:894
+  /// (clangd extension).
+  llvm::Optional<ClientCommand> command;
 };
----------------
sammccall wrote:
> So there's a few places that the command could be specified:
> 1. in the code action, as a thing to do at the end (above)
> 2. as a parameter, when the client executes a command and the server calls applyWorkspaceEdit (here)
> 3. as the return value from executeCommand
> 
> This patch implements 1 & 2. Why this choice? (easiest to prototype is a perfectly sensible answer)
> 
> 1 is restricted to code actions.
> 2 & 3 are (practically) restricted to executeCommand.
> 2 is restricted to when edits are applied.
> 
> (To me, implementing just 1 or just 3 seem like the strongest options...)
sorry, this patch actually implements just 2 (the `clientCommand` was added accidentally in the CodeAction, removing now).

for 1 -- it has a limitation, at the time when clangd sends `CodeAction`, we only run the tweak::prepare, at this time we might not get enough information (AST etc) to compute a client command.

for 3 -- LSP doesn't say much about how to use return value from `executeCommand`, it just suggests servers to send `applyEdit` request to the server. I believe most of LSP clients (at least VSCode) just throw the return value away. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263





More information about the cfe-commits mailing list