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