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