[PATCH] D27754: [clang-format] Implement comment reflowing (again).

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.


================
Comment at: lib/Format/BreakableToken.cpp:471
+                                               WhitespaceManager &Whitespaces) {
+  if (Tok.is(TT_LineComment)) {
+    // If this is the first line of a token, inform Whitespace Manager about it.
----------------
klimek wrote:
> Without looking into this in a lot of detail: this looks like you want a BreakableComment base class, and have BreakableBlockComment and BreakableLineCommentSection derive from it and implement this method.
> 
> Scanning it a bit, it seems like there is still overlap - perhaps it's also possible to pull out a couple of smaller sized methods in the interface and write the algorithm in terms of those? That could also make it easier to understand in general (large method alarm ;)
Next step I'm doing: will extract two subclasses and try to factor out the common functionality. I already did a bit of that with getReflowSplit, but there's more potential for extracting common stuff.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1174-1175
        LineIndex != EndIndex; ++LineIndex) {
-    if (!DryRun)
-      Token->replaceWhitespaceBefore(LineIndex, Whitespaces);
+    Token->replaceWhitespaceBefore(LineIndex, RemainingTokenColumns,
+                                   RemainingSpace, DryRun, Whitespaces);
     unsigned TailOffset = 0;
----------------
klimek wrote:
> Nice that this whole section required so few changes.
> Why do we need to call into this in DryRun mode now, though? Does it need to keep state inside in DryRun?
Yes, replaceWhitespaceBefore recomputes the ContentColumn in case a reflow with the previous line is decided to be made.
Basically, the whole reflow functionality is inside replaceWhitespaceBefore now (except for a bit of control flow stuff, like updating the  ReflowInProgress member variable).


https://reviews.llvm.org/D27754