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

Daniel Jasper djasper at google.com
Thu Aug 22 04:30:13 PDT 2013



================
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)) {
----------------
Manuel Klimek wrote:
> Daniel Jasper wrote:
> > Manuel Klimek wrote:
> > > 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.
> > What's the benefit? We already need to know that we are dealing with a CommaSeparatedList in order to create it appropriately..
> > 
> > I agree that there might be more generic concepts once we have more Roles and understand them.
> I think it's pretty predictable how Role will turn out, as it's a common pattern. 
> 
> It would help in that we could get rid of all the RTTI code?
Cool.. Predict away, because I have no clue ..

Right, we can get rid of the RTTI code (I was unaware that everything else by now is clean .. this has gone through quite a few iterations). However, do you think something like "CommaFound" is good to have as a general method in Role? I am afraid that all the 'events' will be specific to one specific Role each ...

================
Comment at: lib/Format/FormatToken.cpp:109
@@ +108,3 @@
+      ItemEnd = Commas[i];
+      ItemLengths.push_back(ItemEnd->TotalLength - ItemBegin->TotalLength +
+                            ItemBegin->CodePointCount);
----------------
Manuel Klimek wrote:
> I'd add a comment:
> // The comma is counted as part of the item when calculating the length.
Done (also in the descriptions of ItemLengths).

================
Comment at: lib/Format/FormatToken.cpp:76
@@ +75,3 @@
+  FormatToken *ItemBegin = Token->Next;
+  SmallVector<bool, 6> MustBreakBeforeItem;
+  SmallVector<unsigned, 6> ItemLengthsInLastColumn;
----------------
Manuel Klimek wrote:
> Daniel Jasper wrote:
> > Manuel Klimek wrote:
> > > Has the magic number '6' any meaning?
> > No. Any hint on how to pick these?
> Well, usually people seem to take the closest power of two that makes sense if they have no clue :P "6" thus sounds like you put thought into it.
I haven't. Replaced with 8 to avoid confusion.

================
Comment at: lib/Format/FormatToken.cpp:116-118
@@ +115,5 @@
+    }
+    EndOfLineItemLength.push_back(ItemEnd->TotalLength -
+                                  ItemBegin->TotalLength +
+                                  ItemBegin->CodePointCount);
+    ItemBegin = ItemEnd->Next;
----------------
Manuel Klimek wrote:
> I think if we pull out a method for this calculation the code will read nicer...
Done.


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



More information about the cfe-commits mailing list