sammccall added a comment. Thanks, this is a great idea.
It also opens the door to suppressing includes of particular files later (e.g. by regex) with another config option, though we need to be careful of performance there. ================ Comment at: clang-tools-extra/clangd/Config.h:27 +#include "CodeComplete.h" #include "support/Context.h" ---------------- Config shouldn't depend on feature headers as it gets included everywhere :-( This is a pain point, in general we'd need to define the same enum in two places, but often we find a workaround. Here I think CodeCompleteOpts.IncludeInsertion is only actually referenced in two places (one production + one test), so it seems like it might be simplest just to remove that member from CodeCompleteOpts and read it from Config directly instead? ================ Comment at: clang-tools-extra/clangd/ConfigFragment.h:292 + /// - IWYU + llvm::Optional<Located<std::string>> HeaderInsertion; }; ---------------- I'm torn on the naming here. I think `InsertIncludes` would be a much better name: Insert vs Insertion is simpler and more active, and Includes is more accurate and descriptive than Headers. On the other hand, using exactly the same naming as the command-line flag makes it clear that it's the same option. I think *probably* we should do the rename - we'll eventually deprecate the command-line arg and it's better to have the good name in the long run. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134384/new/ https://reviews.llvm.org/D134384 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
