From 2aaedd3a7eaf98fc06c2fac068ceb6ca1d5d3d30 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Thu, 18 Jun 2015 09:12:47 +0000 Subject: [PATCH] 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"); llvm-svn: 240004 --- clang/docs/ClangFormatStyleOptions.rst | 5 +++++ clang/include/clang/Format/Format.h | 5 +++++ clang/lib/Format/ContinuationIndenter.cpp | 16 +++++++++------- clang/unittests/Format/FormatTest.cpp | 29 ++++++++++++++--------------- 4 files changed, 33 insertions(+), 22 deletions(-) diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index edec058..f4c4c8c 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -228,6 +228,11 @@ the configuration (without a prefix: ``Auto``). **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. diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index c4c28ec..ad87a05 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -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. diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 8a55241..7e751d4 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -150,12 +150,6 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { 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(const LineState &State) { 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. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index d9298ef..ea5ff18 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -4635,13 +4635,11 @@ TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) { 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, AlwaysBreakBeforeMultilineStrings) { "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 " -- 2.7.4