krasimir marked 3 inline comments as done.
krasimir added inline comments.

================
Comment at: lib/Format/BreakableToken.h:55-56
+///   been reformatted, and
+/// - replaceWhitespaceBefore, for executing the reflow using a whitespace
+///   manager.
+///
----------------
klimek wrote:
> Shouldn't that be called insertBreakBefore for consistency then?
> 
> Also, perhaps we want to rename replaceWhitespace to cleanupWhitespace or 
> compressWhitespace or something...
insertBreakBefore will not be a good name for this, because it doesn't insert 
breaks.
There are two use-cases for this:
1. The old one: adjusting indent in comments, for example:
```
// before
   // after
```
2. The new one: reflowing, for example
```
before:
  // line1
  // line2
after:
  // line1 line2
```
The break between line1 and line2 gets removed in replaceWhitespaceBefore(2).

I'll rename replaceWhitespace to compressWhitespace.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1158-1159
+        CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
+        Current.TokenText.substr(2).ltrim().startswith("clang-format on") ||
+        Current.TokenText.substr(2).ltrim().startswith("clang-format off"))
       return addMultilineToken(Current, State);
----------------
klimek wrote:
> Generally, we shouldn't need those here, as those should be part of the 
> Token.Finalized state.
Here the problem is that the comments /* clang-format on */ and /* clang-format 
off */ control the formatting of the tokens in between, but do not control 
their own formatting, that is they are not finalized themselves.
What happens is that if we rely solely on Token.Finalized and the ColumnLimit 
is small, say 20, then:
```
/* clang-format off */ 
```
gets broken into:
```
/* clang-format off
  */
```

Add comments about this.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1220
+        RemainingTokenColumns = RemainingTokenColumns + 1 - Split.second;
+        ReflowInProgress = true;
         if (!DryRun)
----------------
klimek wrote:
> Explain this part in a comment.
Realized that the place of this is not here; introduced a proper method 
BreakableToken::getLineLengthAfterCompres instead.


https://reviews.llvm.org/D28764



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to