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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This is really cool! Hm, formatting is troublesome, well spotted :-)

As you're defining a generic mechanism, it'd be useful to have more examples to verify that this actually generalizes.
We might have to make them up (e.g. replace-auto-with-type-and-go-to-definition?)

If we can't show it generalizes, then I think this would still be useful enough just to dispense with generality and add a "renameAt" extension to CodeAction or something like that.



================
Comment at: clang-tools-extra/clangd/Protocol.cpp:544
 
+const llvm::StringLiteral ClientCommand::LSP_RENAME = "lsp.rename";
+
----------------
(why would "lsp" be in these names - surely this is just rename?)


================
Comment at: clang-tools-extra/clangd/Protocol.h:765
 
+// Similar to Command, but this command is executed in the LSP client (clangd
+// extension).
----------------
(If there's precedent for others using a similar protocol, we should link to it)


================
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
+};
----------------
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)


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


================
Comment at: clang-tools-extra/clangd/Protocol.h:894
+  /// (clangd extension).
+  llvm::Optional<ClientCommand> command;
 };
----------------
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...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65263