[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a subscriber: klimek. MyDeveloperDay added a comment. I basically agree with all the comments, I agree with you that I doubt its ever used in SortIncludes:true and DisableFormat:true, I just saw this as a hole that probably based on Myrums Law (https://www.hyrumslaw.com/) m

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-23 Thread Sam Maier via Phabricator via cfe-commits
SamMaier added a comment. In D67843#1679529 , @thakis wrote: > 4. Make it so that if DisableFormat is explicitly set to true and > SortIncludes isn't explicitly set, then it disables SortIncludes. Or, put a > different way, when DisableFormat is set, set

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-23 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. 4. Make it so that if DisableFormat is explicitly set to true and SortIncludes isn't explicitly set, then it disables SortIncludes. Or, put a different way, when DisableFormat is set, set SortIncludes to false at that point. Then an explicit `DisableFormat: true; SortInc

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-23 Thread Sam Maier via Phabricator via cfe-commits
SamMaier added a comment. In D67843#1677983 , @MyDeveloperDay wrote: > I assume the intention was that users could have DisableFormat=true and > SortIncludes=true when they want to sort the includes but not perform any > additional formatting in the cod

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I assume the intention was that users could have DisableFormat=true and SortIncludes=true when they want to sort the includes but not perform any additional formatting in the code. I think by making this change you make it impossible to run clang-format through

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-20 Thread Sam Maier via Phabricator via cfe-commits
SamMaier updated this revision to Diff 221065. SamMaier added a comment. Diff with more context Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67843/new/ https://reviews.llvm.org/D67843 Files: clang/lib/Format/Format.cpp clang/test/Format/disable-format.cpp

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-20 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Makes sense to me, but krasimir should probably approve this. Please upload patches with lots of context (`git diff -U master`); that makes reviewing on phab easier. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67843/new/ https://re

[PATCH] D67843: DisableFormat also now disables SortIncludes

2019-09-20 Thread Sam Maier via Phabricator via cfe-commits
SamMaier created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. SamMaier added a reviewer: thakis. Previously, in order to have clang-format //not// format something, you had to give both: SortIncludes: false DisableFormat: true This is confusing to us