[PATCH] D47935: [clangd] Boost completion score according to file proximity.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 15 00:41:14 PDT 2018


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks! Just nits



================
Comment at: clangd/Quality.cpp:304
   OS << formatv("\tForbidden: {0}\n", S.Forbidden);
-  OS << formatv("\tProximity: {0}\n", S.ProximityScore);
+  OS << formatv("\tIndexSymbolURI: {0}\n", S.IndexSymbolURI);
+  if (S.FileProximityMatch) {
----------------
No camel case here, just words


================
Comment at: clangd/Quality.cpp:305
+  OS << formatv("\tIndexSymbolURI: {0}\n", S.IndexSymbolURI);
+  if (S.FileProximityMatch) {
+    OS << "\t" << *S.FileProximityMatch << "\n";
----------------
Could this just be inlined like the others?

`Index proximity: 0.5 (ProximityRoots{foo/bar.h})`


================
Comment at: clangd/Quality.h:71
 
+class FileProximityMatcher {
+  public:
----------------
nit: can we forward declare this here and move it down (e.g. above TopN) to keep the signals at the top?
(I suspect it'll end up in another header eventually)


================
Comment at: clangd/Quality.h:99
+  /// query.
+  llvm::StringRef IndexSymbolURI;
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
----------------
nit: just SymbolURI (signals should be conceptually source-independent, may be missing)


================
Comment at: clangd/Quality.h:101
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
-  float ProximityScore = 0;
+  float SemaProximityScore = 0;
 
----------------
can you add a FIXME to unify with index proximity score? signals should be source-independent


================
Comment at: unittests/clangd/TestFS.h:60
+// This anchor is used to force the linker to link in the generated object file
+// and thus register unittest: URI scheme plugin.
+extern volatile int UnittestSchemeAnchorSource;
----------------
document the unittest: scheme here?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47935





More information about the cfe-commits mailing list