[PATCH] D83705: [clangd] Config: CompileFlags.Remove

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:126
   void compile(Fragment::CompileFlagsBlock &&F) {
+    if (!F.Remove.empty()) {
+      auto Remove = std::make_shared<ArgStripper>();
----------------
the order of processing "Remove" and "Add" matters, if they are conflict, the `Add` will take precedence, I think this is intended? Maybe worth clarifying in the CompileFlagsBlock comments.


================
Comment at: clang-tools-extra/clangd/ConfigFragment.h:136
+    /// - If the value is a recognized clang flag (like "-I") then it will be
+    ///   removed along with any arguments. Synonyms like --include-dir= wil
+    ///   also be removed.
----------------
nit: wil => will


================
Comment at: clang-tools-extra/clangd/ConfigFragment.h:141
+    /// - Otherwise any argument exactly matching the value is removed.
+    ///
+    /// In all cases, -Xclang is also removed where needed.
----------------
nit: I'd prefer to give a few concrete examples (e.g. examples I mentioned in another patch), it would be helpful for readers to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83705