[PATCH] D36150: [clangd] LSP extension to switch between source/header file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.

Also, +1 to the comments about file extensions, we have to cover at least `.c`, `.cc` and `.cpp` for source files, `.h` and `.hpp` for header files.



================
Comment at: clangd/ClangdLSPServer.cpp:228
+        R"({"jsonrpc":"2.0","id":)" + ID.str() +
+        R"(,"result":)" + result + R"(})");
+}
----------------
We should probably use `Uri` here.


================
Comment at: clangd/ClangdServer.h:185
 
+  std::string switchSourceHeader(std::string path);
+
----------------
Please use descriptive typedefs for paths: `Path` (equivalent to `std::string`) for return type and `PathRef` (equivalent to `StringRef`) for parameter type.


================
Comment at: clangd/ClangdServer.h:185
 
+  std::string switchSourceHeader(std::string path);
+
----------------
ilya-biryukov wrote:
> Please use descriptive typedefs for paths: `Path` (equivalent to `std::string`) for return type and `PathRef` (equivalent to `StringRef`) for parameter type.
Maybe add a small comment for this function to indicate it's a fairly trivial helper?


================
Comment at: clangd/ProtocolHandlers.cpp:260
+  Dispatcher.registerHandler(
+      "textDocument/switchSourceHeader",
+      llvm::make_unique<SwitchSourceHeaderHandler>(Out, Callbacks));
----------------
I don't see this method in the [[ https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md | LSP spec ]].
Is this some kind of extension?

Could you add some comments on that with pointers to proposal/explanation of where this extension is used?


https://reviews.llvm.org/D36150