[clang-format] Add DesignatedInitializerIndentWidth option.

[jp4a50]

Summary

The option allows users to specify how many columns to use to indent
designated initializers that start on a new line.

This addresses the corresponding github issue, albeit in a slightly different way to what was proposed there.

[jp4a50]

Apply clang-format.

[jp4a50]

clang/docs/ClangFormatStyleOptions.rst

2704

Perhaps it would be better to make this a signed integer and default to -1 which would cause ContinuationIndentWidth to be used (i.e. current behaviour)?

[MyDeveloperDay]

clang/include/clang/Format/Format.h

2031

Endcode

clang/lib/Format/Format.cpp

905

Needs a parse test

clang/unittests/Format/FormatTest.cpp

4828

Can we fix the brace positioning too

[owenpan]

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options. Is there a way to fix the issue without adding a new option?

[MyDeveloperDay]

clang/lib/Format/Format.cpp

1372

so lets say someone is using an IndentWidth of 2 and now you introduce this as being 4 here as the default

Without them changing anything, all their DesignatedIntializer code will get refactored to a IndentWidth of 4 rather than the 2 it was previously

This is where we get called out for "changing the defaults", which really we are not we are just reclassifying how it behaves.

What we normally say here is can you point us to a public style that has an independent DesignatedIntializerIndentWidth which is independent from the levels of IndentWidth everywhere else.

Whilst I can see more knobs feels good, this will change code and we'll have to manage that expectation.

[jp4a50]

clang/lib/Format/Format.cpp

1372

Yep, so as per my comment in clang/docs/ClangFormatStyleOptions.rst I think I am changing my mind about this anyway.

My motivation for making this change is to support the KJ style guide which is quoted in the github issue - I work on a team that uses the KJ style guide.

The KJ style guide wants a designated initializer indent width of 2 along with a "normal" indent width of 2, so there is no explicit need for us to have those two values be different.

When originally making these changes, I did think that having "more knobs" was a good idea, but I agree that this could lead to annoying behaviour for some users and they would probably expect the designated initializer indent to match either the normal indent or the continuation indent.

How about I change the option to an integer and, when it's -1 (the default), the designated initializer indent matches the continuation indent, but if it is set to a value >= 0 then that number of columns is used instead?

clang/unittests/Format/FormatTest.cpp

4828

What would you expect the brace positioning to be here? I noticed that, with this configuration at least, the closing brace goes at the end of the same line (as the last initializer) when there is no trailing comma but goes on a new line if there is a trailing comma.

[jp4a50]

Change DesignatedInitializerIndentWidth to a signed integer which defaults to ContinuationIndentWidth when set to -1.

[jp4a50]

clang/lib/Format/Format.cpp

1372

I've reimplemented the option as a signed integer as per my suggestion above.

[jp4a50]

In D146101#4195293, @owenpan wrote:

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options. Is there a way to fix the issue without adding a new option?

@owenpan We could simply change the indentation level of designated initializers for everyone to match IndentWidth instead of ContinuationIndentWidth but I suspect that other users might be unhappy if we made that breaking change? I guess we could also consider making this behaviour only kick in when a specific set of *other* options are specified (e.g. AlwaysBreak) so that it applies to our clang-format style and not *all* others but that just feels like a hack.

I understand the added complexity and maintenance burden of a new option but we do meet the 3 criteria listed in your link.

• it is part of the KJ style guide which is used by the capn proto project which has over 100 maintainers
• the style guide is publicly accessible here
• I'm willing to contribute and maintain patches :)

I think it's also worth noting that the google style guide gives an example of designated initializers indented at 2 spaces (whereas their "continuation indent" for wrapped function parameters is 4).

[owenpan]

In D146101#4201842, @jp4a50 wrote:

I understand the added complexity and maintenance burden of a new option but we do meet the 3 criteria listed in your link.

• it is part of the KJ style guide which is used by the capn proto project which has over 100 maintainers
• the style guide is publicly accessible here

The style guide doesn't mention indenting designated initializers with 2 spaces?

• I'm willing to contribute and maintain patches :)

I think it's also worth noting that the google style guide gives an example of designated initializers indented at 2 spaces (whereas their "continuation indent" for wrapped function parameters is 4).

It's likely an error that the designated initializer example there shows 2-space indents as clang-format uses the 4-space continuation indent width:

clang-format -style=Google
Point p = {
    .x = 1.0, .y = 2.0,
    // z will be 0.0
};
Point p = {
    .x = 1.0, .y = 2.0,
    // z will be 0.0
};

[jp4a50]

The style guide doesn't mention indenting designated initializers with 2 spaces?

That is a fair point. I will get it updated because the author and maintainer of that style guide is the one requesting this change since, in practice, codebases following this style guide indent designated initializers with 2 spaces.

I think it's also worth noting that the google style guide gives an example of designated initializers indented at 2 spaces (whereas their "continuation indent" for wrapped function parameters is 4).

It's likely an error that the designated initializer example there shows 2-space indents as clang-format uses the 4-space continuation indent width:

This is possible, but isn't it also possible that they would prefer designated initializers to be indented at 2 spaces but don't have the option with clang-format currently?

The only general information supplied in the google style guide about indentation is as follows:

• Default indentation is 2 spaces.
• Wrapped parameters have a 4 space indent.

If we are taking a strict definition of parameters, the above suggests to me that designated initializers would fall under the default indentation of 2 spaces.

As mentioned on my other diff, I'm away until next Monday now so won't be able to get back to further comments til then.

[MyDeveloperDay]

What I'd really like to see, is... in the event that ContinuationIndentWidth is set and I do NOTset DesignatedInitializerIndentWidth , then DesignatedInitializerIndentWidth would inherit from that (not the default as you have here), if I set ContinuationIndentWidthto 3 DesignatedInitializerIndentWidth should be 3

Otherwise if I set DesignatedInitializerIndentWidth explicitly then it can be what is set,

i.e. (for base LLVM style)

ContinuationIndentWidth is not set = (default) 4
DesignatedInitializerIndentWidth = not set  (implicitly 4)

ContinuationIndentWidth is set = 3
DesignatedInitializerIndentWidth = not set  (implicitly 3)

ContinuationIndentWidth is not set = (default) 4
DesignatedInitializerIndentWidth = is set to 2  (explicitly 2)

ContinuationIndentWidth is set = 3
DesignatedInitializerIndentWidth = is set to 4 (explicitly 4)

clang/unittests/Format/FormatTest.cpp

4828

part of me would like an option to not have to supply the trailing comma,