hokein added a comment.

Thanks for the comments.



================
Comment at: clangd/ClangdLSPServer.h:40
+                  bool BuildDynamicSymbolIndex,
+                  std::unique_ptr<SymbolIndex> GlobalIdx);
 
----------------
ioeric wrote:
> We are calling this global index and static index in the patch. I think we 
> should be consistent with the naming. Generally, I think this is static 
> index, which might be global index or, say, a set of symbols for test 
> purpose, so I am inclined to static index.
+1 static index -- I forgot to rename the global index.


================
Comment at: clangd/CodeComplete.cpp:583
 
-  Items->isIncomplete = !Index.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
-    Items->items.push_back(indexCompletionItem(Sym, Filter, SSInfo));
-  });
+  // FIXME: figure out a way to merge the symbols from dynamic index and static
+  // index.
----------------
ioeric wrote:
> I would suggest also addressing this `FIXME` in this patch, or at least make 
> it possible to tell from which index source a candidate comes on the client 
> side. Simple concatenation from both indexes would make the results hard to 
> interpret, especially when we don't have a good merging algorithm at this 
> point.
The FIXME is mainly about the merging algorithm.

I still prefer to a follow-up on telling the client from which index source a 
completion item comes. I can foresee we might need to add an new field (like 
`Source`) in the `CompleitionItem`. Let's keep this patch simple at the moment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41668



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to