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

[MyDeveloperDay via Phabricator via cfe-commits]

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(AllowAllParametersOfDeclarationOnNextLine);
+  CHECK_PARSE_BOOL(AlignCompoundAssignments);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
----------------
alphabetic, but see my note as you shouldn't need this?


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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119599/new/

https://reviews.llvm.org/D119599