omtcyfz added a comment.
In https://reviews.llvm.org/D21814#500506, @vmiklos wrote:
> Rebased on top of r277131 and resolved conflicts.
>
> > As for help message, look at clang-tidy. Is there a need in helpMain?
>
>
> I think so; we have this chicken-and-egg problem (see earlier comments of this
> review), that the options parser wants to know the option category, but we
> only
> know the option category after parsing options due to subcommands. This is
> not
> a problem for clang-tidy that doesn't have subcommands (as far as I see).
>
> So one way is the own code for handling the subcommands (that's what I did
> here, and that's what e.g. llvm-cov does), an other way would be to extend
> `tooling::CommonOptionsParser`, so it doesn't want a category in the ctor.
> That
> requirement is a problem for us, since we have two categories, so we can't
> give
> the correct one without parsing the options.
>
> So all in all, the best seems to me is to go with a simple helpMain().
Aha. I see. Well, it's not clean, but we can probably just deal with it later.
Otherwise it gets too long and the patch is never going to be landed. Just
write a `FIXME` then, I think I may look into that on the next week or somewhen.
>
>
> > besides, let me push one thing; it's about passing a vector of USRs to the
> > USRLocFinder instead of passing them 1 by 1; removes a need to write that
> > FIXME of yours :)
>
>
> Great, I've removed it then.
One more paranoid comment:
`class Cla1 { // CHECK: class Kla1`
Most of the time I use `Foo`->`Bar` renaming in tests, just so that it could be
uniform and everyone (including myself) would typically `grep -FUbo "Foo"`
without looking at the source.
Other than that my concerns have been addressed.
Again, I'm not happy about this approach, but if that's a use-case we expect to
have in the future and everyone is happy about it - let's do that.
https://reviews.llvm.org/D21814
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
