================
Comment at: unittests/Format/FormatTest.cpp:938
@@ +937,3 @@
+ getLLVMStyleWithColumns(50)));
+ EXPECT_EQ("int\n"
+ "a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
----------------
Maybe add a FIXME as this is not generally desirable. I know that there is one
in the code, but having that also in the test is also helpful. E.g. imagine you
fix this some day (either directly or by accident doing another change). If you
the find this comment on the breaking test, you know that you can just fix the
test.
================
Comment at: lib/Format/Format.cpp:925
@@ +924,3 @@
+ //
+ // TODO: If we want to handle them correctly, we'll need to adjust
leading
+ // whitespace in consecutive lines when changing indentation of the first
----------------
s/TODO/FIXME/
================
Comment at: lib/Format/Format.cpp:921
@@ -916,1 +920,3 @@
Current.Previous->Type != TT_ImplicitStringLiteral)) {
+ // Don't break line comments with escaped newlines. These look like
+ // separate line comments, but in fact contain consecutive lines
including
----------------
The second sentence is confusing. "These look like separate line comments, but
in fact contain consecutive lines..". There is no real contradiction.
How about:
"These look like separate line comments, but in fact contain a single line
comment with multiple lines including leading whitespace and the '//' markers."?
================
Comment at: lib/Format/Format.cpp:935
@@ +934,3 @@
+ 1;
+ State.Column = Column;
+ return (Column > getColumnLimit())
----------------
I am confused by this. Shouldn't State.Column be set to the remaining length of
the comment after the last escaped newline?
================
Comment at: lib/Format/Format.cpp:937
@@ +936,3 @@
+ return (Column > getColumnLimit())
+ ? Style.PenaltyExcessCharacter * (Column - getColumnLimit())
+ : 0;
----------------
With this, you are adding the penalty for the first line (i.e. for the length
of the comment up to the first escaped newline).
It seems to me that we should either add the penalty for each of the comment's
lines (except the last one, which is added later) or for none of them.
Otherwise, this is a half-way solution that is only correct for a few edge
cases and might confuse people.
================
Comment at: lib/Format/Format.cpp:908
@@ +907,3 @@
+ // Don't break string literals with escaped newlines. We're not likely to
+ // get them correctly, so better don't spoil them.
+ if (Current.TokenText.find("\\\n") != StringRef::npos)
----------------
The second sentence does not add much value. How about:
"Don't break string literals with escaped newlines. As clang-format must not
change the string's content, it is unlikely that we'll end up with a better
format."
http://llvm-reviews.chandlerc.com/D1146
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
