[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99553.
Typz added a comment.

Deprecate BreakConstructorInitializersBeforeComma and replace it with a more 
generic BreakConstructorInitializers option.


https://reviews.llvm.org/D32479

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -56,16 +56,17 @@
 return *Result;
   }
 
-  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
-FormatStyle Style = getLLVMStyle();
+  FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
 Style.ColumnLimit = ColumnLimit;
 return Style;
   }
 
+  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
+return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
+  }
+
   FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) {
-FormatStyle Style = getGoogleStyle();
-Style.ColumnLimit = ColumnLimit;
-return Style;
+return getStyleWithColumns(getGoogleStyle(), ColumnLimit);
   }
 
   void verifyFormat(llvm::StringRef Code,
@@ -2699,6 +2700,128 @@
"() {}"));
 }
 
+TEST_F(FormatTest, BreakConstructorInitializersAfterColonAndComma) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColonAndComma;
+
+  verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
+  verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 45));
+  verifyFormat("Constructor() :\n"
+   "Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 44));
+  verifyFormat("Constructor() :\n"
+   "Inttializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 43));
+
+  verifyFormat("template \n"
+   "Constructor() : Initializer(FitsOnTheLine) {}",
+   getStyleWithColumns(Style, 50));
+
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "a(aa), aaa() {}",
+	  Style);
+
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "a(aa), a(aa),\n"
+  "a(aa) {}",
+	  Style);
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "aa(aa),\n"
+  "aaa() {}",
+	  Style);
+  verifyFormat("Constructor(aa ,\n"
+   "aa ) :\n"
+   "aa(aa) {}",
+			   Style);
+
+  verifyFormat("Constructor() :\n"
+   "(aaa),\n"
+   "(aaa,\n"
+   " aaa),\n"
+   "aaa() {}",
+			   Style);
+
+  verifyFormat("Constructor() :\n"
+   "(\n"
+   "a) {}",
+			   Style);
+
+  verifyFormat("Constructor(int Parameter = 0) :\n"
+   "aa(a),\n"
+   "(a) {}",
+			   Style);
+  verifyFormat("Constructor() :\n"
+   "aa(a), (b) {\n"
+   "}",
+   getStyleWithColumns(Style, 60));
+  verifyFormat("Constructor() :\n"
+   "a(\n"
+   "a(, )) {}",
+			   Style);
+
+  // Here a line could be saved by splitting the second initializer onto two
+  // lines, but that is not desirable.
+  verifyFormat("Constructor() :\n"
+   "(),\n"
+   "aaa(aaa),\n"
+   "at() {}",
+			   Style);
+
+  FormatStyle OnePerLine = Style;
+  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false;
+  verifyFormat("SomeClass::Constructor() :\n"
+   "a(aa),\n"
+   "a(aa),\n"
+   "a(aa) {}",
+   OnePerLine);
+  verifyFormat("SomeClass::Constructor() :\n"
+   "a(aa), // Some comment\n"
+   "a(aa),\n"
+   "

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-17 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment.

Yes, turning that option into an enum seems like the better choice here.


https://reviews.llvm.org/D32479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

Or would it be better to replace (i.e. deprecate) the 
BreakConstructorInitializersBeforeComma option with a multiple choice option, 
as suggested here:
http://clang-developers.42468.n3.nabble.com/clang-format-Proposal-for-a-new-style-for-constructor-and-initializers-line-break-td4041595.html

  /// \brief Different ways to break initializers.
  enum BreakConstructorInitializersStyle
  {
/// Constructor()
/// : initializer1(),
///   initializer2()
BCIS_BeforeColonAfterComma,
/// Constructor()
/// : initializer1()
/// , initializer2()
BCIS_BeforeColonAndComma,
/// Constructor() :
/// initializer1(),
/// initializer2()
BCIS_AfterColonAndComma
  };
  BreakConstructorInitializersStyle BreakConstructorInitializers


https://reviews.llvm.org/D32479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment.

ping?


https://reviews.llvm.org/D32479



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits