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 switch, let's put my
> function call here".
And then trying to format it with clang-format they'll be immediately thrown
off because clang-format gets the indentation wrong :).
But I don't want to argue this to death. I understand what you are saying. I
just don't think any of your suggested formats make this situation better.
> I am not saying it happens often, I am just saying it breaks indentation :
> i.e. that two nested blocks should not close at the same depth. Breaking such
> things makes code harder to read/understand, and when you don't properly get
> the code you can more easily make a mistake. Obviously it should be caught in
> testing, but it is not always.
>
> That said, I am not trying to say in any way that my approach is better. I
> think both `CaseBlockIndent = Block` or your variant (with the extra line
> break before opening brace; but applying it all the time) will indeed be
> consistent with the code and not break indentation; keeping the current
> behavior when `IndentCaseLabels = true` is also not a problem IMHO.
An extra thing to consider is that my approach is also consistent with having
something before this block:
case A:
{
f();
g();
}
h();
break;
case B:
f();
{
g();
}
h();
break;
>> And to me (but this is obviously objective), your suggestions hide the
>> structure of the code even more as they lead to a state where the closing
>> brace is not aligned with the start of the line/statement that contains the
>> opening braces. That looks like a bug to me more than anything else and I
>> don't think there is another situation where clang-format would do that.
>
> The exact same thing happens for functions blocks, class blocks and control
> statements, depending on BraceWrapping modes. Actually, IMO wrapping the
> opening brace should probably be done according to
> `BraceWrapping.AfterControlStatement` flag.
>
> // BraceWrapping.AfterControlStatement = false
> switch (x) {
> case 0: {
> foo();
> break;
> }
> }
> // BraceWrapping.AfterControlStatement = true
> switch (x)
> {
> case 0:
> {
> foo();
> break;
> }
> }
>
> But I agree the `CaseBlockIndent = ClosingBrace` mode is definitely not
> consistent with the code. I think it is clearer this way, but that is
> definitely my own subjective opinion: in this case, I accept to lose the
> extra indent for the sake of compactness and to somehow mimic the "regular"
> case blocks (e.g. which do not include an actual block), but that is indeed
> really my personnal way to read code.
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).
Repository:
rC Clang
https://reviews.llvm.org/D43183
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
