Proposal: improved completion results

src/clang_complete.cc

@@ -29,15 +29,16 @@ unsigned Flags() {
 
 unsigned GetCompletionPriority(const CXCompletionString& str,
                                CXCursorKind result_kind,
-                               const std::string& label) {
+                               const std::string& typedText) {
   unsigned priority = clang_getCompletionPriority(str);
 
   // XXX: What happens if priority overflows?
   if (result_kind == CXCursor_Destructor) {
     priority *= 100;
   }
   if (result_kind == CXCursor_ConversionFunction ||
-      (result_kind == CXCursor_CXXMethod && StartsWith(label, "operator"))) {
+      (result_kind == CXCursor_CXXMethod &&
+       StartsWith(typedText, "operator"))) {
     priority *= 100;
   }
   if (clang_getCompletionAvailability(str) != CXAvailability_Available) {
@@ -149,6 +150,103 @@ lsCompletionItemKind GetCompletionKind(CXCursorKind cursor_kind) {
   }
 }
 
+void BuildCompletionItemTexts(std::vector<lsCompletionItem>& out,
+                              CXCompletionString completion_string,
+                              bool include_snippets) {
+  assert(!out.empty());
+  auto out_first = out.size() - 1;
+
+  std::string result_type;
+
+  int num_chunks = clang_getNumCompletionChunks(completion_string);
+  for (int i = 0; i < num_chunks; ++i) {
+    CXCompletionChunkKind kind =
+      clang_getCompletionChunkKind(completion_string, i);
+
+    std::string text;
+    switch (kind) {
+    case CXCompletionChunk_LeftParen:       text = '(';  break;
+    case CXCompletionChunk_RightParen:      text = ')';  break;
+    case CXCompletionChunk_LeftBracket:     text = '[';  break;
+    case CXCompletionChunk_RightBracket:    text = ']';  break;
+    case CXCompletionChunk_LeftBrace:       text = '{';  break;
+    case CXCompletionChunk_RightBrace:      text = '}';  break;
+    case CXCompletionChunk_LeftAngle:       text = '<';  break;
+    case CXCompletionChunk_RightAngle:      text = '>';  break;
+    case CXCompletionChunk_Comma:           text = ", "; break;
+    case CXCompletionChunk_Colon:           text = ':';  break;
+    case CXCompletionChunk_SemiColon:       text = ';';  break;
+    case CXCompletionChunk_Equal:           text = '=';  break;
+    case CXCompletionChunk_HorizontalSpace: text = ' ';  break;
+    case CXCompletionChunk_VerticalSpace:   text = ' ';  break;
+
+    case CXCompletionChunk_ResultType:
+      result_type =
+          ToString(clang_getCompletionChunkText(completion_string, i));
+      continue;
+
+    case CXCompletionChunk_TypedText:
+    case CXCompletionChunk_Placeholder:
+    case CXCompletionChunk_Text:
+    case CXCompletionChunk_Informative:
+      text = ToString(clang_getCompletionChunkText(completion_string, i));
+
+      for (auto i = out_first; i < out.size(); ++i) {
+        // first typed text is used for filtering
+        if (kind == CXCompletionChunk_TypedText && out[i].filterText.empty())
+          out[i].filterText = text;
+
+        if (kind == CXCompletionChunk_Placeholder)
+          out[i].parameters_.push_back(text);
+      }
+      break;
+
+    case CXCompletionChunk_CurrentParameter:
+      // We have our own parsing logic for active parameter. This doesn't seem
+      // to be very reliable.
+      continue;
+
+    case CXCompletionChunk_Optional: {
+      CXCompletionString nested =
+          clang_getCompletionChunkCompletionString(completion_string, i);
+      // duplicate last element, the recursive call will complete it
+      out.push_back(out.back());
+      BuildCompletionItemTexts(out, nested, include_snippets);
+      continue;
+    }
+    }
+
+    for (auto i = out_first; i < out.size(); ++i)
+      out[i].label += text;
+
+    if (kind == CXCompletionChunk_Informative)
+      continue;
+
+    for (auto i = out_first; i < out.size(); ++i) {
+      if (!include_snippets && !out[i].parameters_.empty())
+        continue;
+
+      if (kind == CXCompletionChunk_Placeholder) {
+        out[i].insertText +=
+            "${" + std::to_string(out[i].parameters_.size()) + ":" + text + "}";
+        out[i].insertTextFormat = lsInsertTextFormat::Snippet;
+      } else {
+        out[i].insertText += text;
+      }
+    }
+  }
+
+  if (result_type.empty())
+    return;
+
+  for (auto i = out_first; i < out.size(); ++i) {
+    // ' : ' for variables,
+    // ' -> ' (trailing return type-like) for functions
+    out[i].label += (out[i].label == out[i].filterText ? " : " : " -> ");
+    out[i].label += result_type;
+  }
+}
+
 // |do_insert|: if |!do_insert|, do not append strings to |insert| after
 // a placeholder.
 void BuildDetailString(CXCompletionString completion_string,
@@ -387,6 +485,8 @@ void CompletionQueryMain(ClangCompleteManager* completion_manager) {
       {
         if (request->on_complete) {
           std::vector<lsCompletionItem> ls_result;
+          // this is a guess but can be larger in case of optional parameters,
+          // as they may be expanded into multiple items
           ls_result.reserve(cx_results->NumResults);
 
           timer.Reset();
@@ -407,29 +507,52 @@ void CompletionQueryMain(ClangCompleteManager* completion_manager) {
             // TODO: fill in more data
             lsCompletionItem ls_completion_item;
 
-            bool do_insert = true;
-            // kind/label/detail/docs/sortText
             ls_completion_item.kind = GetCompletionKind(result.CursorKind);
-            BuildDetailString(
-                result.CompletionString, ls_completion_item.label,
-                ls_completion_item.detail, ls_completion_item.insertText,
-                do_insert, ls_completion_item.insertTextFormat,
-                &ls_completion_item.parameters_,
-                completion_manager->config_->client.snippetSupport);
-            if (completion_manager->config_->client.snippetSupport &&
-                ls_completion_item.insertTextFormat ==
-                    lsInsertTextFormat::Snippet) {
-              ls_completion_item.insertText += "$0";
-            }
-
             ls_completion_item.documentation = ToString(
                 clang_getCompletionBriefComment(result.CompletionString));
 
-            ls_completion_item.priority_ = GetCompletionPriority(
-                result.CompletionString, result.CursorKind,
-                ls_completion_item.label);
-
-            ls_result.push_back(ls_completion_item);
+            // label/detail/filterText/insertText/priority
+            if (completion_manager->config_->completion.detailedLabel) {
+              ls_completion_item.detail = ToString(
+                  clang_getCompletionParent(result.CompletionString, nullptr));
+
+              auto first_idx = ls_result.size();
+              ls_result.push_back(ls_completion_item);
+
+              // label/filterText/insertText
+              BuildCompletionItemTexts(
+                  ls_result, result.CompletionString,
+                  completion_manager->config_->client.snippetSupport);
+
+              for (auto i = first_idx; i < ls_result.size(); ++i) {
+                if (completion_manager->config_->client.snippetSupport &&
+                    ls_result[i].insertTextFormat ==
+                        lsInsertTextFormat::Snippet) {
+                  ls_result[i].insertText += "$0";
+                }
+
+                ls_result[i].priority_ = GetCompletionPriority(
+                    result.CompletionString, result.CursorKind,
+                    ls_result[i].filterText);
+              }
+            } else {
+              bool do_insert = true;
+              BuildDetailString(
+                  result.CompletionString, ls_completion_item.label,
+                  ls_completion_item.detail, ls_completion_item.insertText,
+                  do_insert, ls_completion_item.insertTextFormat,
+                  &ls_completion_item.parameters_,
+                  completion_manager->config_->client.snippetSupport);
+              if (completion_manager->config_->client.snippetSupport &&
+                  ls_completion_item.insertTextFormat ==
+                      lsInsertTextFormat::Snippet) {
+                ls_completion_item.insertText += "$0";
+              }
+              ls_completion_item.priority_ = GetCompletionPriority(
+                  result.CompletionString, result.CursorKind,
+                  ls_completion_item.label);
+              ls_result.push_back(ls_completion_item);
+            }
           }
 
           timer.ResetAndPrint("[complete] Building " +

src/config.h

@@ -135,6 +135,25 @@ struct Config {
     // false for the option. This option is the most useful for LSP clients
     // that implement their own filtering and sorting logic.
     bool filterAndSort = true;
+
+    // Some completion UI, such as Emacs' completion-at-point and company-lsp,
+    // display completion item label and detail side by side.
+    // This does not look right, when you see things like:
+    //     "foo" "int foo()"
+    //     "bar" "void bar(int i = 0)"
+    // When this option is enabled, the completion item label is very detailed,
+    // it shows the full signature of the candidate.
+    // The detail just contains the completion item parent context.
+    // Also, in this mode, functions with default arguments,
+    // generates one more item per default argument
+    // so that the right function call can be selected.
+    // That is, you get something like:
+    //     "int foo()" "Foo"
+    //     "void bar()" "Foo"
+    //     "void bar(int i = 0)" "Foo"
+    // Be wary, this is quickly quite verbose,
+    // items can end up truncated by the UIs.
+    bool detailedLabel = false;
   };
   Completion completion;
 
@@ -162,7 +181,7 @@ struct Config {
   std::vector<std::string> dumpAST;
 };
 MAKE_REFLECT_STRUCT(Config::ClientCapability, snippetSupport);
-MAKE_REFLECT_STRUCT(Config::Completion, filterAndSort);
+MAKE_REFLECT_STRUCT(Config::Completion, filterAndSort, detailedLabel);
 MAKE_REFLECT_STRUCT(Config::Index, comments, attributeMakeCallsToCtor);
 MAKE_REFLECT_STRUCT(Config,
                     compilationDatabaseDirectory,

src/language_server_api.h

@@ -365,7 +365,7 @@ struct lsCompletionItem {
 
   // A string that should be used when filtering a set of
   // completion items. When `falsy` the label is used.
-  // std::string filterText;
+  std::string filterText;
 
   // A string that should be inserted a document when selecting
   // this completion. When `falsy` the label is used.
@@ -407,6 +407,7 @@ MAKE_REFLECT_STRUCT(lsCompletionItem,
                     documentation,
                     sortText,
                     insertText,
+                    filterText,
                     insertTextFormat,
                     textEdit);
 

src/messages/text_document_completion.cc

@@ -68,17 +68,16 @@ struct Out_TextDocumentComplete
 };
 MAKE_REFLECT_STRUCT(Out_TextDocumentComplete, jsonrpc, id, result);
 
-bool CompareLsCompletionItem(const lsCompletionItem& item1,
-                             const lsCompletionItem& item2) {
-  if (item1.found_ != item2.found_)
-    return item1.found_ > item2.found_;
-  if (item1.skip_ != item2.skip_)
-    return item1.skip_ < item2.skip_;
-  if (item1.priority_ != item2.priority_)
-    return item1.priority_ < item2.priority_;
-  if (item1.label.length() != item2.label.length())
-    return item1.label.length() < item2.label.length();
-  return item1.label < item2.label;
+bool CompareLsCompletionItem(const lsCompletionItem& lhs,
+                             const lsCompletionItem& rhs) {
+  const auto lhsFilterTextLength = lhs.filterText.length();
+  const auto lhsLabelLength = lhs.label.length();
+  const auto rhsFilterTextLength = rhs.filterText.length();
+  const auto rhsLabelLength = rhs.label.length();
+  return std::tie(lhs.found_, lhs.skip_, lhs.priority_, lhsFilterTextLength,
+                  lhs.filterText, lhsLabelLength, lhs.label) <
+         std::tie(rhs.found_, rhs.skip_, rhs.priority_, rhsFilterTextLength,
+                  rhs.filterText, rhsLabelLength, lhs.label);
 }
 
 template <typename T>
@@ -129,11 +128,18 @@ void FilterAndSortCompletionResponse(
     return;
   }
 
+  // make sure all items have |filterText| set, code that follow needs it
+  for (auto& item : items) {
+    if (item.filterText.empty()) {
+      item.filterText = item.label;
+    }
+  }
+
   // If the text doesn't start with underscore,
   // remove all candidates that start with underscore.
   if (!complete_text.empty() && complete_text[0] != '_') {
     auto filter = [](const lsCompletionItem& item) {
-      return item.label[0] == '_';
+      return item.filterText[0] == '_';
     };
     items.erase(std::remove_if(items.begin(), items.end(), filter),
                 items.end());
@@ -142,7 +148,7 @@ void FilterAndSortCompletionResponse(
   // Fuzzy match.
   for (auto& item : items)
     std::tie(item.found_, item.skip_) =
-        SubsequenceCountSkip(complete_text, item.label);
+        SubsequenceCountSkip(complete_text, item.filterText);
 
   // Order all items and set |sortText|.
   std::sort(items.begin(), items.end(), CompareLsCompletionItem);