From f54617858a0df936746b7f521a8ffb032289d02f Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Wed, 28 Aug 2013 10:03:58 +0000 Subject: [PATCH] clang-format: Improve token breaking behavior. Two changes: * Don't add an extra penalty on breaking the same token multiple times. Generally, we should prefer not to break, but once we break, the normal line breaking penalties apply. * Slightly increase the penalty for breaking comments. In general, the author has put some thought into how to break the comment and we should not overwrite this unnecessarily. With a 40-column column limit, formatting aaaaaa("aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa"); Leads to: Before: aaaaaa( "aaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaa " "aaaaaaaaaaaaaaaa"); After: aaaaaa("aaaaaaaaaaaaaaaa " "aaaaaaaaaaaaaaaa " "aaaaaaaaaaaaaaaa"); git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@189466 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/ContinuationIndenter.cpp | 6 ++++-- lib/Format/Format.cpp | 2 +- unittests/Format/FormatTest.cpp | 18 ++++++++++++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 798e4f3aec..f4ae5cce01 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/lib/Format/ContinuationIndenter.cpp @@ -666,8 +666,7 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, assert(NewRemainingTokenColumns < RemainingTokenColumns); if (!DryRun) Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces); - Penalty += Current.is(tok::string_literal) ? Style.PenaltyBreakString - : Style.PenaltyBreakComment; + Penalty += Current.SplitPenalty; unsigned ColumnsUsed = Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first); if (ColumnsUsed > getColumnLimit()) { @@ -691,6 +690,9 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, State.Stack[i].BreakBeforeParameter = true; } + Penalty += Current.is(tok::string_literal) ? Style.PenaltyBreakString + : Style.PenaltyBreakComment; + State.Stack.back().LastSpace = StartColumn; } return Penalty; diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 1cd55d7297..3982ba6a77 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -152,7 +152,7 @@ namespace clang { namespace format { void setDefaultPenalties(FormatStyle &Style) { - Style.PenaltyBreakComment = 45; + Style.PenaltyBreakComment = 60; Style.PenaltyBreakFirstLessLess = 120; Style.PenaltyBreakString = 1000; Style.PenaltyExcessCharacter = 1000000; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 067a259b2c..122b367a22 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -970,8 +970,8 @@ TEST_F(FormatTest, DontSplitLineCommentsWithEscapedNewlines) { } TEST_F(FormatTest, PriorityOfCommentBreaking) { - EXPECT_EQ("if (xxx == yyy && // aaaaaaaaaaaa\n" - " // bbbbbbbbb\n" + EXPECT_EQ("if (xxx ==\n" + " yyy && // aaaaaaaaaaaa bbbbbbbbb\n" " zzz)\n" " q();", format("if (xxx == yyy && // aaaaaaaaaaaa bbbbbbbbb\n" @@ -5392,10 +5392,10 @@ TEST_F(FormatTest, BreakStringLiteralsBeforeUnbreakableTokenSequence) { " \"f\");", format("someFunction1234567890(\"aaabbbcccdddeeefff\");", getLLVMStyleWithColumns(24))); - EXPECT_EQ("someFunction(\n" - " \"aaabbbcc \"\n" - " \"dddeeefff\");", - format("someFunction(\"aaabbbcc dddeeefff\");", + EXPECT_EQ("someFunction(\"aaabbbcc \"\n" + " \"ddde \"\n" + " \"efff\");", + format("someFunction(\"aaabbbcc ddde efff\");", getLLVMStyleWithColumns(25))); EXPECT_EQ("someFunction(\"aaabbbccc \"\n" " \"ddeeefff\");", @@ -5413,6 +5413,12 @@ TEST_F(FormatTest, BreakStringLiteralsBeforeUnbreakableTokenSequence) { " int i;", format("#define A string s = \"1234567890\"; int i;", getLLVMStyleWithColumns(20))); + // FIXME: Put additional penalties on breaking at non-whitespace locations. + EXPECT_EQ("someFunction(\"aaabbbcc \"\n" + " \"dddeeeff\"\n" + " \"f\");", + format("someFunction(\"aaabbbcc dddeeefff\");", + getLLVMStyleWithColumns(25))); } TEST_F(FormatTest, DoNotBreakStringLiteralsInEscapeSequence) { -- 2.50.1