================
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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
