klimek added inline comments.
================
Comment at: lib/Format/ContinuationIndenter.cpp:1028
+ unsigned Penalty =
+ handleEndOfLine(Current, State, DryRun, AllowBreak);
----------------
krasimir wrote:
> Why `handleEndOfLine`? Is it guaranteed that here we've reached the end of
> the line?
No, this is used to do things in case we have reached the end of the line, as
that's hard to check. I also wanted a better name, but wasn't able to come up
with one (and we do similar things in other places here, where checking a
condition is equivalent to running through all the code to do the things
necessary).
================
Comment at: lib/Format/ContinuationIndenter.cpp:1284
+ unsigned StartColumn = State.Column - Current.ColumnWidth;
+ auto Delimiter = *getRawStringDelimiter(Current.TokenText);
// The text of a raw string is between the leading 'R"delimiter(' and the
----------------
krasimir wrote:
> At this point, we compute `getRawStringDelimiter` twice: once here, and once
> in `getRawStringStyle` in the caller side. Ideally, we'd only like to compute
> it once. That's why it was a parameter before.
I got that, but I think passing around the delimiter is super awkward, and
computing it is cheap. If computing it is the problem we should store it in the
FormatToken.
================
Comment at: lib/Format/ContinuationIndenter.cpp:1408
+std::unique_ptr<BreakableToken> ContinuationIndenter::createBreakableToken(
+ const FormatToken &Current, LineState &State, bool AllowBreak) {
unsigned StartColumn = State.Column - Current.ColumnWidth;
----------------
krasimir wrote:
> If `AllowBreak == false`, in which cases will this return a non-`nullptr`?
Only for string literals, as AllowBreak is really about whether we can break in
the token space, but for comments (or raw strings, but those are handled
differently anyway) we break in the comment space.
================
Comment at: unittests/Format/FormatTest.cpp:6322
"#include \"some long include\" // with a comment\n"
- "#include \"some very long include
paaaaaaaaaaaaaaaaaaaaaaath\"",
+ "#include \"some very long include path\"\n"
+ "#include <some/very/long/include/path>\n",
----------------
krasimir wrote:
> What happened to the old test line?
It was unnecessarily long. I found it weird that it went out of its way to add
more a's over the line limit.
https://reviews.llvm.org/D39900
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
