[PATCH] D64305: [clangd] Add path mappings functionality

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 7 03:04:27 PST 2020


sammccall added a comment.

Thanks! The latest snapshot looks good. Landing it now with a few minor tweaks mentioned below.

(And trivial local style things, we generally prefer to drop braces on simple if statements etc.)



================
Comment at: clang-tools-extra/clangd/PathMapping.cpp:26
+void recursivelyMap(llvm::json::Value &V, PathMapping::Direction Dir,
+                    const PathMappings &Mappings, const MapperFunc &MF) {
+  using Kind = llvm::json::Value::Kind;
----------------
Removed the MapperFunc argument here as it's always doPathMapping.

Then this is just applyPathMappings, so merged the two.


================
Comment at: clang-tools-extra/clangd/PathMapping.h:40
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const PathMapping &M);
+
----------------
changed to client=server to match the flag syntax


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:358
+    llvm::cl::desc(
+        "Comma separated list of '<client_path>=<server_path>' pairs "
+        "that can be used to map between file locations on the client "
----------------
I extended this doc a bit to clarify what "client" and "server" paths mean, and explain the first-match-wins.
(I don't think the example reflects first-match-wins, so I reversed the order)


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:675
+      auto Err = Mappings.takeError();
+      llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+      return 1;
----------------
changed to elog("{0}", Mappings.takeError());


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64305/new/

https://reviews.llvm.org/D64305





More information about the cfe-commits mailing list