klimek added inline comments.
================
Comment at: lib/Format/BreakableToken.cpp:279-280
+ return Content.size() >= 2 &&
+ Content != "clang-format on" &&
+ Content != "clang-format off" &&
+ !Content.endswith("\\") &&
----------------
Can we now use delimitsRegion here?
================
Comment at: lib/Format/ContinuationIndenter.cpp:1090-1092
+// Checks if Token delimits formatting regions, like /* clang-format off */.
+// Token must be a comment.
+static bool delimitsRegion(const FormatToken &Token) {
----------------
delimitsRegion seems overly abstract. Perhaps switchesFormatting?
================
Comment at: lib/Format/ContinuationIndenter.cpp:1205-1210
+ if (SplitBefore.first != StringRef::npos) {
+ TailOffset = SplitBefore.first + SplitBefore.second;
+ ReflowInProgress = true;
+ } else {
+ ReflowInProgress = false;
+ }
----------------
krasimir wrote:
> klimek wrote:
> > (optional) I'd probably write this:
> > ReflowInProgress = SplitBefore.first != StringRef::npos;
> > if (ReflowInProgress) {
> > TailOffset = SplitBefore.first + SplitBefore.second;
> > }
> How about this?
That works.
================
Comment at: lib/Format/ContinuationIndenter.cpp:1231-1233
+ unsigned RemainingTokenColumnsAfterCompress =
+ Token->getLineLengthAfterCompress(LineIndex, TailOffset,
+ RemainingTokenColumns, Split);
----------------
krasimir wrote:
> klimek wrote:
> > I think AfterCompression reads more naturally. Perhaps we should use "trim"
> > instead of compress, now that I think about it.
> > So I'd probably go for getTrimmedLineLength or something.
> I don't agree with "trimmig" because i have a connotation of "trimming"
> around the ends of strings like in ltrim/rtrim and this fundamentally
> operates somewhere inside. +1 for AfterCompression, though.
Yea, correct, I completely missed that this might happen in the middle of the
string if we happen to be the first token that runs over, which is a rather
interesting case :)
================
Comment at: lib/Format/WhitespaceManager.cpp:131
+ Changes[i - 1].Kind == tok::comment &&
+ OriginalWhitespaceStart != PreviousOriginalWhitespaceEnd;
}
----------------
krasimir wrote:
> klimek wrote:
> > Why was this needed? (what breaks without this change)
> This is a dirty hack. The problem is that WhitespaceManager does its own
> formatting for trailing comments and sometimes it re-formats a piece of line
> even after it's been reflown with the previous line. Consider if we reflow
> the second line up:
> ```
> // line 1
> // line 2
> ```
> That amounts to 2 changes, first (delimited by ()) for the whitespace between
> the two tokens, and second (delimited by []) for the beginning of the next
> line:
> ```
> // line 1(
> )[// ]line 2
> ```
> So in the end we have two changes like this:
> ```
> // line 1()[ ]line 2
> ```
> Note that the OriginalWhitespaceStart of the second change is the same as the
> PreviousOriginalWhitespaceEnd of the first change.
> In this case, the above code marks the second line as trailing comment and
> afterwards applies its own trailing-comment-alignment logic to it, which
> might introduce extra whitespace before "line 2".
>
> For a proper solution we need a mechanism to say that a particular change
> emitted through the whitespace manager breaks the sequence of trailing
> comments.
>
>
Ok. In this case this needs a largish comment with a FIXME, like you just
described ;)
https://reviews.llvm.org/D28764
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
