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

Francois Ferrand via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 15 07:36:27 PST 2018


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

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


Repository:
  rC Clang

https://reviews.llvm.org/D43183





More information about the cfe-commits mailing list