[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.


================
Comment at: include/clang/Format/Format.h:699
+  /// This option is **deprecated* and is retained for backwards compatibility.
   bool BreakConstructorInitializersBeforeComma;
 
----------------
I don't think you need to keep this around. The YAML parsing logic can correctly set the new field instead. So basically just call mapOptional for both names of configuration fields but always set BreakConstructorInitializers. And then map true/false to the appropriate enum values.


================
Comment at: include/clang/Format/Format.h:710
+    /// \endcode
+    BCIS_BeforeColonAfterComma,
+    /// Break constructor initializers before the colon and commas, and align
----------------
Call this just "BeforeColon".


================
Comment at: include/clang/Format/Format.h:718
+    /// \endcode
+    BCIS_BeforeColonAndComma,
+    /// Break constructor initializers after the colon and commas.
----------------
Call this just "BeforeComma".


================
Comment at: include/clang/Format/Format.h:725
+    /// \endcode
+    BCIS_AfterColonAndComma
+  };
----------------
Call this just "AfterColon".


================
Comment at: lib/Format/ContinuationIndenter.cpp:61
          ((Previous.isNot(TT_CtorInitializerComma) ||
-          !Style.BreakConstructorInitializersBeforeComma) &&
+           (Style.BreakConstructorInitializers !=
+                FormatStyle::BCIS_BeforeColonAndComma)) &&
----------------
You don't need parentheses to surround comparisons. Remove them here and elsewhere.


================
Comment at: lib/Format/ContinuationIndenter.cpp:196
+           FormatStyle::BCIS_AfterColonAndComma) &&
+      (State.Column + State.Line->Last->TotalLength - Previous.TotalLength >
+           getColumnLimit(State) ||
----------------
Why can you drop the "+2" here?

Also, I'd like to structure this so we have to duplicate less of the logic. But I am not really sure it's possible.


https://reviews.llvm.org/D32479