[PATCH] D53213: [clangd] Send CodeAction responses to textDocument/codeAction (LSP 3.8)

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/Protocol.h:390
+  /// Flattened from codeAction.codeActionLiteralSupport.
+  // FIXME: flatten other properties in this way.
+  bool codeActionLiteralSupport = false;
----------------
kadircet wrote:
> sammccall wrote:
> > kadircet wrote:
> > > What is the reason behind this one? Is it because clients must handle unknown items on their own and fallback to a default one?
> > > 
> > > Since that default is client specific, behavior might change from client to client. I agree that clients should be up-to-date with the specs and handle all kinds of items but these might still create confusions during the transition period.
> > > 
> > > For example, ycmd decided to fallback to None instead of Text when they don't know about a symbolkind of a completion item, so users will get to see "File" for the include insertions on both folders and files but when they update to a newer clangd, they will start to see "File" for files and "None" for directory elements. Which I believe might create confusion, but we could still fallback to File for those elements(if we handled them within clangd) and user experience would neither worsen or improve.
> > > 
> > > (Currently ycmd's symbolkindcapabilities are actually up-to-date with specs, so this issue wouldn't happen. Just wanted to make my point clearer). 
> > Sorry, I don't really understand the question.
> > 
> > Are you talking about the default for `codeActionLiteralSupport`? The protocol says servers must send `Command`s unless the client indicates support for `CodeAction`s. There's no room for a different default here.
> > 
> > Or flattening of other properties? That will have no effect on logic, it just simplifies the code (see D53266).
> > 
> Ok, I thought we were also going to throw away the "valueset"s and just keep whether the client has the capability(and therefore graceful handling) or not.
Ah, no. That might actually be a better complexity tradeoff, but I'm not proposing changing the existing behavior at the moment.

On the other hand, for CodeActions themselves, this patch does rely on the client handling arbitrary `CodeActionKind`s, it doesn't look at exactly which ones the client understands.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53213