[clangd] Use -completion-style=bundled by default if signature help is available

sam-mccall · sam-mccall · commit 5f092e31ab85 · 2019-07-08T17:27:15.000Z
Summary: I didn't manage to find something nicer than optional<bool>, but at least I found a sneakier comment. Reviewers: kadircet Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D64216 llvm-svn: 365356
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -366,6 +366,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
   CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
+  if (!CCOpts.BundleOverloads.hasValue())
+    CCOpts.BundleOverloads = Params.capabilities.HasSignatureHelp;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -169,7 +169,7 @@ struct CompletionCandidate {
   // Returns a token identifying the overload set this is part of.
   // 0 indicates it's not part of any overload set.
   size_t overloadSet(const CodeCompleteOptions &Opts) const {
-    if (!Opts.BundleOverloads)
+    if (!Opts.BundleOverloads.getValueOr(false))
       return 0;
     llvm::SmallString<256> Scratch;
     if (IndexResult) {
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
@@ -62,7 +62,10 @@ struct CodeCompleteOptions {
   bool IncludeIneligibleResults = false;
 
   /// Combine overloads into a single completion item where possible.
-  bool BundleOverloads = false;
+  /// If none, the the implementation may choose an appropriate behavior.
+  /// (In practice, ClangdLSPServer enables bundling if the client claims
+  /// to supports signature help).
+  llvm::Optional<bool> BundleOverloads;
 
   /// Limit the number of results returned (0 means no limit).
   /// If more results are available, we set CompletionList.isIncomplete.
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
@@ -323,6 +323,7 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) {
       }
     }
     if (auto *Help = TextDocument->getObject("signatureHelp")) {
+      R.HasSignatureHelp = true;
       if (auto *Info = Help->getObject("signatureInformation")) {
         if (auto *Parameter = Info->getObject("parameterInformation")) {
           if (auto OffsetSupport = Parameter->getBoolean("labelOffsetSupport"))
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
@@ -393,9 +393,15 @@ struct ClientCapabilities {
   bool CompletionFixes = false;
 
   /// Client supports hierarchical document symbols.
+  /// textDocument.documentSymbol.hierarchicalDocumentSymbolSupport
   bool HierarchicalDocumentSymbol = false;
 
+  /// Client supports signature help.
+  /// textDocument.signatureHelp
+  bool HasSignatureHelp = false;
+
   /// Client supports processing label offsets instead of a simple label string.
+  /// textDocument.signatureHelp.signatureInformation.parameterInformation.labelOffsetSupport
   bool OffsetsInSignatureHelp = false;
 
   /// The supported set of CompletionItemKinds for textDocument/completion.
@@ -407,12 +413,14 @@ struct ClientCapabilities {
   bool CodeActionStructure = false;
 
   /// Client supports semantic highlighting.
+  /// textDocument.semanticHighlightingCapabilities.semanticHighlighting
   bool SemanticHighlighting = false;
 
   /// Supported encodings for LSP character offsets. (clangd extension).
   llvm::Optional<std::vector<OffsetEncoding>> offsetEncoding;
 
   /// The content format that should be used for Hover requests.
+  /// textDocument.hover.contentEncoding
   MarkupKind HoverContentFormat = MarkupKind::PlainText;
 };
 bool fromJSON(const llvm::json::Value &, ClientCapabilities &);
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -58,8 +58,7 @@ static llvm::cl::opt<CompletionStyleFlag> CompletionStyle(
                    "completion, with full type information"),
         clEnumValN(Bundled, "bundled",
                    "Similar completion items (e.g. function overloads) are "
-                   "combined. Type information shown where possible")),
-    llvm::cl::init(Detailed));
+                   "combined. Type information shown where possible")));
 
 // FIXME: Flags are the wrong mechanism for user preferences.
 // We should probably read a dotfile or similar.
@@ -487,7 +486,8 @@ int main(int argc, char *argv[]) {
   clangd::CodeCompleteOptions CCOpts;
   CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
   CCOpts.Limit = LimitResults;
-  CCOpts.BundleOverloads = CompletionStyle != Detailed;
+  if (CompletionStyle.getNumOccurrences())
+    CCOpts.BundleOverloads = CompletionStyle != Detailed;
   CCOpts.ShowOrigins = ShowOrigins;
   CCOpts.InsertIncludes = HeaderInsertion;
   if (!HeaderInsertionDecorators) {

Original file line number	Diff line number	Diff line change
`@@ -323,6 +323,7 @@ bool fromJSON(const llvm::json::Value &Params, ClientCapabilities &R) {`
`323`	`323`	`}`
`324`	`324`	`}`
`325`	`325`	`if (auto *Help = TextDocument->getObject("signatureHelp")) {`
	`326`	`+ R.HasSignatureHelp = true;`
`326`	`327`	`if (auto *Info = Help->getObject("signatureInformation")) {`
`327`	`328`	`if (auto *Parameter = Info->getObject("parameterInformation")) {`
`328`	`329`	`if (auto OffsetSupport = Parameter->getBoolean("labelOffsetSupport"))`