r240004 - clang-format: Make AlwaysBreakBeforeMultilineStrings more conservative.

[Daniel Jasper]

Author: djasper
Date: Thu Jun 18 04:12:47 2015
New Revision: 240004

URL: http://llvm.org/viewvc/llvm-project?rev=240004&view=rev
Log:
clang-format: Make AlwaysBreakBeforeMultilineStrings more conservative.

In essence this is meant to consistently indent multiline strings by a
fixed amount of spaces from the start of the line. Don't do this in
cases where it wouldn't help anyway.

Before:
  someFunction(aaaaa,
               "aaaaa"
               "bbbbb");

After:
  someFunction(aaaaa, "aaaaa"
                      "bbbbb");

Modified:
    cfe/trunk/docs/ClangFormatStyleOptions.rst
    cfe/trunk/include/clang/Format/Format.h
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=240004&r1=240003&r2=240004&view=diff
==============================================================================
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Jun 18 04:12:47 2015
@@ -228,6 +228,11 @@ the configuration (without a prefix: ``A
 **AlwaysBreakBeforeMultilineStrings** (``bool``)
   If ``true``, always break before multiline string literals.
 
+  This flag is mean to make cases where there are multiple multiline strings
+  in a file look more consistent. Thus, it will only take effect if wrapping
+  the string at that point leads to it being indented
+  ``ContinuationIndentWidth`` spaces from the start of the line.
+
 **AlwaysBreakTemplateDeclarations** (``bool``)
   If ``true``, always break after the ``template<...>`` of a
   template declaration.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=240004&r1=240003&r2=240004&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Thu Jun 18 04:12:47 2015
@@ -287,6 +287,11 @@ struct FormatStyle {
   bool AlwaysBreakTemplateDeclarations;
 
   /// \brief If \c true, always break before multiline string literals.
+  ///
+  /// This flag is mean to make cases where there are multiple multiline strings
+  /// in a file look more consistent. Thus, it will only take effect if wrapping
+  /// the string at that point leads to it being indented
+  /// \c ContinuationIndentWidth spaces from the start of the line.
   bool AlwaysBreakBeforeMultilineStrings;
 
   /// \brief Different ways to use tab in formatting.

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=240004&r1=240003&r2=240004&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Jun 18 04:12:47 2015
@@ -150,12 +150,6 @@ bool ContinuationIndenter::mustBreak(con
       State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
       !Current.isOneOf(tok::r_paren, tok::r_brace))
     return true;
-  if (Style.AlwaysBreakBeforeMultilineStrings &&
-      State.Column > State.Stack.back().Indent && // Breaking saves columns.
-      !Previous.isOneOf(tok::kw_return, tok::lessless, tok::at) &&
-      !Previous.isOneOf(TT_InlineASMColon, TT_ConditionalExpr) &&
-      nextIsMultilineString(State))
-    return true;
   if (((Previous.is(TT_DictLiteral) && Previous.is(tok::l_brace)) ||
        Previous.is(TT_ArrayInitializerLSquare)) &&
       Style.ColumnLimit > 0 &&
@@ -170,9 +164,17 @@ bool ContinuationIndenter::mustBreak(con
       State.Stack.back().BreakBeforeParameter)
     return true;
 
-  if (State.Column < getNewLineColumn(State))
+  unsigned NewLineColumn = getNewLineColumn(State);
+  if (State.Column < NewLineColumn)
     return false;
 
+  if (Style.AlwaysBreakBeforeMultilineStrings &&
+      NewLineColumn == State.FirstIndent + Style.ContinuationIndentWidth &&
+      !Previous.isOneOf(tok::kw_return, tok::lessless, tok::at) &&
+      !Previous.isOneOf(TT_InlineASMColon, TT_ConditionalExpr) &&
+      nextIsMultilineString(State))
+    return true;
+
   // Using CanBreakBefore here and below takes care of the decision whether the
   // current style uses wrapping before or after operators for the given
   // operator.

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=240004&r1=240003&r2=240004&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jun 18 04:12:47 2015
@@ -4635,13 +4635,11 @@ TEST_F(FormatTest, AlwaysBreakBeforeMult
   verifyFormat("aaaa(qqq, \"bbbb\"\n"
                "          \"cccc\");",
                NoBreak);
-  verifyFormat("aaaa(qqq,\n"
-               "     \"bbbb\"\n"
-               "     \"cccc\");",
+  verifyFormat("aaaa(qqq, \"bbbb\"\n"
+               "          \"cccc\");",
                Break);
-  verifyFormat("aaaa(qqq,\n"
-               "     L\"bbbb\"\n"
-               "     L\"cccc\");",
+  verifyFormat("aaaa(qqq, L\"bbbb\"\n"
+               "          L\"cccc\");",
                Break);
 
   // As we break before unary operators, breaking right after them is bad.
@@ -4663,11 +4661,11 @@ TEST_F(FormatTest, AlwaysBreakBeforeMult
                    "b\\\n"
                    "c\";",
                    NoBreak));
-  EXPECT_EQ("x =\n"
+  EXPECT_EQ("xxxx =\n"
             "    \"a\\\n"
             "b\\\n"
             "c\";",
-            format("x = \"a\\\n"
+            format("xxxx = \"a\\\n"
                    "b\\\n"
                    "c\";",
                    Break));
@@ -7492,13 +7490,14 @@ TEST_F(FormatTest, BreaksStringLiterals)
 
   // Verify that splitting the strings understands
   // Style::AlwaysBreakBeforeMultilineStrings.
-  EXPECT_EQ("aaaaaaaaaaaa(aaaaaaaaaaaaa,\n"
-            "             \"aaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa \"\n"
-            "             \"aaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa\");",
-            format("aaaaaaaaaaaa(aaaaaaaaaaaaa, \"aaaaaaaaaaaaaaaaaaaaaa "
-                   "aaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa "
-                   "aaaaaaaaaaaaaaaaaaaaaa\");",
-                   getGoogleStyle()));
+  EXPECT_EQ(
+      "aaaaaaaaaaaa(\n"
+      "    \"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa \"\n"
+      "    \"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa\");",
+      format("aaaaaaaaaaaa(\"aaaaaaaaaaaaaaaaaaaaaaaaaa "
+             "aaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaa "
+             "aaaaaaaaaaaaaaaaaaaaaa\");",
+             getGoogleStyle()));
   EXPECT_EQ("return \"aaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaa \"\n"
             "       \"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaa\";",
             format("return \"aaaaaaaaaaaaaaaaaaaaaa "