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