[PATCH] Don't break line comments with escaped newlines.

[Daniel Jasper]

================
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