From 259a038a97c37109752c70a05e17aa811de3bd8f Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 27 May 2013 11:50:16 +0000 Subject: [PATCH] Fix hacky way of preventing a certain type of line break. In general, we like to avoid line breaks like: ... SomeParameter, OtherParameter).DoSomething( ... as they tend to make code really hard to read (how would you even indent the next line?). Previously we have implemented this in a hacky way, which has now shown to lead to problems. This fixes a few weird looking formattings, such as: Before: aaaaa( aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) .aaaaa(aaaaa), aaaaaaaaaaaaaaaaaaaaa); After: aaaaa(aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).aaaaa(aaaaa), aaaaaaaaaaaaaaaaaaaaa); git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182731 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/Format.cpp | 27 +++++++++++++++++---------- unittests/Format/FormatTest.cpp | 3 +++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 4193266629..dcb5f8a6ed 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -257,6 +257,7 @@ public: State.ParenLevel = 0; State.StartOfStringLiteral = 0; State.StartOfLineLevel = State.ParenLevel; + State.LowestLevelOnLine = State.ParenLevel; State.IgnoreStackForComparison = false; // The first token has already been indented and thus consumed. @@ -412,6 +413,9 @@ private: /// \brief The \c ParenLevel at the start of this line. unsigned StartOfLineLevel; + /// \brief The lowest \c ParenLevel on the current line. + unsigned LowestLevelOnLine; + /// \brief The start column of the string literal, if we're in a string /// literal sequence, 0 otherwise. unsigned StartOfStringLiteral; @@ -448,6 +452,8 @@ private: return ParenLevel < Other.ParenLevel; if (StartOfLineLevel != Other.StartOfLineLevel) return StartOfLineLevel < Other.StartOfLineLevel; + if (LowestLevelOnLine != Other.LowestLevelOnLine) + return LowestLevelOnLine < Other.LowestLevelOnLine; if (StartOfStringLiteral != Other.StartOfStringLiteral) return StartOfStringLiteral < Other.StartOfStringLiteral; if (IgnoreStackForComparison || Other.IgnoreStackForComparison) @@ -556,6 +562,7 @@ private: if (Current.isOneOf(tok::arrow, tok::period)) State.Stack.back().LastSpace += Current.FormatTok.TokenLength; State.StartOfLineLevel = State.ParenLevel; + State.LowestLevelOnLine = State.ParenLevel; // Any break on this level means that the parent level has been broken // and we need to avoid bin packing there. @@ -752,6 +759,8 @@ private: State.Stack.pop_back(); --State.ParenLevel; } + State.LowestLevelOnLine = + std::min(State.LowestLevelOnLine, State.ParenLevel); // Remove scopes created by fake parenthesis. for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) { @@ -996,6 +1005,14 @@ private: Previous.Parent && Previous.Parent->isOneOf(tok::l_brace, tok::l_paren, tok::comma)) return false; + // This prevents breaks like: + // ... + // SomeParameter, OtherParameter).DoSomething( + // ... + // As they hide "DoSomething" and are generally bad for readability. + if (Previous.opensScope() && + State.LowestLevelOnLine < State.StartOfLineLevel) + return false; return !State.Stack.back().NoLineBreak; } @@ -1034,16 +1051,6 @@ private: (Previous.ClosesTemplateDeclaration && State.ParenLevel == 0))) return true; - // This prevents breaks like: - // ... - // SomeParameter, OtherParameter).DoSomething( - // ... - // As they hide "DoSomething" and generally bad for readability. - if (Current.isOneOf(tok::period, tok::arrow) && - getRemainingLength(State) + State.Column > getColumnLimit() && - State.ParenLevel < State.StartOfLineLevel) - return true; - if (Current.Type == TT_StartOfName && Line.MightBeFunctionDecl && State.Stack.back().BreakBeforeParameter && State.ParenLevel == 0) return true; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 8d56d7236e..132dbb5aa2 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -2610,6 +2610,9 @@ TEST_F(FormatTest, WrapsAtFunctionCallsIfNecessary) { " .WillRepeatedly(Return(SomeValue));"); verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n" " .insert(ccccccccccccccccccccccc);"); + verifyFormat("aaaaa(aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).aaaaa(aaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( "aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" -- 2.40.0