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

[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 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