[PATCH] D41946: [clangd] Add support for different file URI schemas.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/URI.h:31
+public:
+  /// \brief Returns decoded scheme e.g. "https"
+  llvm::StringRef scheme() const { return Scheme; }
----------------
sammccall wrote:
> nit: prefer to omit `\brief` unless you want the brief comment to be something other than the first sentence.
> 
> (I know we have this in lots  of places - it adds noise, and I finally got around to looking it up in the style guide!)
Good to know. Thanks!


================
Comment at: unittests/clangd/URITests.cpp:1
+//===-- URITests.cpp  ---------------------------------*- C++ -*-----------===//
+//
----------------
sammccall wrote:
> you may want a test that round-trips `getVirtualTestFilePath("x")` through a file URI and back again.
> This should exercise the platform-specific code path - that path is under C:\ on windows, and /tmp/ on linux. The buildbots should give us platform coverage.
Good idea! Didn't know such a nice function exists.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41946