[PATCH] D49580: [clang-format] Adding style option for absolute formatting

Jacek Olesiak via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 30 02:57:55 PDT 2018


jolesiak added a comment.

First of all, thanks, Arnaud, for looking into this.

Let's address the mentioned bug first.
Let's assume that we use `IndentReference = Relative`. I think that this particular formatting bug is local (i.e. can be solved while processing a `@protocol` block). Look at this example:

  if (true) {
    int a = 42;
      int b = 42;
    int c = 42;
  }
  int d = 42;

When we run `clang-format -lines=4:4` we get:

  if (true) {
    int a = 42;
      int b = 42;
      int c = 42;
  }
  int d = 42;

Please note that neither `}` nor `int d = 42` is indented. 
But this is a different behavior from what we see after running `clang-format -lines=3:3` on:

  @protocol A
   @optional
  // comment
  - (void)f;
  @end
  MACRO

the output is:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO

Please note that `@end` is indented (has different indentation than `@protocol`; `MACRO` is indented as well).
To clarify why this is undesired, `@end` ends `@protocol`, not `@optional`. By the way, `clang-format` doesn't think that `@end` should match `@optional`, check the output of an extended example:

  @protocol A
   @optional
   // comment
   - (void)f;
   @end
   MACRO
   @end

Second `@end` still doesn't match the `@protocol`.

I think it's perfectly fine to allow users to have custom indentations if they so choose. In this particular example, though, `clang-format` should stop indenting when it reaches `@end`, giving the following output:

  @protocol A
   @optional
   // comment
   - (void)f;
  @end
  MACRO

Moving to the general case, I must say that introducing `IndentReference = Relative/Absolute` is a very interesting approach. I can imagine situations when somebody actually want to use `IndentReference = Absolute`. However, adding an additional option comes at quite big cost and I think that in this case it outweighs the benefits.
I think that a very good outcome could be to comment what is happening next to `IndentTracker.adjustToUnmodifiedLine(TheLine);` line. E.g. "We adjust an indentation to match the existing code to give users flexibility (instead of ignoring it). It's their responsibility to provide a correct formatting of lines they didn't initially change if these lines break formatting.".


https://reviews.llvm.org/D49580





More information about the cfe-commits mailing list