Typz added inline comments.
================ Comment at: lib/Format/ContinuationIndenter.cpp:1339 +unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, + LineState &State, ---------------- djasper wrote: > Can you create a patch that doesn't move the code around so much? Seems > unnecessary and hard to review. Moving the code around is an unfortunate requirement for this patch: we must actually do the "reflowing" twice, to find out which solution (actually reflowing or not) gives the least penalty. Therefore the function that reflowing must be moved out of `breakProtrudingToken()`... All I can do is try moving the new function after `breakProtrudingToken()`, maybe Phabricator will better show the change. ================ Comment at: lib/Format/ContinuationIndenter.cpp:1446 + // Do not count the penalty twice, it will be added afterwards + if (State.Column > getColumnLimit(State)) { + unsigned ExcessCharacters = State.Column - getColumnLimit(State); ---------------- djasper wrote: > I believe that this is incorrect. reflowProtrudingToken counts the length of > the unbreakable tail and here you just remove the penalty of the token > itself. E.g. in: > > string s = f("aaa"); > > the ");" is the unbreakable tail of the stringl This behavior has not changed : before the commit, the last token was not included in the penalty [c.f. `if` at line 1338 in original code]. To make the comparison significative, the last token's penalty is included in the penalty returned by `reflowProtrudingToken()` (hence the removal of the test at line 1260); and it is removed before returning from this function, to keep the same behavior as before. https://reviews.llvm.org/D33589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits