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

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 28 12:22:00 PDT 2023


HazardyKnusperkeks added inline comments.


================
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:
> jp4a50 wrote:
> > 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.
> Yep! I would prefer the `optional`, but as you pointed out, we already got `PPIndentWidth`using `-1`.
>From the C++ side I totally agree. One could use `value_or()`, which would make the code much more readable.
And just because `PPIndentWidth` is using -1 is no reason to repeat that, we could just as easily change `PPIndentWidht` to an optional.

But how would it look in yaml?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101



More information about the cfe-commits mailing list