[PATCH] D146101: [clang-format] Add DesignatedInitializerIndentWidth option.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D146101#4204653 <https://reviews.llvm.org/D146101#4204653>, @jp4a50 wrote:

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

I really doubt it as the creators of clang-format and a number of reviewers/contributors/maintainers from the recent past were Google engineers, I believe.

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

IMO it's clear that in Google style IndentWidth = 2 and ContinuationIndentWidth = 4. I think in general IndentWidth is for block indent and ContinuationIndentWidth is for everything else.



================
Comment at: clang/include/clang/Format/Format.h:2037
+  /// \endcode
+  int DesignatedInitializerIndentWidth;
+
----------------



================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:1659
       opensProtoMessageField(Current, Style)) {
+    const FormatToken *NextNoComment = Current.getNextNonComment();
     if (Current.opensBlockOrBlockTypeList(Style)) {
----------------
Can you also move it into the `else if` below to reduce the scope of the pointer? Please note the typo `NextNoComment`.


================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+      const auto DesignatedInitializerIndentWidth =
+          Style.DesignatedInitializerIndentWidth < 0
+              ? Style.ContinuationIndentWidth
+              : Style.DesignatedInitializerIndentWidth;
+      NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;
----------------
Using -1 to mean `ContinuationIndentWidth` is unnecessary and somewhat confusing IMO.


================
Comment at: clang/lib/Format/Format.cpp:1372
   LLVMStyle.DerivePointerAlignment = false;
+  LLVMStyle.DesignatedInitializerIndentWidth = -1;
   LLVMStyle.DisableFormat = false;
----------------



================
Comment at: clang/unittests/Format/ConfigParseTest.cpp:250-251
+              DesignatedInitializerIndentWidth, 34);
+  CHECK_PARSE("DesignatedInitializerIndentWidth: -1",
+              DesignatedInitializerIndentWidth, -1);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
----------------
Delete.


================
Comment at: clang/unittests/Format/FormatTest.cpp:4845
+  Style.ContinuationIndentWidth = 8;
+  Style.DesignatedInitializerIndentWidth = -1; // Use ContinuationIndentWidth.
+  verifyFormat("auto s = SomeStruct{\n"
----------------
Delete it and rearrange the tests so that the unspecified (default) `DesignatedInitializerIndentWidth` is tested first.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146101/new/

https://reviews.llvm.org/D146101