[PATCH] D40310: Restructure how we break tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1707
+          RemainingTokenColumns = Token->getRemainingLength(
+              NextLineIndex, TailOffset, ContentStartColumn);
+          Reflow = true;
----------------
krasimir wrote:
> When we're reflowing we replace the reflow split with a space, so IMO this should be:
> ```
> RemainingTokenColumns = Token->getRemainingLength(
>               NextLineIndex, TailOffset, ContentStartColumn + 1);
> ```
Actually, ContentStartColumn is just calculated incorrectly above. Added tests, and added the +1 above, which makes it +1 for all code below.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1721
+                Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+                                ContentStartColumn, CommentPragmasRegex);
+            if (Split.first == StringRef::npos) {
----------------
krasimir wrote:
> Looks like `ContentStartColumn` is consistently used as the start column of the reflown content on the next line.
> I suggest to `++ContentStartColumn` at the beginning of the body of this if statement (and adjust below appropriately).
Yep, that's what I also figured out - that also led to removing ++ContentStartColumn in the reflow case below, which was what made this work at all.
Added tests to ReflowsCommentsPrecise, which flow through all of the corner cases of the if's here.


https://reviews.llvm.org/D40310