kbobyrev created this revision. kbobyrev added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang.
This is not a complete patch, but rather RFC: while the schema implementation itself does not seem very complicated it likely requires changes to the way we keep track of available schemas. Right now, llvm::Registry allows to .instantiate() found schemas but non of the existing ones require parameters. For the portable URI schema we do need to understand where the project is and hence it would require some internal stored parameters. One way of dealing with it would be exposing a singleton that would store those parameters assuming there could be only one RepoScheme at a time. This does not look very nice from my perspective since it'd be a part of some global state but it would do the trick. What do you think? Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D82354 Files: clang-tools-extra/clangd/URI.cpp clang-tools-extra/clangd/URI.h clang-tools-extra/clangd/unittests/URITests.cpp
Index: clang-tools-extra/clangd/unittests/URITests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/URITests.cpp +++ clang-tools-extra/clangd/unittests/URITests.cpp @@ -182,6 +182,16 @@ EXPECT_TRUE(FailedResolve("file:a/b/c")); } +TEST(URITest, RepoScheme) { + EXPECT_THAT(createOrDie("/home/username/dev/src/llvm-project/a/b/c", "repo"), + "repo://remoteindex.com/a/b/c"); + EXPECT_EQ(parseOrDie("repo://remoteindex.com/llvm/a/b/c").body(), + "/llvm/a/b/c"); + EXPECT_EQ(parseOrDie("repo://remoteindex.com/llvm/a/b/c").authority(), + "remoteindex.com"); + EXPECT_EQ(parseOrDie("repo://remoteindex.com/llvm/a/b/c").scheme(), "repo"); +} + } // namespace } // namespace clangd } // namespace clang Index: clang-tools-extra/clangd/URI.h =================================================================== --- clang-tools-extra/clangd/URI.h +++ clang-tools-extra/clangd/URI.h @@ -31,7 +31,7 @@ /// Returns decoded scheme e.g. "https" llvm::StringRef scheme() const { return Scheme; } - /// Returns decoded authority e.g. "reviews.lvm.org" + /// Returns decoded authority e.g. "reviews.llvm.org" llvm::StringRef authority() const { return Authority; } /// Returns decoded body e.g. "/D41946" llvm::StringRef body() const { return Body; } Index: clang-tools-extra/clangd/URI.cpp =================================================================== --- clang-tools-extra/clangd/URI.cpp +++ clang-tools-extra/clangd/URI.cpp @@ -7,7 +7,9 @@ //===----------------------------------------------------------------------===// #include "URI.h" +#include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/Error.h" #include "llvm/Support/Format.h" @@ -58,6 +60,55 @@ } }; +// FIXME(kirillbobyrev): Repo scheme requires two parameters: repository +// identifier (e.g. address; can be omitted in case there would be only one +// repository scheme used at a time which is probably true for the first +// iteration) and path to the project root. llvm::Registry which is used to +// store all known schemas does not support constructors with parameters. Hence, +// right now the prototype implementation contains hard-coded static parameters +// that should be actually passed to the constructor. Doing this properly +// requires modifications to the URISchemeRegistry pattern usage. +class RepoScheme : public URIScheme { +public: + // Authority points to the repository address, should be same as + // Repository. + // HintPath points to the repository root in filesystem, should be same as + // Root. + llvm::Expected<std::string> + getAbsolutePath(llvm::StringRef Authority, llvm::StringRef Body, + llvm::StringRef HintPath) const override { + if (Authority != Repository) + return make_string_error( + "Unable to resolve URI because authorities do not match: given " + + Authority + "have " + Repository); + if (HintPath != Root) + return make_string_error( + "Unable to resolve repository URI with given root " + HintPath + + " different from " + Root); + llvm::SmallString<20> AbsolutePath = llvm::StringRef(Root); + llvm::sys::path::append(AbsolutePath, Body); + return AbsolutePath.str().str(); + } + + llvm::Expected<URI> + uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override { + llvm::SmallString<12> Body = AbsolutePath; + // Strip Root from AbsolutePath. + if (!llvm::sys::path::replace_path_prefix(Body, Root, "")) + return make_string_error("Unable to create URI " + AbsolutePath + + " outside the repository " + Root); + return URI("repo", Repository, Body.str()); + } + + static const std::string Repository; + static const std::string Root; +}; + +const std::string RepoScheme::Repository = "remoteindex.com"; +const std::string RepoScheme::Root = "/home/username/dev/src/llvm-project"; + +static URISchemeRegistry::Add<RepoScheme> X("repo", "Remote indexing service"); + llvm::Expected<std::unique_ptr<URIScheme>> findSchemeByName(llvm::StringRef Scheme) { if (Scheme == "file") @@ -96,13 +147,13 @@ void percentEncode(llvm::StringRef Content, std::string &Out) { std::string Result; for (unsigned char C : Content) - if (shouldEscape(C)) - { + if (shouldEscape(C)) { Out.push_back('%'); Out.push_back(llvm::hexdigit(C / 16)); Out.push_back(llvm::hexdigit(C % 16)); - } else - { Out.push_back(C); } + } else { + Out.push_back(C); + } } /// Decodes a string according to percent-encoding. @@ -151,8 +202,7 @@ return Result; // If authority if empty, we only print body if it starts with "/"; otherwise, // the URI is invalid. - if (!Authority.empty() || llvm::StringRef(Body).startswith("/")) - { + if (!Authority.empty() || llvm::StringRef(Body).startswith("/")) { Result.append("//"); percentEncode(Authority, Result); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits