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

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc12aa69a0be9: [clang-format] Add 
BracedInitializerIndentWidth option (authored by jp4a50, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D146101?vs=517868=518140#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4855,6 +4855,191 @@
"[5] = ee};");
 }
 
+TEST_F(FormatTest, BracedInitializerIndentWidth) {
+  auto Style = getLLVMStyleWithColumns(60);
+  Style.BinPackArguments = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BracedInitializerIndentWidth = 6;
+
+  // Non-initializing braces are unaffected by BracedInitializerIndentWidth.
+  verifyFormat("enum class {\n"
+   "  One,\n"
+   "  Two,\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  Foo() {}\n"
+   "  void bar();\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("auto foo = [&] {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("{\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  // Non-brace initialization is unaffected by BracedInitializerIndentWidth.
+  verifyFormat("SomeClass clazz(\n"
+   "\"xx\", \"yy\",\n"
+   "\"zz\");\n",
+   Style);
+
+  // The following types of initialization are all affected by
+  // BracedInitializerIndentWidth. Aggregate initialization.
+  verifyFormat("int LongVariable[2] = {\n"
+   "  1000, 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  \"\", \"\",\n"
+   "  \"\"};\n",
+   Style);
+  // Designated initializers.
+  verifyFormat("int LongVariable[1] = {\n"
+   "  [0] = 1000, [1] = 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  .foo = \"x\",\n"
+   "  .bar = \"y\",\n"
+   "  .baz = \"z\"};\n",
+   Style);
+  // List initialization.
+  verifyFormat("SomeStruct s{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("new SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Member initializer.
+  verifyFormat("class SomeClass {\n"
+   "  SomeStruct s{\n"
+   "\"x\",\n"
+   "\"y\",\n"
+   "\"z\",\n"
+   "  };\n"
+   "};\n",
+   Style);
+  // Constructor member initializer.
+  verifyFormat("SomeClass::SomeClass : strct{\n"
+   " \"x\",\n"
+   " \"y\",\n"
+   " \"z\",\n"
+   "   } {}\n",
+   Style);
+  // Copy initialization.
+  verifyFormat("SomeStruct s = SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Copy list initialization.
+  verifyFormat("SomeStruct s = {\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@jp4a50 There's a mismatch between Format.h and and the resulting rst file. 
I'll rerun dump_format_style.py to fix it before merging.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

In D146101#4270509 , @owenpan wrote:

> In D146101#4268487 , @jp4a50 wrote:
>
>> Would appreciate someone committing for me.
>
> Can you rebase it? I got a conflict in ReleaseNotes.rst.

@owenpan apologies for the delay. I've rebased now.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 517868.
jp4a50 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4855,6 +4855,191 @@
"[5] = ee};");
 }
 
+TEST_F(FormatTest, BracedInitializerIndentWidth) {
+  auto Style = getLLVMStyleWithColumns(60);
+  Style.BinPackArguments = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BracedInitializerIndentWidth = 6;
+
+  // Non-initializing braces are unaffected by BracedInitializerIndentWidth.
+  verifyFormat("enum class {\n"
+   "  One,\n"
+   "  Two,\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  Foo() {}\n"
+   "  void bar();\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("auto foo = [&] {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("{\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  // Non-brace initialization is unaffected by BracedInitializerIndentWidth.
+  verifyFormat("SomeClass clazz(\n"
+   "\"xx\", \"yy\",\n"
+   "\"zz\");\n",
+   Style);
+
+  // The following types of initialization are all affected by
+  // BracedInitializerIndentWidth. Aggregate initialization.
+  verifyFormat("int LongVariable[2] = {\n"
+   "  1000, 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  \"\", \"\",\n"
+   "  \"\"};\n",
+   Style);
+  // Designated initializers.
+  verifyFormat("int LongVariable[1] = {\n"
+   "  [0] = 1000, [1] = 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  .foo = \"x\",\n"
+   "  .bar = \"y\",\n"
+   "  .baz = \"z\"};\n",
+   Style);
+  // List initialization.
+  verifyFormat("SomeStruct s{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("new SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Member initializer.
+  verifyFormat("class SomeClass {\n"
+   "  SomeStruct s{\n"
+   "\"x\",\n"
+   "\"y\",\n"
+   "\"z\",\n"
+   "  };\n"
+   "};\n",
+   Style);
+  // Constructor member initializer.
+  verifyFormat("SomeClass::SomeClass : strct{\n"
+   " \"x\",\n"
+   " \"y\",\n"
+   " \"z\",\n"
+   "   } {}\n",
+   Style);
+  // Copy initialization.
+  verifyFormat("SomeStruct s = SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Copy list initialization.
+  verifyFormat("SomeStruct s = {\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Assignment operand initialization.
+  verifyFormat("s = {\n"
+   "  \"x\",\n"
+   "  

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D146101#4268487 , @jp4a50 wrote:

> Would appreciate someone committing for me.

Can you rebase it? I got a conflict in ReleaseNotes.rst.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

Commit details as follows as per other diffs:

Name: Jon Phillips
Email: jonap2...@gmail.com

Would appreciate someone committing for me.


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

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

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan 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;

HazardyKnusperkeks wrote:
> jp4a50 wrote:
> > owenpan wrote:
> > > HazardyKnusperkeks wrote:
> > > > MyDeveloperDay wrote:
> > > > > rymiel wrote:
> > > > > > owenpan wrote:
> > > > > > > jp4a50 wrote:
> > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > owenpan wrote:
> > > > > > > > > > jp4a50 wrote:
> > > > > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > > > > 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` 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?
> > > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > > 
> > > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > > parsing.
> > > > > > > > > > > 
> > > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > > because users may have *explicitly* specified `-1` in 
> > > > > > > > > > > their YAML.
> > > > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason 
> > > > > > > > > > > to repeat that, we could just as easily change 
> > > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > > 
> > > > > > > > > > We would have to deal with backward compatibility to avoid 
> > > > > > > > > > regressions though.
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > parsing.
> > > > > > > > > > 
> > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > because users may have *explicitly* specified `-1` in their 
> > > > > > > > > > YAML.
> > > > > > > > > 
> > > > > > > > > You need an explicit entry, because you need to be able to 
> > > > > > > > > write the empty optional on `--dump-config`.
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > 

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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
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;

jp4a50 wrote:
> owenpan wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > rymiel wrote:
> > > > > owenpan wrote:
> > > > > > jp4a50 wrote:
> > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > owenpan wrote:
> > > > > > > > > jp4a50 wrote:
> > > > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > > > 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` 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?
> > > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > > 
> > > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would 
> > > > > > > > > > set the `std::optional` to `4` but if 
> > > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the 
> > > > > > > > > > YAML then the optional would simply not be set during 
> > > > > > > > > > parsing.
> > > > > > > > > > 
> > > > > > > > > > Of course, if we were to change `PPIndentWidth` to work 
> > > > > > > > > > that way too, it would technically be a breaking change 
> > > > > > > > > > because users may have *explicitly* specified `-1` in their 
> > > > > > > > > > YAML.
> > > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason 
> > > > > > > > > > to repeat that, we could just as easily change 
> > > > > > > > > > `PPIndentWidht` to an optional.
> > > > > > > > > 
> > > > > > > > > We would have to deal with backward compatibility to avoid 
> > > > > > > > > regressions though.
> > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > 
> > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > > 
> > > > > > > > > Of course, if we were to change `PPIndentWidth` to work that 
> > > > > > > > > way too, it would technically be a breaking change because 
> > > > > > > > > users may have *explicitly* specified `-1` in their YAML.
> > > > > > > > 
> > > > > > > > You need an explicit entry, because you need to be able to 
> > > > > > > > write the empty optional on `--dump-config`.
> > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > 
> > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > > 
> > > > > > > > > Of course, if we were to change 

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

In D146101#4261293 , @MyDeveloperDay 
wrote:

> If all comments and concerns are done, then I'm inclined to accept, but I'd 
> like @owenpan  and @HazardyKnusperkeks to give their opinion before we land 
> this.

Sure. Thanks!




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:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > rymiel wrote:
> > > > owenpan wrote:
> > > > > jp4a50 wrote:
> > > > > > HazardyKnusperkeks wrote:
> > > > > > > owenpan wrote:
> > > > > > > > jp4a50 wrote:
> > > > > > > > > HazardyKnusperkeks wrote:
> > > > > > > > > > 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` 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?
> > > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > > 
> > > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > > 
> > > > > > > > > Of course, if we were to change `PPIndentWidth` to work that 
> > > > > > > > > way too, it would technically be a breaking change because 
> > > > > > > > > users may have *explicitly* specified `-1` in their YAML.
> > > > > > > > > And just because `PPIndentWidth` is using -1 is no reason to 
> > > > > > > > > repeat that, we could just as easily change `PPIndentWidht` 
> > > > > > > > > to an optional.
> > > > > > > > 
> > > > > > > > We would have to deal with backward compatibility to avoid 
> > > > > > > > regressions though.
> > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > 
> > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > 
> > > > > > > > Of course, if we were to change `PPIndentWidth` to work that 
> > > > > > > > way too, it would technically be a breaking change because 
> > > > > > > > users may have *explicitly* specified `-1` in their YAML.
> > > > > > > 
> > > > > > > You need an explicit entry, because you need to be able to write 
> > > > > > > the empty optional on `--dump-config`.
> > > > > > > > In YAML we wouldn't need to support empty optional being 
> > > > > > > > *explicitly* specified - it would just be the default.
> > > > > > > > 
> > > > > > > > So specifying `DesignatedInitializerIndentWidth: 4` would set 
> > > > > > > > the `std::optional` to `4` but if 
> > > > > > > > `DesignatedInitializerIndentWidth` was omitted from the YAML 
> > > > > > > > then the optional would simply not be set during parsing.
> > > > > > > > 
> > > > > > > > Of course, if we were to change `PPIndentWidth` to work that 
> > > 

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

If all comments and concerns are done, then I'm inclined to accept, but I'd 
like @owenpan  and @HazardyKnusperkeks to give their opinion before we land 
this.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

Ping for review again plz.


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

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

[sstwcw via Phabricator via cfe-commits]

sstwcw added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:72
 
-  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
-  if napplied == 1:
-return 'List of ' + pluralize(to_yaml_type(subtype))
+  match = re.match(r'std::vector<(.*)>$', typestr)
+  if match:

rymiel wrote:
> jp4a50 wrote:
> > I changed this from `subn` to `match` here since it's just a simpler way of 
> > expressing the same thing.
> (Just FYI, those pythons sources are pretty ancient and untouched, I planned 
> on refactoring the whole thing using more modern, idiomatic Python but then 
> concluded that it's not really necessary)
Is curdeius the one who knows about the Python scripts and the YAML parser?  
Did he move on to meaningful things?


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 marked 29 inline comments as done.
jp4a50 added a comment.

Marked comments as done.


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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Can you mark your comments as done


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 marked 4 inline comments as done.
jp4a50 added a comment.

Just FYI this is ready for review again. I believe I've addressed all comments.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added a comment.

Following on from our discussion about new options needing motivation from 
public style guides, I've updated the KJ style guide to clarify its stance on 
braced init lists: 
https://github.com/capnproto/capnproto/blob/master/style-guide.md#spacing-and-bracing


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.



Comment at: clang/include/clang/Format/Format.h:949
+  /// If unset, ``ContinuationIndentWidth`` is used.
+  /// \code
+  ///   AlignAfterOpenBracket: AlwaysBreak

owenpan wrote:
> HazardyKnusperkeks wrote:
> > jp4a50 wrote:
> > > MyDeveloperDay wrote:
> > > > did you check generating the html from the rst? I can never remember if 
> > > > we need a newline before the \code
> > > Nope - how do I do that exactly? I would guess a newline is not needed 
> > > based on other examples.
> > > did you check generating the html from the rst? I can never remember if 
> > > we need a newline before the \code
> > 
> > 
> > Nope - how do I do that exactly? I would guess a newline is not needed 
> > based on other examples.
> 
> See D147327#4236718.
HTML looks good.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1659
   opensProtoMessageField(Current, Style)) {
+const FormatToken *NextNonComment = Current.getNextNonComment();
 if (Current.opensBlockOrBlockTypeList(Style)) {

HazardyKnusperkeks wrote:
> Why did you move it?
I originally needed it in the new `else if` block since I was checking if it 
was a designated initializer period but the condition has since changed. Thanks 
for spotting. I've moved it back.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1664-1668
+  const auto BracedInitializerIndentWidth =
+  Style.BracedInitializerIndentWidth
+  ? *Style.BracedInitializerIndentWidth
+  : Style.ContinuationIndentWidth;
+  NewIndent = CurrentState.LastSpace + BracedInitializerIndentWidth;

HazardyKnusperkeks wrote:
> You can keep the local variable if you want, but please use `value_or`, it 
> expresses the intent better.
Have used `.value_or()` and got rid of temp variable since it's more concise 
now.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 510886.
jp4a50 added a comment.

Minor review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4855,6 +4855,191 @@
"[5] = ee};");
 }
 
+TEST_F(FormatTest, BracedInitializerIndentWidth) {
+  auto Style = getLLVMStyleWithColumns(60);
+  Style.BinPackArguments = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BracedInitializerIndentWidth = 6;
+
+  // Non-initializing braces are unaffected by BracedInitializerIndentWidth.
+  verifyFormat("enum class {\n"
+   "  One,\n"
+   "  Two,\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  Foo() {}\n"
+   "  void bar();\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("auto foo = [&] {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("{\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  // Non-brace initialization is unaffected by BracedInitializerIndentWidth.
+  verifyFormat("SomeClass clazz(\n"
+   "\"xx\", \"yy\",\n"
+   "\"zz\");\n",
+   Style);
+
+  // The following types of initialization are all affected by
+  // BracedInitializerIndentWidth. Aggregate initialization.
+  verifyFormat("int LongVariable[2] = {\n"
+   "  1000, 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  \"\", \"\",\n"
+   "  \"\"};\n",
+   Style);
+  // Designated initializers.
+  verifyFormat("int LongVariable[1] = {\n"
+   "  [0] = 1000, [1] = 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  .foo = \"x\",\n"
+   "  .bar = \"y\",\n"
+   "  .baz = \"z\"};\n",
+   Style);
+  // List initialization.
+  verifyFormat("SomeStruct s{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("new SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Member initializer.
+  verifyFormat("class SomeClass {\n"
+   "  SomeStruct s{\n"
+   "\"x\",\n"
+   "\"y\",\n"
+   "\"z\",\n"
+   "  };\n"
+   "};\n",
+   Style);
+  // Constructor member initializer.
+  verifyFormat("SomeClass::SomeClass : strct{\n"
+   " \"x\",\n"
+   " \"y\",\n"
+   " \"z\",\n"
+   "   } {}\n",
+   Style);
+  // Copy initialization.
+  verifyFormat("SomeStruct s = SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Copy list initialization.
+  verifyFormat("SomeStruct s = {\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Assignment operand initialization.
+  verifyFormat("s = {\n"
+   "  \"x\",\n"
+   " 

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

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/include/clang/Format/Format.h:949
+  /// If unset, ``ContinuationIndentWidth`` is used.
+  /// \code
+  ///   AlignAfterOpenBracket: AlwaysBreak

HazardyKnusperkeks wrote:
> jp4a50 wrote:
> > MyDeveloperDay wrote:
> > > did you check generating the html from the rst? I can never remember if 
> > > we need a newline before the \code
> > Nope - how do I do that exactly? I would guess a newline is not needed 
> > based on other examples.
> > did you check generating the html from the rst? I can never remember if we 
> > need a newline before the \code
> 
> 
> Nope - how do I do that exactly? I would guess a newline is not needed based 
> on other examples.

See D147327#4236718.


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

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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:949
+  /// If unset, ``ContinuationIndentWidth`` is used.
+  /// \code
+  ///   AlignAfterOpenBracket: AlwaysBreak

jp4a50 wrote:
> MyDeveloperDay wrote:
> > did you check generating the html from the rst? I can never remember if we 
> > need a newline before the \code
> Nope - how do I do that exactly? I would guess a newline is not needed based 
> on other examples.
> did you check generating the html from the rst? I can never remember if we 
> need a newline before the \code





Comment at: clang/lib/Format/ContinuationIndenter.cpp:1659
   opensProtoMessageField(Current, Style)) {
+const FormatToken *NextNonComment = Current.getNextNonComment();
 if (Current.opensBlockOrBlockTypeList(Style)) {

Why did you move it?



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1664-1668
+  const auto BracedInitializerIndentWidth =
+  Style.BracedInitializerIndentWidth
+  ? *Style.BracedInitializerIndentWidth
+  : Style.ContinuationIndentWidth;
+  NewIndent = CurrentState.LastSpace + BracedInitializerIndentWidth;

You can keep the local variable if you want, but please use `value_or`, it 
expresses the intent better.


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

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 510022.
jp4a50 added a comment.

Alphabetical ordering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146101

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/dump_format_style.py
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4855,6 +4855,191 @@
"[5] = ee};");
 }
 
+TEST_F(FormatTest, BracedInitializerIndentWidth) {
+  auto Style = getLLVMStyleWithColumns(60);
+  Style.BinPackArguments = true;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BracedInitializerIndentWidth = 6;
+
+  // Non-initializing braces are unaffected by BracedInitializerIndentWidth.
+  verifyFormat("enum class {\n"
+   "  One,\n"
+   "  Two,\n"
+   "};\n",
+   Style);
+  verifyFormat("class Foo {\n"
+   "  Foo() {}\n"
+   "  void bar();\n"
+   "};\n",
+   Style);
+  verifyFormat("void foo() {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("auto foo = [&] {\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  verifyFormat("{\n"
+   "  auto bar = baz;\n"
+   "  return baz;\n"
+   "};\n",
+   Style);
+  // Non-brace initialization is unaffected by BracedInitializerIndentWidth.
+  verifyFormat("SomeClass clazz(\n"
+   "\"xx\", \"yy\",\n"
+   "\"zz\");\n",
+   Style);
+
+  // The following types of initialization are all affected by
+  // BracedInitializerIndentWidth. Aggregate initialization.
+  verifyFormat("int LongVariable[2] = {\n"
+   "  1000, 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  \"\", \"\",\n"
+   "  \"\"};\n",
+   Style);
+  // Designated initializers.
+  verifyFormat("int LongVariable[1] = {\n"
+   "  [0] = 1000, [1] = 2000};",
+   Style);
+  verifyFormat("SomeStruct s{\n"
+   "  .foo = \"x\",\n"
+   "  .bar = \"y\",\n"
+   "  .baz = \"z\"};\n",
+   Style);
+  // List initialization.
+  verifyFormat("SomeStruct s{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  verifyFormat("new SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Member initializer.
+  verifyFormat("class SomeClass {\n"
+   "  SomeStruct s{\n"
+   "\"x\",\n"
+   "\"y\",\n"
+   "\"z\",\n"
+   "  };\n"
+   "};\n",
+   Style);
+  // Constructor member initializer.
+  verifyFormat("SomeClass::SomeClass : strct{\n"
+   " \"x\",\n"
+   " \"y\",\n"
+   " \"z\",\n"
+   "   } {}\n",
+   Style);
+  // Copy initialization.
+  verifyFormat("SomeStruct s = SomeStruct{\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Copy list initialization.
+  verifyFormat("SomeStruct s = {\n"
+   "  \"x\",\n"
+   "  \"y\",\n"
+   "  \"z\",\n"
+   "};\n",
+   Style);
+  // Assignment operand initialization.
+  verifyFormat("s = {\n"
+   "  \"x\",\n"
+   " 

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

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 added inline comments.



Comment at: clang/include/clang/Format/Format.h:949
+  /// If unset, ``ContinuationIndentWidth`` is used.
+  /// \code
+  ///   AlignAfterOpenBracket: AlwaysBreak

MyDeveloperDay wrote:
> did you check generating the html from the rst? I can never remember if we 
> need a newline before the \code
Nope - how do I do that exactly? I would guess a newline is not needed based on 
other examples.


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

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

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:72
 
-  subtype, napplied = re.subn(r'^std::vector<(.*)>$', r'\1', typestr)
-  if napplied == 1:
-return 'List of ' + pluralize(to_yaml_type(subtype))
+  match = re.match(r'std::vector<(.*)>$', typestr)
+  if match:

jp4a50 wrote:
> I changed this from `subn` to `match` here since it's just a simpler way of 
> expressing the same thing.
(Just FYI, those pythons sources are pretty ancient and untouched, I planned on 
refactoring the whole thing using more modern, idiomatic Python but then 
concluded that it's not really necessary)


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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:949
+  /// If unset, ``ContinuationIndentWidth`` is used.
+  /// \code
+  ///   AlignAfterOpenBracket: AlwaysBreak

did you check generating the html from the rst? I can never remember if we need 
a newline before the \code



Comment at: clang/include/clang/Format/Format.h:4289
DerivePointerAlignment == R.DerivePointerAlignment &&
+   BracedInitializerIndentWidth == R.BracedInitializerIndentWidth &&
DisableFormat == R.DisableFormat &&

can this be alphabetic



Comment at: clang/lib/Format/Format.cpp:905
 IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment);
+IO.mapOptional("BracedInitializerIndentWidth",
+   Style.BracedInitializerIndentWidth);

also alphabetic



Comment at: clang/lib/Format/Format.cpp:1373
   LLVMStyle.DerivePointerAlignment = false;
+  LLVMStyle.BracedInitializerIndentWidth = std::nullopt;
   LLVMStyle.DisableFormat = false;

ditto alphabetic


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