[PATCH] clang-format: Add ability to align assignment operators

Daniel Jasper djasper at google.com
Wed Apr 8 22:46:46 PDT 2015


================
Comment at: include/clang/Format/Format.h:250
@@ -249,1 +249,3 @@
 
+  /// \brief If \c true, aligns variable assignments.
+  ///
----------------
Variable assignments sounds a bit vague. Is this for field assignments, too? Is this only meant to be in declarations or in all assignments? Does this only apply to "=" or also to the other assignment operators "+=", ...? I can answer these questions from the code, but a bit more detail might not hurt here and I am not yet sure the name AlignAssignmentOperators is ideal. Maybe AlignConsecutiveAssignments?

================
Comment at: include/clang/Format/Format.h:255
@@ +254,3 @@
+  /// \code
+  /// int aaaa       = 12;
+  /// int b          = 23;
----------------
Please put the example as it would actually be formatted. There is too much whitespace here.

================
Comment at: lib/Format/WhitespaceManager.cpp:267
@@ -265,1 +266,3 @@
 
+void WhitespaceManager::alignAssignmentOperators() {
+  if (!Style.AlignAssignmentOperators)
----------------
The multi-line behavior of this (i.e. when a single declaration spans more than one line) is really interesting. Yet, there are no tests for it.

================
Comment at: lib/Format/WhitespaceManager.cpp:315
@@ +314,3 @@
+    int Shift = 0;
+    if (Changes[i].NewlinesBefore > 0) {
+      AlignedEquals = false;
----------------
In LLVM style, we usually don't use {} for single-statement ifs.

================
Comment at: unittests/Format/FormatTest.cpp:8350
@@ -8349,1 +8349,3 @@
 
+TEST_F(FormatTest, AlignAssignmentOperators) {
+  FormatStyle Alignment = getLLVMStyle();
----------------
Just thought of one more thing: Default parameters.

What happens to:

  void SomeFunction(int parameter = 0) {
    int i = 1;
  }

Similarly, would we want to align:

  void SomeFunction(int parameter = 1,
                    int i =         2) { ...

? (The latter should definitely be another patch).

================
Comment at: unittests/Format/FormatTest.cpp:8355
@@ +8354,3 @@
+               "int oneTwoThree = 123;", Alignment);
+  EXPECT_EQ("int a = 5;\n"
+            "int oneTwoThree = 123;",
----------------
I still think verifyFormat would be preferable for many of those, especially all those not testing the behavior around empty lines.

================
Comment at: unittests/Format/FormatTest.cpp:8393
@@ +8392,3 @@
+            "int one = 1;\n"
+            "method();\n"
+            "int oneTwoThree = 123;\n"
----------------
Pure virtual methods might also be interesting, i.e.:

  class C {
  public:
    int i = 1;
    virtual void f() = 0;
  };

http://reviews.llvm.org/D8821

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list