[PATCH] D33201: [clangd] Refactor ProtocolHandlers to decouple them from ClangdLSPServer

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdLSPServer.cpp:20
+template <typename T>
+std::string replacementsToEdits(StringRef Code, const T &Replacements) {
+  // Turn the replacements into the format specified by the Language Server
----------------
krasimir wrote:
> Hm, this is a bit too generic for my taste. Is it ever used generically?
Nope, it's now only used with one type, specialized it to it.


================
Comment at: clangd/ClangdLSPServer.h:33
+  /// each instance of ClangdLSPServer.
+  void run(std::istream &In);
 
----------------
krasimir wrote:
> It seems strange to have the In here and the Out in the constructor.
Didn't want to store unnecessary fields in the class, but we do need JSONOutput &Out as a member, since there's consumeDiagnostics which uses it.
Would you prefer std::istream &In to be moved into a constructor parameter and stored as a field?


https://reviews.llvm.org/D33201