[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: lib/Format/BreakableToken.cpp:178
                                               Split Split) const {
   // Example: consider the content
   // lala  lala
----------------
Offtopic: Should add a FIXME that this doesn't really work in the presence of tabs.


================
Comment at: lib/Format/BreakableToken.h:154
   /// \p Split has been compressed into a single space.
   unsigned getLineLengthAfterCompression(unsigned RemainingTokenColumns,
                                          Split Split) const;
----------------
nit: the comment doesn't make sense anymore.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1625
+          if (ExcessCharactersPenalty < NewBreakPenalty) {
+            ContinueOnLine = true;
+          }
----------------
nit: no braces around single-statement if body


================
Comment at: lib/Format/ContinuationIndenter.cpp:1707
+          RemainingTokenColumns = Token->getRemainingLength(
+              NextLineIndex, TailOffset, ContentStartColumn);
+          Reflow = true;
----------------
When we're reflowing we replace the reflow split with a space, so IMO this should be:
```
RemainingTokenColumns = Token->getRemainingLength(
              NextLineIndex, TailOffset, ContentStartColumn + 1);
```


================
Comment at: lib/Format/ContinuationIndenter.cpp:1709
+          Reflow = true;
+          if (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+            DEBUG(llvm::dbgs() << "    Over limit after reflow, need: "
----------------
IMO this should be `ContentStartColumn + RemainingTokenColumns + 1`, accounting for the space that replaces the reflow split.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1721
+                Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+                                ContentStartColumn, CommentPragmasRegex);
+            if (Split.first == StringRef::npos) {
----------------
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).


https://reviews.llvm.org/D40310