[PATCH] D40310: Restructure how we break tokens.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: lib/Format/BreakableToken.cpp:198
+         "Getting the length of a part of the string literal indicates that "
+         "the code tries to reflow it.");
+  return UnbreakableTailLength + Postfix.size() +
----------------
klimek wrote:
> krasimir wrote:
> > How about clients that explicitly pass `Length = Line.size() - Offset`?
> That is different (I now also went and updated the comment for getRangeLength to explain that).
> 
> Generally, Length == Line.size() - Offset is still a portion of the content, as opposed to npos, which has a special meaning. I'm wondering whether I should just pull out a differently named method for it, now that I'm thinking about it.
Yes please!


================
Comment at: lib/Format/ContinuationIndenter.cpp:1504
                                  : Style.PenaltyBreakComment;
-  unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength;
+  // Stores whether we introduce a break anywhere in the token.
   bool BreakInserted = Token->introducesBreakBeforeToken();
----------------
klimek wrote:
> krasimir wrote:
> > Does a reflow count as a break?
> I do believe so (well, the break in the reflow counts, the reflow itself is not a break, but removing a break :)
So, if we add a break then remove a break while reflowing, will `BreakInserted` be true or false?


================
Comment at: unittests/Format/FormatTestComments.cpp:2149
+  // to keep this?
+  EXPECT_EQ("// some text\n"
+            "// that\n"
----------------
klimek wrote:
> krasimir wrote:
> > This is like this for cases like lists in comments:
> > ```
> > blah-blah-blah:
> >   1. blah
> >   2. blah-blah
> > ```
> > I think here the block comments behavior might be wrong.
> Note that on the doc I shared you voted the reverse ;)
Then I should consider re-voting :)


https://reviews.llvm.org/D40310