[PATCH] Split long lines in multi-line comments.

[Daniel Jasper]

================
Comment at: unittests/Format/FormatTest.cpp:661
@@ +660,3 @@
+            format("/*\n"
+                   "This is a long      "
+                   "comment that doesn't "
----------------
nit: Is there a specific reason to split here and in the next line? I think not, but it makes you wonder a little.

================
Comment at: unittests/Format/FormatTest.cpp:658
@@ +657,3 @@
+            "comment that doesn't\n"
+            "fit on one line.   \n"
+            "*/",
----------------
I think we should also delete trailing whitespace anywhere within block comments (we do that in line comments). Probably best to do it in a follow-up patch, though.

================
Comment at: lib/Format/Format.cpp:210
@@ +209,3 @@
+
+  bool safeToSplitLine(StringRef Line) {
+    return Line.find_first_of("/\\:<>()[]{}*&%") == StringRef::npos;
----------------
nit: I'd prefer isLineSafeToSplit

================
Comment at: lib/Format/Format.cpp:196
@@ -195,2 +195,3 @@
 private:
-  void indentBlockComment(const FormatToken &Tok, int Indent) {
+  StringRef findCommonPrefix(ArrayRef<StringRef> Lines) {
+    if (Lines.size() < 3)
----------------
This needs a comment describing the logic (or input->output examples).

================
Comment at: lib/Format/Format.cpp:201
@@ +200,3 @@
+    for (size_t i = 2; i < Lines.size(); ++i) {
+      for (size_t j = 0; j < Prefix.size() && j < Lines[i].size(); ++j)
+        if (Prefix[j] != Lines[i][j]) {
----------------
Nit: Add {}

================
Comment at: lib/Format/Format.cpp:254
@@ +253,3 @@
+    if (Prefix.endswith("*")) {
+      NewLine += " ";
+      --ColumnLimit;
----------------
I presume, that the " " is not part of the prefix because findCommonPrefix also looks at the last line of a comment which can be "*/". Why can't findCommonPrefix not just ignore that line instead?

================
Comment at: lib/Format/Format.cpp:274
@@ +273,3 @@
+            SourceMgr,
+            TokenLoc.getLocWithOffset(Line.data() + BreakPos - Text.data()),
+            SpacePos - BreakPos + 1, NewLine));
----------------
I am a bit worried that this seems to rely on implementation details of StringRef (with data() always returning the exact pointer offset after calls to split, substr and ltrim).

================
Comment at: lib/Format/Format.cpp:246
@@ +245,3 @@
+    StringRef Prefix = findCommonPrefix(Lines);
+    size_t ColumnLimit = Style.ColumnLimit - Prefix.size() - IndentDelta;
+    std::string NewLine("\n");
----------------
Try to reuse more stuff from breakProtrudingToken(), which does something similar for String literals. Especially, reuse WhitespaceManager::breakToken(), which already does some of this logic and also works in multi-line defines.


http://llvm-reviews.chandlerc.com/D547