[PATCH] D83822: [clangd] Support config over LSP.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1220
       CDB->setCompileCommand(File, std::move(New));
       ModifiedFiles.insert(File);
     }
----------------
nit: maybe just set `ReparseAllFiles` in here too, and change the condition below to only `return ReparseAllFiles`


================
Comment at: clang-tools-extra/clangd/Protocol.h:494
+  // Each value is an Object or null, and replaces any fragment with that key.
+  // Fragments are used in key order ("!foo" is low-priority, "~foo" is high).
+  std::map<std::string, llvm::json::Value> fragments;
----------------
it feels like clients will only be using two types of keys, a "global" one for sending over (likely project-specific) user preferences and another for directory/file specific configs. The latter is likely to have the path as a key, whereas the former will likely be a special identifier. I am glad that `!` comes before both `/` and any capital letters(letter drives).

I think it is only important to have a distinction between these two types (a subset of what you describe here). Current design makes it really easy on our side, I am just afraid of confusing clients. Maybe we should just have an enum/boolean tag saying if the fragment is global or not instead of relying on the key orderings ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83822