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