[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

PiotrZSL added a comment. Herald added a reviewer: njames93. Herald added a project: All. This need to be rebase due to conflict to land. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas marked 2 inline comments as done. janosimas added a comment. Yes, I'll need help landing it Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 400854. janosimas added a comment. I changed the wording and added an example as suggested. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 Files:

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. Do you need help landing the change? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___ cfe-commits mailing list

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good with one suggestion. Comment at: clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py:123 + 'lines.' +

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 398314. janosimas added a comment. Herald added a subscriber: carlosgalvezp. I added a comment in the docs noting this is a breaking change. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. In D49864#2857630 , @janosimas wrote: > That makes sense. > Should I add it somewhere? Or do I need to talk to someone? Please update clang-tools-extra/docs/ReleaseNotes.rst. It looks like there's no other docs for this script.

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. That makes sense. Should I add it somewhere? Or do I need to talk to someone? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 ___

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. Function-wise the patch looks good, but I'm somewhat concerned about silently breaking users. At the very least there should be a relevant entry in the release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 347622. janosimas added a comment. Fix test issue. Use correct clang-tidy flag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 Files:

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 347489. janosimas added a comment. Fix a mistake in my last patch. I added a `--` in a command that should not have it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

aaron.ballman added a comment. In D49864#274 , @janosimas wrote: > I manage to upload it, copy-pasting the content to the text box. I have no > idea why it is not working with the upload. > > Updates from the last change > > - Modified the tests to

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. > Hmm, I'm not certain what's going on there. When I try to upload a patch for > the review, Phab lets me upload it (but I didn't try to submit the changes). > At what stage are you getting the failure? Is it when uploading the patch > itself, or at some other point?

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 347459. janosimas added a comment. I manage to upload it, copy-pasting the content to the text box. I have no idea why it is not working with the upload. Updates from the last change - Modified the tests to match the input - Addend file context

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

aaron.ballman added a comment. In D49864#2777300 , @janosimas wrote: > I found the issue with my diff and I was able to get the expected file. > > I'm still having issues uploading the file, this is the message I get in the > browser console: > > POST

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. I found the issue with my diff and I was able to get the expected file. I'm still having issues uploading the file, this is the message I get in the browser console: POST https://reviews.llvm.org/differential/revision/update/49864/ 500 Repository: rG LLVM

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

aaron.ballman added a comment. In D49864#2776570 , @janosimas wrote: > Hi, sorry for taking so long for such a small change. > I did the changes and generated a diff with the requested context. > It's a huge file with a lot more diff than my changes, is

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. Hi, sorry for taking so long for such a small change. I did the changes and generated a diff with the requested context. It's a huge file with a lot more diff than my changes, is that right? When I try to upload it, I'm getting `Unhandled Exception ("Exception")`, no

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Apologies again for the long delay. A diff with full context would still be appreciated. Please read https://llvm.org/docs/Phabricator.html for instructions. The

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 298865. janosimas added a comment. Here a diff with the rebased code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864 Files:

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. Sorry for the delay. This patch fell through the cracks. If you're still interested, could you rebase it on top of current HEAD and upload a full diff? Or use the arcanist tool, see https://llvm.org/docs/Phabricator.html. Repository: rCTE Clang Tools Extra CHANGES

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. I also noticed there is a `clang-format-diff` that also has the `-p` option, it would be nice to update it for consistency. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49864/new/ https://reviews.llvm.org/D49864

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas updated this revision to Diff 234005. janosimas added a comment. Herald added a subscriber: mgehre. Herald added a project: clang. I reviewed the code over the discussion with the `--` option, I also changed the `-p` optin to `-strip` to avoid confusion with the `clang-tidy` option.

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. I like a lot of this syntax you proposed, makes a lot more sense to me. The script will be shorter and the doc will be "auto updated" with the clang-tidy doc. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49864

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

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=' +

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. I was thinking about the usage of `--` and `-extra-arg`, don't they do the same thing? To be honest, for me, the current behavior of `--` doesn't make much sense. If there is a use case for `-extra-arg-before` and `-extra-arg`, they are much more clearer in intent.

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added inline comments. 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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added inline comments. 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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas requested review of this revision. janosimas added inline comments. 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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added inline comments. 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

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

janosimas added a comment. To use in a git pre-commit I wanted to use the flags: -warnings-as-errors=* -header-filter=.* I wanted that the script returned an error value for my selected checks, even if they are just warnings. Repository: rCTE Clang Tools Extra

[PATCH] D49864: [clang-tidy] The script clang-tidy-diff.py doesn't accept 'pass by' options (--)

alexfh added a comment. This is intended, IIUC. The syntax of the clang-tidy-diff.py mirrors the syntax of clang-tidy itself, and the `--` option is used in the same way as in clang-tidy - to denote the start of compiler arguments (and switch to the fixed compilation database). Do you have a