[PATCH] D119599: Add option to align compound assignments like `+=`

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 12 02:05:09 PST 2022

MyDeveloperDay added inline comments.

Comment at: clang/lib/Format/WhitespaceManager.cpp:468
+  // Widths of the aligned text.
+  // The part to the left of the anchor. For right-justified assignment
+  // operators, this includes the initial space before the sign but not
whats an anchor?

Comment at: clang/lib/Format/WhitespaceManager.cpp:580
-    unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-    int LineLengthAfter = Changes[i].TokenLength;
+    unsigned ChangeWidthLeft = Changes[i].StartOfTokenColumn;
+    unsigned ChangeWidthAnchor = 0;
I like the final outcome, I'm uneasy about renaming all the variables, just because you now understand them. I'm struggling to read the algorithm in the same context as the prior version

Comment at: clang/unittests/Format/FormatTest.cpp:16399
+  Alignment.AlignCompoundAssignments = true;
+  verifyFormat("aa <= 5;\n"
+               "a               &= 5;\n"
curdeius wrote:
> Please test spacing in separate `verifyFormat` cases as in the Format.h option description.
I sort of feel this isn't enough tests

Comment at: clang/unittests/Format/FormatTest.cpp:16852
-  // otherwise the function parameters will be re-flowed onto a single line.
-  Alignment.ColumnLimit = 0;
   EXPECT_EQ("int    a(int   x,\n"
Huge `no` from me, don't change the tests because they break based on your change.

Comment at: clang/unittests/Format/FormatTest.cpp:19273
+  CHECK_PARSE_BOOL(AlignCompoundAssignments);
alphabetic, but see my note as you shouldn't need this?

Comment at: clang/unittests/Format/FormatTest.cpp:19277
+  CHECK_PARSE_BOOL(AlignTrailingComments);
`IJKL` last L looked i came before l right? so why did you move this down

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list