[PATCH] D39057: [clangd][WIP] Integrate the refactoring actions into clangd

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 23 09:24:57 PDT 2017


sammccall added a comment.

As noted on the other patch, I'm not sure EditorCommands is a useful abstraction.

This shows up one of the problems: clangd is mostly decoupled from the actual behavior/semantics of the commands by the EditorCommands abstraction. However LSP doesn't say what the structure of ExecuteCommandParams.arguments is - it depends on the individual command. But clangd needs to parse those arguments out of the JSON! So we're left with all the EditorCommands needing to take the same set of arguments, which then get hardcoded in clangd.

Coupling clangd more closely to the refactorings seems simpler and more flexible - what would we be losing?

Other than the EditorCommands question, implementation looks fine!



================
Comment at: clangd/Protocol.h:535
+struct CommandArgument {
+  union {
+    llvm::AlignedCharArrayUnion<Range> SelectionRange;
----------------
Why a union-of-unions instead of AlignedCharArrayUnion<Range, TextDocumentIdentifier>?


Repository:
  rL LLVM

https://reviews.llvm.org/D39057





More information about the cfe-commits mailing list