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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 marked an inline comment as done.
jp4a50 added a comment.

All actionable comments have been addressed.



================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:1665-1669
+      const auto DesignatedInitializerIndentWidth =
+          Style.DesignatedInitializerIndentWidth < 0
+              ? Style.ContinuationIndentWidth
+              : Style.DesignatedInitializerIndentWidth;
+      NewIndent = CurrentState.LastSpace + DesignatedInitializerIndentWidth;
----------------
owenpan wrote:
> owenpan wrote:
> > Using -1 to mean `ContinuationIndentWidth` is unnecessary and somewhat confusing IMO.
> Please disregard my comment above.
Just to make sure we are on the same page, does this mean that you are happy with the approach of using `-1` as a default value to indicate that `ContinuationIndentWidth` should be used?

I did initially consider using `std::optional<unsigned>` and using empty optional to indicate that `ContinuationIndentWidth` should be used but I saw that `PPIndentWidth` was using `-1` to default to using `IndentWidth` so I followed that precedent.


================
Comment at: clang/unittests/Format/FormatTest.cpp:4828
+               "    .yyyyyyyyyyyyyyyyyy = 2,\n"
+               "    .zzzzzzzzzzzzzzzzzz = 3};\n",
+               Style);
----------------
MyDeveloperDay wrote:
> jp4a50 wrote:
> > MyDeveloperDay wrote:
> > > Can we fix the brace positioning too
> > 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.
> part of me would like an option to not have to supply the trailing comma, 
I agree that it's slightly odd behaviour but I think it's pretty orthogonal to this change and would deserve it's own diff.

I think it's also pretty debatable what the behaviour *should* be. Should the `AlignAfterOpenBracket` option dictate the formatting of braces like this? Oddly enough, specifying `AlwaysBreak` (as I have done in this test) does indeed result in breaking after the `{` but changing it to `BlockIndent` doesn't put the closing `}` on a newline. Should we instead add a new `AlignAfterOpenBrace` analogue of `AlignAfterOpenBracket`?

Would you like me to to raise a separate issue to track this (if it doesn't already exist)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101