[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 158313. Typz added a comment. Herald added a subscriber: acoomans. Regenerate documentation Repository: rC Clang https://reviews.llvm.org/D43183 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147079. Typz added a comment. rebase Repository: rC Clang https://reviews.llvm.org/D43183 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-03-02 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Got two responses so far. 1. Having to closing braces in the same column is weird, but not that weird. Consider namespace n { void f() { ... } } 2. Richard Smith (code owner of Clang) says that he doesn't really like the two closing braces in the same

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-28 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. New options for this would not be acceptable IMO. Too much cost for too little benefit. I'd suggest to first make the change to fall back to the style with a regular block when there are statements other than break after the closing brace. That is always bad, no

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > I don't agree that that's the same thing. The closing brace is still neatly > aligned with the line of the opening brace (which happens to be just the > opening brace). This invariant is not really applicable to switch statements, where code of each "branch" is already

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D43183#1008784, @Typz wrote: > A user can create an error by reasoning like this, after the code has been > formatted this way (a long time ago) : "oh, I need to make an extra function > call, but in all cases ah, here is the end of the

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. A user can create an error by reasoning like this, after the code has been formatted this way (a long time ago) : "oh, I need to make an extra function call, but in all cases ah, here is the end of the switch, let's put my function call here". I am not saying it

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. That doesn't explain to me how this is error prone. I can't think how you'd create an error by this that would not be caught by the compiler. Much less if you consistently use clang-format. It's fundamentally what you get for IndentCaseLabels: false. Even without

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D43183#1008632, @djasper wrote: > Yes, that's what I mean. What do you mean, the style is too error prone? When `IndentCaseLabels` is false, the code gets formatted in a way that "hides" the structure of the code, by indenting the end of the

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Yes, that's what I mean. What do you mean, the style is too error prone? Repository: rC Clang https://reviews.llvm.org/D43183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Hum, not sure I fully got your proposal. So you actually mean that clang-format you format like this (with 4-space indent for clarity): switch (x) { case 0: break; case 1: { foo(); break; } case 2: { foo(); } break; case 3: {

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D43183#1006224, @Typz wrote: > It is explicitly documented in google style guide: > https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements > : > > > case blocks in switch statements can have curly braces or not,

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements : > case blocks in switch statements can have curly braces or not, depending on > your preference. If you do include curly braces they should

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D43183#1006170, @Typz wrote: > > We'll just format those cases in a somewhat weird way and users can either > > accept that or change their code to not need it. > > I think we have a really diverging opinion on this. From my experience, >

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > We'll just format those cases in a somewhat weird way and users can either > accept that or change their code to not need it. I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question :

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. To me none of these options make sense. For the case you are describing, I agree that the current behavior is not ideal, but neither are any of the alternatives. However, I think that's fine. We'll just format those cases in a somewhat weird way and users can either

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Do you have a reference to style guides recommending any of this? Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. Do you have a reference to style guides recommending any of this? Repository: rC Clang https://reviews.llvm.org/D43183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. When a block is started after a case label, clang-format does add extra indent to the content of this block: the block content is indented one level (with respect to the switch) while the closing brace is not indented,