================
Comment at: lib/Format/Format.cpp:1252
@@ +1251,3 @@
+ assert(!UnwrappedLines.empty());
+ UnwrappedLines.rbegin()->push_back(TheLine);
+ }
----------------
Daniel Jasper wrote:
> I would use back(), but probably irrelevant ..
Done.
================
Comment at: lib/Format/FormatToken.h:78
@@ -77,1 +77,3 @@
+enum FormatState {
+ FS_Unformatted,
----------------
Daniel Jasper wrote:
> I don't like this name as it is too closely related to LineState and
> ParenState. We'll eventually confuse LineState (which actually is a
> FormatState with this). Maybe FormatDecision?
Done.
================
Comment at: lib/Format/FormatToken.h:350
@@ +349,3 @@
+
+ /// \brief If true, this token has been fully formatted (indented and
+ /// potentially re-formatted inside), and we do not allow further formatting
----------------
Daniel Jasper wrote:
> nit: If \c true, ...
Done.
================
Comment at: lib/Format/FormatToken.h:347
@@ -339,1 +346,3 @@
+ /// \brief Stores the formatting decision for the token once it was made.
+ FormatState State;
----------------
Daniel Jasper wrote:
> Ha, so you agree that it is a decision (didn't see this before making the
> name suggestion above) :-)
Ack.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:194
@@ +193,3 @@
+void UnwrappedLineParser::reset() {
+ PPBranchLevel = -1;
+ Line.reset(new UnwrappedLine);
----------------
Daniel Jasper wrote:
> Seems weird to initialize this to 0 in the constructor and to -1 here
Done.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:223
@@ +222,3 @@
+ I != E; ++I) {
+ Callback.consumeUnwrappedLine(*I);
+ }
----------------
Daniel Jasper wrote:
> This seems like a really weird interface now (iterating over an array, adding
> each line and then calling finish). Would it make sense to change to a single
> method call where we can pass in an array? Then on the other side you could
> save the logic to add an empty array for the next set of lines in
> finishRun(), which is not really intuitive..
I agree, but I'd like to change that in a follow-up patch. I think we might
just want to return the unwrapped lines here, as they'll now get copied anyway.
================
Comment at: lib/Format/UnwrappedLineParser.cpp:228
@@ +227,3 @@
+ while (!PPLevelBranchIndex.empty() &&
+ *PPLevelBranchIndex.rbegin() + 1 == *PPLevelBranchCount.rbegin()) {
+ PPLevelBranchIndex.resize(PPLevelBranchIndex.size() - 1);
----------------
Daniel Jasper wrote:
> Use PPLevelBranchIndex.back() here and below.
Done.
http://llvm-reviews.chandlerc.com/D1883
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
