[PATCH] D14325: [clang-format] Do not align assignments that aren't after the same number of commas. (Closes: 25329)

[Beren Minor via cfe-commits]

berenm marked an inline comment as done.

================
Comment at: lib/Format/WhitespaceManager.cpp:150-151
@@ -149,109 +149,4 @@
 }
 
-// Walk through all of the changes and find sequences of "=" to align.  To do
-// so, keep track of the lines and whether or not an "=" was found on align. If
-// a "=" is found on a line, extend the current sequence. If the current line
-// cannot be part of a sequence, e.g. because there is an empty line before it
-// or it contains non-assignments, finalize the previous sequence.
-//
-// FIXME: The code between assignment and declaration alignment is mostly
-// duplicated and would benefit from factorization.
-void WhitespaceManager::alignConsecutiveAssignments() {
-  if (!Style.AlignConsecutiveAssignments)
-    return;
-
-  unsigned MinColumn = 0;
-  unsigned MaxColumn = UINT_MAX;
-  unsigned StartOfSequence = 0;
-  unsigned EndOfSequence = 0;
-  bool FoundAssignmentOnLine = false;
-  bool FoundLeftBraceOnLine = false;
-  bool FoundLeftParenOnLine = false;
-
-  // Aligns a sequence of assignment tokens, on the MinColumn column.
-  //
-  // Sequences start from the first assignment token to align, and end at the
-  // first token of the first line that doesn't need to be aligned.
-  //
-  // We need to adjust the StartOfTokenColumn of each Change that is on a line
-  // containing any assignment to be aligned and located after such assignment
-  auto AlignSequence = [&] {
-    if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
-      alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
-    MinColumn = 0;
-    MaxColumn = UINT_MAX;
-    StartOfSequence = 0;
-    EndOfSequence = 0;
-  };
-
-  for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-    if (Changes[i].NewlinesBefore != 0) {
-      EndOfSequence = i;
-      // If there is a blank line, if the last line didn't contain any
-      // assignment, or if we found an open brace or paren, the sequence ends
-      // here.
-      if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine ||
-          FoundLeftBraceOnLine || FoundLeftParenOnLine) {
-        // NB: In the latter case, the sequence should end at the beggining of
-        // the previous line, but it doesn't really matter as there is no
-        // assignment on it
-        AlignSequence();
-      }
-
-      FoundAssignmentOnLine = false;
-      FoundLeftBraceOnLine = false;
-      FoundLeftParenOnLine = false;
-    }
-
-    // If there is more than one "=" per line, or if the "=" appears first on
-    // the line of if it appears last, end the sequence
-    if (Changes[i].Kind == tok::equal &&
-        (FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
-         Changes[i + 1].NewlinesBefore > 0)) {
-      AlignSequence();
-    } else if (Changes[i].Kind == tok::r_brace) {
-      if (!FoundLeftBraceOnLine)
-        AlignSequence();
-      FoundLeftBraceOnLine = false;
-    } else if (Changes[i].Kind == tok::l_brace) {
-      FoundLeftBraceOnLine = true;
-      if (!FoundAssignmentOnLine)
-        AlignSequence();
-    } else if (Changes[i].Kind == tok::r_paren) {
-      if (!FoundLeftParenOnLine)
-        AlignSequence();
-      FoundLeftParenOnLine = false;
-    } else if (Changes[i].Kind == tok::l_paren) {
-      FoundLeftParenOnLine = true;
-      if (!FoundAssignmentOnLine)
-        AlignSequence();
-    } else if (!FoundAssignmentOnLine && !FoundLeftBraceOnLine &&
-               !FoundLeftParenOnLine && Changes[i].Kind == tok::equal) {
-      FoundAssignmentOnLine = true;
-      if (StartOfSequence == 0)
-        StartOfSequence = i;
-
-      unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-      int LineLengthAfter = -Changes[i].Spaces;
-      for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j)
-        LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
-      unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
-
-      if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) {
-        AlignSequence();
-        StartOfSequence = i;
-      }
-
-      MinColumn = std::max(MinColumn, ChangeMinColumn);
-      MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
-    }
-  }
-
-  EndOfSequence = Changes.size();
-  AlignSequence();
-}
-
-void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
-                                                    unsigned End,
-                                                    unsigned Column) {
-  bool FoundAssignmentOnLine = false;
+// Align a single sequence of tokens, see AlignTokens below.
+template <typename F>
----------------
Haha, good question. I initially moved the variable there, but then it didn't looked really better, so I moved the variables back into the function...


http://reviews.llvm.org/D14325