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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry, our comments crossed...



================
Comment at: clangd/URI.cpp:57
+      Body = "/";
+    Body += path::convert_to_slash(AbsolutePath, path::Style::posix);
+    return (llvm::Twine(Scheme) + ":" + percentEncode(Body)).str();
----------------
ioeric wrote:
> sammccall wrote:
> > conversely, here you want native (which is the default)
> Don't we still need to convert \ to / on windows?
yes. `convert_to_slash(path, style)` assumes that `path` is in `style`.

`convert_to_slash("c:\\foo.txt", windows)` --> "c:/foo.txt"
`convert_to_slash("c:\\foo.txt", posix)` --> "c:\\foo.txt" (because that's a perfectly valid filename on a posix system! well, apart from the colon)
`convert_to_slash("c:\\foo.txt", native)` --> "c:/foo.txt" if the host is windows, "c:\\foo.txt" if the host is windows (this is what you want)



================
Comment at: clangd/URI.h:29
+  llvm::StringRef scheme() const { return Scheme; }
+  /// \brief Returns decoded authority.
+  llvm::StringRef authority() const { return Authority; }
----------------
ioeric wrote:
> sammccall wrote:
> > e.g. "//reviews.llvm.org"
> I think authority doesn't include leading "//"? 
You're right! my mistake.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41946