alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/tool/clang-tidy-diff.py:123-130
if args.fix:
command.append('-fix')
if args.checks != '':
command.append('-checks=' + quote + args.checks + quote)
if args.quiet:
command.append('-quiet')
if args.build_path is not None:
----------------
janosimas wrote:
> alexfh wrote:
> > janosimas wrote:
> > > janosimas wrote:
> > > > alexfh wrote:
> > > > > If we make the script leave out the `--` flag, we should stop
> > > > > forwarding these flags and the `extra_arg(_before)?` below. Otherwise
> > > > > it's too confusing (should one place -fix before `--` or after? what
> > > > > about `-warnings-as-errors`?).
> > > > >
> > > > > Please also update the usage example at the top.
> > > > What about keep the current `--` behavior and add a new flag
> > > > `-extra-tidy-flags` ?
> > > `-extra-tidy-arg` to maintain consistency.
> > What's the benefit of `-extra-tidy-arg` compared to pass-by arguments?
> `-extra-tidy-arg` would keep the current `--` behavior and cover other cases
> of extra flags not supported by this script.
> I was worried about how changing the behavior would affect people that
> already using it.
>
> -extra-tidy-arg would keep the current -- behavior and cover other cases of
> extra flags not supported by this script.
> I was worried about how changing the behavior would affect people that
> already using it.
Good point about backward compatibility, however it may be a good time to reset
the complexity at the expense of breaking the interface. Luckily, call-sites
will be easy to upgrade.
As you may have noticed, the system is already quite confusing. At the very
beginning the script tried to mimic clang-tidy command-line interface with an
addition of diff file parsing (which is translated to -line-filter=).
Gradually, the script got more and more options - some of them were just
forwarded to clang-tidy, some were needed for the script. Some options started
conflicting (e.g. the script's -p and clang-tidy's -p) and had to be given
different names (-path -> -p).
At this point adding another way to pass options to clang-tidy will just make
this all even more confusing. So I'm suggesting to clearly split the script's
options and clang-tidy's options:
clang-tidy-diff.py [-clang-tidy-binary ...] [-p ...] [-regex ...] [-iregex
...] -- [-fix] [-checks=...] [other clang-tidy options]
If there's a need to run clang-tidy with the fixed compilation database, the
second `--` will be needed, e.g.:
git diff -U0 HEAD^ | clang-tidy-diff.py -p1 -- -fix -- -std=c++11
The good thing is that script won't have to do anything extra to enable this
and that it's very easy to explain: everything after the first `--` is just
passed to clang-tidy verbatim.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49864
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
