<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 15, 2014 at 10:34 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On Thu, May 15, 2014 at 10:09 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><p dir="ltr"><br>
On 15 May 2014 10:03, "Manuel Klimek" <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br>
><br>
> After thinking a bit about this, I'd like to revisit this from scratch (as it has changed quite a bit).<br>
><br>
> Why is: -check=<regexp> not enough? Give it a good enough default value (that is printed if you say -help), and we have the following use cases:<br>
> - add something to the default set: clang-tidy -help, copy default set, add own</p>
</div><p dir="ltr">This is already problematic: making the user copy default value and adjust it is inconvenient.</p></blockquote></div><div>On the other hand, I expect it to take less time for a user to copy and adjust a simple regexp than to understand how the list of regexps is built up.</div>
</div></div></div></blockquote><div><br></div><div>It's a list of globs, regexps seem to be an overkill for this purpose. It could be a list of prefixes, but globs seem to be more intuitive.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> I'd say that having to figure out and remember that you need to add "-.*" in front of your checks is more inconvenient than to do -help and copy a regexp. </div>
</div></div></div></blockquote><div><br></div><div>The problem with current approach is that we have -checks= and -disable-checks= as two different flags, which are applied in a certain order and don't inherit default values. This has many implications:</div>
<div> * it's impossible to just re-enable some of the disabled checks (e.g., you want to run normal set of checks + a single experimental check, which is disabled by default)</div><div> * it's difficult to disable one check: just specifying -disable-checks=one-noisy-check resets the default value of -disable-checks=, which could contain 'many-buggy-checks|many-noisy-checks'. This is far from being convenient and is rarely what the user wants.<br>
</div><div><br></div><div>So I suggest this behavior as a more convenient and flexible alternative.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div>One question is: what's the more common use case?</div><div>I'd expect this:</div><div>- people will configure the checks they want to run in general for their project once</div><div>- people will want to run a single check to see what it triggers, if it's fine, they'll add it to their configuration</div>
</div></div></div></blockquote><div><br></div><div>There are more use-cases to consider:</div><div> * convenience of the command-line interface: first-time users/experimenters may want to conveniently try different checks and their combinations without creating/editing configuration files - this needs the ability to adjust default settings without completely resetting them.</div>
<div> * discoverability of new checks: regular users, who want to automatically get all new checks, and disable only the ones they don't like/need - this needs wildcards and negative filters.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div>Do you expect users will change the checks they use a lot?</div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"> Also, how do you exclude something? Say, you run clang-tidy and see a number of results from checks you don't care about. How do you exclude them?</p>
</blockquote></div><div>I figure out which checks I care about and include them.</div></div></div></div></blockquote><div><br></div><div>It's inconvenient when the list of checks is large.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>
<p dir="ltr">> - run only a single check: -check=my-full.check<br>
> - run only google checks: -check=google-.*<br>
><br>
><br>
><br>
><br>
><br>
> On Wed, May 14, 2014 at 10:13 PM, Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>> wrote:<br>
>><br>
>> Hi klimek,<br>
>><br>
>> Make checks filtering more intuitive and easy to use. Remove<br>
>> -disable-checks and change the format of -checks= to a comma-separated list of<br>
>> globs with optional '-' prefix to denote exclusion. The -checks= option is now<br>
>> cumulative, so it modifies defaults, not overrides them. Each glob adds or<br>
>> removes to the current set of checks, so the filter can be refined or overriden<br>
>> by adding globs.<br>
>><br>
>> Example:<br>
>> The default value for -checks= is<br>
>> '*,-clang-analyzer-alpha*,-llvm-include-order,-llvm-namespace-comment,-google-*',<br>
>> which allows all checks except for the ones named clang-analyzer-alpha* and<br>
>> others specified with the leading '-'. To allow all google-* checks one can<br>
>> write:<br>
>> clang-tidy -checks=google-* ...<br>
>> If one needs only google-* checks, we first need to remove everything (-*):<br>
>> clang-tidy -checks=-*,google-*<br>
>> etc.<br>
>><br>
>> I'm not sure if we need to change something here, so I didn't touch the docs<br>
>> yet.<br>
>><br>
>> <a href="http://reviews.llvm.org/D3770" target="_blank">http://reviews.llvm.org/D3770</a><br>
>><br>
>> Files:<br>
>> clang-tidy/ClangTidyDiagnosticConsumer.cpp<br>
>> clang-tidy/ClangTidyDiagnosticConsumer.h<br>
>> clang-tidy/ClangTidyOptions.h<br>
>> clang-tidy/tool/ClangTidyMain.cpp<br>
>> test/clang-tidy/arg-comments.cpp<br>
>> test/clang-tidy/basic.cpp<br>
>> test/clang-tidy/check_clang_tidy_fix.sh<br>
>> test/clang-tidy/check_clang_tidy_output.sh<br>
>> test/clang-tidy/deduplication.cpp<br>
>> test/clang-tidy/diagnostic.cpp<br>
>> test/clang-tidy/file-filter.cpp<br>
>> test/clang-tidy/fix.cpp<br>
>> test/clang-tidy/macros.cpp<br>
>> test/clang-tidy/nolint.cpp<br>
>> test/clang-tidy/select-checks.cpp<br>
>> test/clang-tidy/static-analyzer.cpp<br>
>> test/clang-tidy/temporaries.cpp<br>
>> unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp<br>
><br>
></p></div></div></blockquote></div></div></div></div></div></blockquote></div>
</div></div>