Thanks! Committed in r243266 ~Aaron
On Mon, Jul 27, 2015 at 9:18 AM, Alexander Kornienko <[email protected]> wrote: > LG. Thanks! > > -- Alex > > On Mon, Jul 27, 2015 at 3:06 PM, Aaron Ballman <[email protected]> > wrote: >> >> On Mon, Jul 27, 2015 at 8:57 AM, Alexander Kornienko <[email protected]> >> wrote: >> > On Mon, Jul 27, 2015 at 2:56 PM, Alexander Kornienko <[email protected]> >> > wrote: >> >> >> >> Ah, forgot one more thing: clang-tidy suggests to add >> >> -header-filter='.*' >> >> in some cases. This needs to be updated as well. >> > >> > >> > Here: clang-tidy/tool/ClangTidyMain.cpp:192 >> >> That appears to be the only instance of it in the source or the docs. >> >> ~Aaron >> >> > >> >> >> >> >> >> -- Alex >> >> On Mon, Jul 27, 2015 at 2:35 PM, Aaron Ballman <[email protected]> >> >> wrote: >> >>> >> >>> On Mon, Jul 27, 2015 at 6:56 AM, Alexander Kornienko >> >>> <[email protected]> >> >>> wrote: >> >>> > Manuel, thanks for the correction. Omitting quotes would be a >> >>> > problem >> >>> > with >> >>> > `-checks *`, not `-checks=*` (which is used in the docs). >> >>> > >> >>> > Aaron, this patch looks almost good then. Note that the description >> >>> > of >> >>> > command-line arguments is just a dump of `clang-tidy -help`, so you >> >>> > need to >> >>> > fix the documentation in the code and then paste `clang-tidy -help` >> >>> > output >> >>> > to the .rst file and indent it appropriately. >> >>> >> >>> Thank you for pointing that out, I've corrected in this patch. >> >>> >> >>> > Could you also check whether the -config=... examples work on >> >>> > windows? >> >>> >> >>> They do work, from my simple tests. >> >>> >> >>> ~Aaron >> >>> >> >>> > It might also be useful to add a section describing the >> >>> > windows-specific >> >>> > aspects of clang-tidy usage some time in the future. >> >>> > >> >>> > -- Alex >> >>> > >> >>> > >> >>> > On Mon, Jul 27, 2015 at 11:57 AM, Manuel Klimek <[email protected]> >> >>> > wrote: >> >>> >> >> >>> >> I think that something starts with -check= on the disk is low >> >>> >> probability >> >>> >> enough that we don't lose much by not escaping it for unix users, >> >>> >> while >> >>> >> gaining a lot less confusion on windows. >> >>> >> >> >>> >> On Fri, Jul 24, 2015 at 5:39 PM Alexander Kornienko >> >>> >> <[email protected]> >> >>> >> wrote: >> >>> >>> >> >>> >>> I think, we need to leave the examples valid for unix-like shells >> >>> >>> and >> >>> >>> add >> >>> >>> a short section describing differences of shells or giving >> >>> >>> windows-specific >> >>> >>> usage instructions. Some examples are just impossible to make >> >>> >>> compatible >> >>> >>> with all shells (e.g. -checks='*', even though this is not >> >>> >>> particularly >> >>> >>> useful). >> >>> >>> >> >>> >>> >> >>> >>> On Fri, Jul 24, 2015 at 4:52 PM, Aaron Ballman >> >>> >>> <[email protected]> >> >>> >>> wrote: >> >>> >>>> >> >>> >>>> On Fri, Jul 24, 2015 at 8:44 AM, Manuel Klimek >> >>> >>>> <[email protected]> >> >>> >>>> wrote: >> >>> >>>> > On Fri, Jul 24, 2015 at 2:34 PM Aaron Ballman >> >>> >>>> > <[email protected]> >> >>> >>>> > wrote: >> >>> >>>> >> >> >>> >>>> >> On Fri, Jul 24, 2015 at 8:32 AM, Manuel Klimek >> >>> >>>> >> <[email protected]> >> >> >>> >> >>> >>>> >> wrote: >> >>> >>>> >> > Seems like we need different instructions for different >> >>> >>>> >> > shells >> >>> >>>> >> > then >> >>> >>>> >> > :( >> >>> >>>> >> > The problem is that otherwise the -*... can be subject to >> >>> >>>> >> > shell >> >>> >>>> >> > expansion if >> >>> >>>> >> > it happens to match some files. >> >>> >>>> >> >> >>> >>>> >> Ah, I kind of wondered if this was a shell issue. Thank you >> >>> >>>> >> for >> >>> >>>> >> the >> >>> >>>> >> verification! >> >>> >>>> >> >> >>> >>>> >> Do you think it makes sense to update the option parsing code >> >>> >>>> >> to >> >>> >>>> >> strip >> >>> >>>> >> the single quotes if they are present? >> >>> >>>> > >> >>> >>>> > >> >>> >>>> > No, I don't think it's the tool's job to handle idiosyncrasies >> >>> >>>> > of >> >>> >>>> > the >> >>> >>>> > various shells. >> >>> >>>> > For the docs I see two possibilities: >> >>> >>>> > a) have 2 versions, one for cmd.exe, one for *sh. >> >>> >>>> > b) the probability that users will actually have file named >> >>> >>>> > -something, is >> >>> >>>> > not that high, we use the non-quoted version >> >>> >>>> >> >>> >>>> I kind of lean towards (b) with the understanding (which may be >> >>> >>>> incorrect) that users of the shell are expected to understand >> >>> >>>> when >> >>> >>>> to >> >>> >>>> quote arguments and when not to. That being said, I don't have a >> >>> >>>> strong opinion on it. >> >>> >>>> >> >>> >>>> ~Aaron >> >>> >>>> >> >>> >>>> > >> >>> >>>> > Alex, thoughts? >> >>> >>>> > >> >>> >>>> >> >> >>> >>>> >> >> >>> >>>> >> ~Aaron >> >>> >>>> >> >> >>> >>>> >> > >> >>> >>>> >> > >> >>> >>>> >> > On Thu, Jul 23, 2015 at 7:49 PM Aaron Ballman >> >>> >>>> >> > <[email protected]> >> >>> >>>> >> > wrote: >> >>> >>>> >> >> >> >>> >>>> >> >> This patch addresses two issues (I can split the patch if >> >>> >>>> >> >> it >> >>> >>>> >> >> is >> >>> >>>> >> >> desired): >> >>> >>>> >> >> >> >>> >>>> >> >> 1) The docs have some non-ASCII characters in them that >> >>> >>>> >> >> aren't >> >>> >>>> >> >> really >> >>> >>>> >> >> required. >> >>> >>>> >> >> 2) The docs suggest setting the checks using single quotes, >> >>> >>>> >> >> which >> >>> >>>> >> >> does >> >>> >>>> >> >> not work (at least, on Windows). >> >>> >>>> >> >> >> >>> >>>> >> >> When you specify checks like -checks='-*,misc-some-check', >> >>> >>>> >> >> the >> >>> >>>> >> >> single >> >>> >>>> >> >> quotes are not stripped by the option parser. When >> >>> >>>> >> >> converting >> >>> >>>> >> >> the >> >>> >>>> >> >> flags into globs to pass along to regex, the single quotes >> >>> >>>> >> >> remain >> >>> >>>> >> >> as >> >>> >>>> >> >> part of the regular expression, and do not match >> >>> >>>> >> >> appropriately. >> >>> >>>> >> >> When >> >>> >>>> >> >> the single quotes are left off, the globs are correctly >> >>> >>>> >> >> generated. >> >>> >>>> >> >> >> >>> >>>> >> >> ~Aaron >> >>> >>> _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
