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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
