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

This revision was automatically updated to reflect the committed changes. Closed by commit rL323101: [clangd] Add support for different file URI schemas. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D41946 Files:

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

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 > so

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

ioeric updated this revision to Diff 130862. ioeric marked 4 inline comments as done. ioeric added a comment. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: clangd/CMakeLists.txt clangd/URI.cpp clangd/URI.h unittests/clangd/CMak

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG with some nits addressed Comment at: clangd/URI.cpp:138 + + auto Pos = Uri.find(':'); + if (Pos == llvm::StringRef::npos) ioeric wrote: > sammccal

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

ioeric added inline comments. Comment at: clangd/URI.cpp:57 + Body = "/"; +Body += path::convert_to_slash(AbsolutePath, path::Style::posix); +return (llvm::Twine(Scheme) + ":" + percentEncode(Body)).str(); sammccall wrote: > ioeric wrote: > > sammcca

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

ioeric updated this revision to Diff 130766. ioeric marked 16 inline comments as done. ioeric added a comment. - Addressed review comments. - Check windows vs unit paths in tests. - Properly check absolute paths. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: cla

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

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:

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

sammccall added inline comments. Comment at: clangd/URI.cpp:94 + +std::vector createEncodeMap() { + std::vector Result(128, ""); Performance doesn't really matter here, but it looks like this is intended to save time by precomputing, and I don't think it's any

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

ioeric updated this revision to Diff 130621. ioeric marked 14 inline comments as done. ioeric added a comment. - Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: clangd/CMakeLists.txt clangd/URI.cpp clangd/URI.h unittests/clangd/CMakeL

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

ioeric added inline comments. Comment at: clangd/URI.cpp:43 + Body.consume_front("/"); +return llvm::sys::path::convert_to_slash(Body); + } sammccall wrote: > Don't you want the opposite here, convert from unix to native? Ah, right! C

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

sammccall added a comment. Few more interface nits, sorry for picking here! Will get through the impl in the morning. Comment at: clangd/URI.cpp:43 + Body.consume_front("/"); +return llvm::sys::path::convert_to_slash(Body); + } Don't you want the opp

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

ioeric added a comment. Thanks for the comments! I've refactored the code according to your suggestions. PTAL Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

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

ioeric updated this revision to Diff 130201. ioeric marked 17 inline comments as done. ioeric added a comment. Herald added a reviewer: jkorous-apple. - Addressed review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41946 Files: clangd/CMakeLists.txt clangd/URI.c

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

sammccall added a comment. Lots of quibbles here, some things we can certainly defer. Happy to chat offline if you're not sure... Comment at: clangd/URI.cpp:17 + +std::string FileURI::toString() const { return Schema + "://" + Path; } + This isn't correct, we

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

ioeric created this revision. ioeric added a reviewer: sammccall. Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek. DO NOT SUBMIT: We should replace the existing URI struct in Protocol.h with the new URI and rename FileURI to URI. But before doing that, I'd like to early feedba