Fix a bunch of stuffs in completion.

src/include_complete.cc

@@ -78,14 +78,15 @@ bool TrimPath(Project* project,
 
 lsCompletionItem BuildCompletionItem(Config* config,
                                      const std::string& path,
-                                     bool /*use_angle_brackets*/,
+                                     bool use_angle_brackets,
                                      bool is_stl) {
   lsCompletionItem item;
   item.label = ElideLongPath(config, path);
-  item.detail = path;
+  item.detail = path; // the include path, used in de-duplicating
   item.textEdit = lsTextEdit();
   item.textEdit->newText = path;
   item.insertTextFormat = lsInsertTextFormat::PlainText;
+  item.use_angle_brackets_ = use_angle_brackets;
   if (is_stl) {
     item.kind = lsCompletionItemKind::Module;
     item.priority_ = 2;
@@ -107,6 +108,7 @@ void IncludeComplete::Rescan() {
 
   completion_items.clear();
   absolute_path_to_completion_item.clear();
+  inserted_paths.clear();
 
   if (!match_ && (!config_->includeCompletionWhitelist.empty() ||
                   !config_->includeCompletionBlacklist.empty()))
@@ -130,6 +132,23 @@ void IncludeComplete::Rescan() {
   });
 }
 
+void IncludeComplete::InsertCompletionItem(const std::string& absolute_path,
+                                           lsCompletionItem&& item) {
+  if (inserted_paths.insert({item.detail, inserted_paths.size()}).second) {
+    completion_items.push_back(item);
+    // insert if not found or with shorter include path
+    auto it = absolute_path_to_completion_item.find(absolute_path);
+    if (it == absolute_path_to_completion_item.end() ||
+        completion_items[it->second].detail.length() > item.detail.length())
+      absolute_path_to_completion_item[absolute_path] = completion_items.size();
+  } else {
+    lsCompletionItem& inserted_item = completion_items[inserted_paths[item.detail]];
+    // Update |use_angle_brackets_|, prefer quotes.
+    if (!item.use_angle_brackets_)
+      inserted_item.use_angle_brackets_ = false;
+  }
+}
+
 void IncludeComplete::AddFile(const std::string& absolute_path) {
   if (!EndsWithAny(absolute_path,
                    config_->includeCompletionWhitelistLiteralEnding))
@@ -143,18 +162,12 @@ void IncludeComplete::AddFile(const std::string& absolute_path) {
   lsCompletionItem item = BuildCompletionItem(
       config_, trimmed_path, use_angle_brackets, false /*is_stl*/);
 
-  if (is_scanning) {
+  std::unique_lock<std::mutex> lock(completion_items_mutex, std::defer_lock);
-    std::lock_guard<std::mutex> lock(completion_items_mutex);
+  if (is_scanning)
-    if (absolute_path_to_completion_item
+    lock.lock();
-            .insert(std::make_pair(absolute_path, completion_items.size()))
+  InsertCompletionItem(absolute_path, std::move(item));
-            .second)
+  if (lock)
-      completion_items.push_back(item);
+    lock.unlock();
-  } else {
-    if (absolute_path_to_completion_item
-            .insert(std::make_pair(absolute_path, completion_items.size()))
-            .second)
-      completion_items.push_back(item);
-  }
 }
 
 void IncludeComplete::InsertIncludesFromDirectory(std::string directory,
@@ -184,13 +197,9 @@ void IncludeComplete::InsertIncludesFromDirectory(std::string directory,
       });
 
   std::lock_guard<std::mutex> lock(completion_items_mutex);
-  for (const CompletionCandidate& result : results) {
+  for (CompletionCandidate& result : results)
-    if (absolute_path_to_completion_item
+    InsertCompletionItem(result.absolute_path,
-            .insert(
+                         std::move(result.completion_item));
-                std::make_pair(result.absolute_path, completion_items.size()))
-            .second)
-      completion_items.push_back(result.completion_item);
-  }
 }
 
 void IncludeComplete::InsertStlIncludes() {

src/include_complete.h

@@ -27,18 +27,23 @@ struct IncludeComplete {
   optional<lsCompletionItem> FindCompletionItemForAbsolutePath(
       const std::string& absolute_path);
 
+  // Insert item to |completion_items|.
+  // Update |absolute_path_to_completion_item| and |inserted_paths|.
+  void InsertCompletionItem(const std::string& absolute_path,
+                            lsCompletionItem&& item);
+
   // Guards |completion_items| when |is_scanning| is true.
   std::mutex completion_items_mutex;
   std::atomic<bool> is_scanning;
   std::vector<lsCompletionItem> completion_items;
 
-  // Absolute file path to the completion item in |completion_items|. Also
+  // Absolute file path to the completion item in |completion_items|.
-  // verifies that we only have one completion item per absolute path.
+  // Keep the one with shortest include path.
-  // We cannot just scan |completion_items| for this information because the
-  // same path can often be epxressed in mutliple ways; a trivial example is
-  // angle vs quote include style (ie, <foo> vs "foo").
   std::unordered_map<std::string, int> absolute_path_to_completion_item;
 
+  // Only one completion item per include path.
+  std::unordered_map<std::string, int> inserted_paths;
+
   // Cached references
   Config* config_;
   Project* project_;

src/language_server_api.h

@@ -351,13 +351,16 @@ struct lsCompletionItem {
   std::string detail;
 
   // A human-readable string that represents a doc-comment.
-  std::string documentation;
+  optional<std::string> documentation;
 
   // Internal information to order candidates.
   bool found_;
   std::string::size_type skip_;
   unsigned priority_;
 
+  // Use <> or "" by default as include path.
+  bool use_angle_brackets_ = false;
+
   // A string that shoud be used when comparing this item
   // with other items. When `falsy` the label is used.
   std::string sortText;

src/lex_utils.cc

@@ -64,22 +64,25 @@ ParseIncludeLineResult ParseIncludeLine(const std::string& line) {
 
 void DecorateIncludePaths(const std::smatch& match,
                           std::vector<lsCompletionItem>* items) {
-  char quote0, quote1;
+  std::string spaces_after_include = " ";
-  if (match[5].compare("\"") == 0)
+  if (match[3].compare("include") == 0 && match[5].length())
-    quote0 = quote1 = '"';
+    spaces_after_include = match[4].str();
-  else
-    quote0 = '<', quote1 = '>';
 
-  std::string spaces_between_include_and_quote =
+  std::string prefix =
-      match[3].compare("include") == 0 ? match[4].str() : " ";
+      match[1].str() + '#' + match[2].str() + "include" + spaces_after_include;
-
+  std::string suffix = match[7].str();
-  std::string prefix = match[1].str() + '#' + match[2].str() + "include" +
-                       spaces_between_include_and_quote + quote0;
-  std::string suffix = std::string(1, quote1) + match[7].str();
 
   for (lsCompletionItem& item : *items) {
-    item.textEdit->newText = prefix + item.textEdit->newText + suffix;
+    char quote0, quote1;
-    item.label = prefix + item.label + suffix;
+    if (match[5].compare("<") == 0 ||
+        (match[5].length() == 0 && item.use_angle_brackets_))
+      quote0 = '<', quote1 = '>';
+    else
+      quote0 = quote1 = '"';
+
+    item.textEdit->newText =
+        prefix + quote0 + item.textEdit->newText + quote1 + suffix;
+    item.label = prefix + quote0 + item.label + quote1 + suffix;
     item.filterText = nullopt;
   }
 }