sammccall added inline comments.

================
Comment at: clangd/ClangdServer.cpp:119
+  auto FS = FSProvider.getFileSystem();
+  auto Status = FS->status(RootPath);
+  if (!Status)
----------------
why validate it?


================
Comment at: clangd/FindSymbols.h:30
 /// \p Limit limits the number of results returned (0 means no limit).
+/// \p RootPath Root path of the workspace. This is used as the hint path when
+/// resolving URIs.
----------------
Change to HintPath? There's no reason to couple this code to the fact that it's 
the workspace root in clangdserver.


================
Comment at: clangd/FindSymbols.h:30
 /// \p Limit limits the number of results returned (0 means no limit).
+/// \p RootPath Root path of the workspace. This is used as the hint path when
+/// resolving URIs.
----------------
sammccall wrote:
> Change to HintPath? There's no reason to couple this code to the fact that 
> it's the workspace root in clangdserver.
you sometimes pass "" here, so we should document the semantics of that


================
Comment at: unittests/clangd/TestFS.cpp:70
+/// unittest: is a scheme that refers to files relative to testRoot().
+/// URI body is relative path to testRoot().
 class TestScheme : public URIScheme {
----------------
is a path relative to testRoot()


================
Comment at: unittests/clangd/TestFS.cpp:93
     return URI(Scheme, /*Authority=*/"",
-               llvm::sys::path::convert_to_slash(Body));
+               StringRef(llvm::sys::path::convert_to_slash(Body)).ltrim('/'));
   }
----------------
why this change?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48290



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

Reply via email to