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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
