[PATCH] clang-format: Add column layout formatting for braced lists

[Manuel Klimek]

================
Comment at: lib/Format/TokenAnnotator.cpp:275
@@ -271,1 +274,3 @@
+        Left->Role.reset(new CommaSeparatedList(Style));
+      Left->getRoleAs<CommaSeparatedList>()->CommaFound(Current);
     } else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
----------------
I'd much rather have an interface for Role that contains all the "events" that can happen that are significant to roles, and use virtual methods.

================
Comment at: lib/Format/FormatToken.cpp:49-67
@@ +48,21 @@
+    unsigned Spaces = State.NextToken->SpacesRequiredBefore;
+    if (Item < Commas.size() && State.NextToken->Previous == Commas[Item]) {
+      if (!State.NextToken->isTrailingComment()) {
+        Spaces += Format->ColumnSizes[Column] - ItemLengths[Item];
+        ++Column;
+      }
+      ++Item;
+      if (Column == Format->Columns) {
+        Column = 0;
+        NewLine = true;
+      }
+    }
+    if (State.NextToken->MustBreakBefore) {
+      Column = 0;
+      NewLine = true;
+    }
+    if (NewLine)
+      Penalty += State.NextToken->SplitPenalty;
+    Spaces -= State.NextToken->SpacesRequiredBefore;
+    Penalty += Indenter->addTokenToState(State, NewLine, DryRun, Spaces);
+  }
----------------
All this needs more comments on how this works...

================
Comment at: lib/Format/FormatToken.cpp:76
@@ +75,3 @@
+  FormatToken *ItemBegin = Token->Next;
+  SmallVector<bool, 6> MustBreakBeforeItem;
+  SmallVector<unsigned, 6> ItemLengthsInLastColumn;
----------------
Has the magic number '6' any meaning?

================
Comment at: lib/Format/FormatToken.cpp:85
@@ +84,3 @@
+    const FormatToken *ItemEnd =
+        i == Commas.size() ? Token->MatchingParen->getPreviousNonComment()
+                           : Commas[i];
----------------
At a first glance this seems inbalanced: wouldn't ItemEnd be one to early in for the last item?

================
Comment at: lib/Format/FormatToken.cpp:87-88
@@ +86,4 @@
+                           : Commas[i];
+    ItemLengths.push_back(ItemEnd->TotalLength - ItemBegin->TotalLength +
+                          ItemBegin->CodePointCount);
+    if (ItemEnd->Next && !ItemEnd->Next->HasUnescapedNewline &&
----------------
This needs comments - why does subtracting two different token's length give anything useful?

================
Comment at: lib/Format/FormatToken.cpp:105
@@ +104,3 @@
+    bool HasRowWithSufficientColumns = false;
+    for (unsigned i = 0, e = ItemLengths.size(), Column = 0; i != e; ++i) {
+      if (MustBreakBeforeItem[i] || Column == Columns) {
----------------
Pulling out Columns would make this line idiomatic and easier to read for me.

================
Comment at: lib/Format/FormatToken.h:379
@@ +378,3 @@
+    /// \brief The total length in number of lines.
+    unsigned TotalLength;
+
----------------
I'd probably call that LineCount or something else with Line in it then...

================
Comment at: lib/Format/FormatToken.cpp:39
@@ +38,3 @@
+    return 0;
+  unsigned RemainingColumns = Style.ColumnLimit - State.Stack.back().Indent;
+  const ColumnFormat *Format = getColumnFormat(RemainingColumns);
----------------
So 'RemainingColumns' is in characters, but 'Column' below is in "entries"?


http://llvm-reviews.chandlerc.com/D1457