[PATCH] D90531: [clangd] Add clang-tidy options to config

[Nathan James via Phabricator via cfe-commits]

njames93 marked 13 inline comments as done.
njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/ConfigFragment.h:183
+  struct ClangTidyBlock {
+    llvm::Optional<Located<bool>> Enable;
+    /// List of checks to enable or disable, can use wildcards.
----------------
sammccall wrote:
> I wonder if it's worth having this when we support `Remove: *`.
> 
> technically this does something slightly different:
>  - it still runs the machinery just with no actual checks
>  - you can enable: true later to override it, without losing the previously configured list of checks
> 
> Is one of these important? Is there some other benefit?
> (I'm not opposed to having this if you want it, but I'm curious)
I'm not 100% sure what you are asking here.


================
Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:188
 
+  class DynamicDictParser {
+  public:
----------------
sammccall wrote:
> instead of adding a second class for this, can we reuse DictParser, and change `unrecognized()` so:
>  - the callback gets access to the key location and the value
>  - the callback is responsible for emitting the "unknown key" error if it wants one
> 
> i.e. the default `unrecognized` handler is:
> 
> ```
> [this](Located<std::string> Key, Node &Value) { Outer->warning("Unknown " + Description + " key " + *Key); }
> ```
> 
> and we replace it for parsing CheckOptions
Not sure I'm a huge fan of that approach. I can't imagine a use case where the 2 dictionary modes will be used at the same time so there isn't a need for it to support both. 
By that I mean you wont have a dictionary where its expected to have both known and unknown keys.

`DictParser` could well be implemented as a a specialisation of `DynamicDictParser` but not much would be gained from that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90531