[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire marked 2 inline comments as done. steveire added inline comments. Comment at: clang-query/QueryParser.cpp:285 +if (VarStr.empty()) + return new InvalidQuery("expected variable name"); +if (Var == PQV_Invalid) aaron.ballman wrote: > This seem

[PATCH] D52857: [clang-query] Add non-exclusive output API

This revision was automatically updated to reflect the committed changes. Closed by commit rL345522: [clang-query] Add non-exclusive output API (authored by steveire, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52857?vs=171046&id=1

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some minor fixes. At some point (not necessarily as part of this patch), you should also update `docs\ReleaseNotes.rst` to broadly list the new features you've been add

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire updated this revision to Diff 171046. steveire added a comment. New command design Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52857 Files: clang-query/Query.cpp clang-query/Query.h clang-query/QueryParser.cpp clang-query/QueryParser.h unittests/clang-quer

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1272420, @steveire wrote: > Yep, that's the suggestion. That will result in commands such as: > > - `enable output detailed-ast` > - `disable output detailed-ast` > - `set output detailed-ast` > - `enable output diag` > - `disable

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire added a comment. Yep, that's the suggestion. That will result in commands such as: - `enable output detailed-ast` - `disable output detailed-ast` - `set output detailed-ast` - `enable output diag` - `disable output diag` - `set output diag` etc, which I think addresses all concerns. R

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1272276, @steveire wrote: > I prefer the API from Peter. I think it's a good additional step from where > Aaron and I reached in IRC discussion (this patch currently). > > I can change the patch to use that later if you agree Aaro

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire added a comment. I prefer the API from Peter. I think it's a good additional step from where Aaron and I reached in IRC discussion (this patch currently). I can change the patch to use that later if you agree Aaron? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52857

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire added a comment. > This is already not the case with set output diag etc. Oh, I'm wrong here. I misread your post, sorry. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52857 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire added a comment. > This seems confusing to me because I would expect set foo bar to set the > value of a variable foo to bar, This is already not the case with `set output diag` etc. That said, I don't mind spelling this ` output foo`. It's not my decision. It's up to the reviewers.

[PATCH] D52857: [clang-query] Add non-exclusive output API

pcc added a comment. I'm not really involved in clang-query development these days, so this is more of a drive-by comment. It wasn't really obvious to me what the wording of the help text for `set enable-output` meant: > Set whether to enable content non-exclusively. I figured out from the na

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire updated this revision to Diff 170439. steveire added a comment. Update docs Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52857 Files: clang-query/Query.cpp clang-query/Query.h clang-query/QueryParser.cpp clang-query/QueryParser.h unittests/clang-query/Query

[PATCH] D52857: [clang-query] Add non-exclusive output API

steveire updated this revision to Diff 170435. steveire retitled this revision from "[clang-query] Add non-exclusive output API " to "[clang-query] Add non-exclusive output API". steveire added a comment. New design Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52857 Files: