<div dir="ltr">Hi Daniel,<div><br></div><div><div>I'm not sure I understand the logic behind modifying 'AlwaysBreak' to be 'AlwaysBreakExceptForSingleArgument'.</div><div><br></div><div>AlwaysBreak is always going to lead to "wasted space".</div><div>Some consider consistently break to not be a waste but actually contribute to readability, it seems that was what 'AlwaysBreak' was about.<br></div><div><br></div><div>Would you reconsider this change, or make it an option different from 'AlwaysBreak'?</div></div><div><br></div><div><br></div><div><br></div><div>We just completed the roll out clang-format at my company.</div><div><div>We are using: AlignAfterOpenBracket: AlwaysBreak</div></div><div>We tuned the settings to have a style that does not clash too much with our original style, and that will look like a regression for us.<br></div><div><br></div><div>Kind regards,</div><div>Jean-Philippe.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2016-03-17 12:00 GMT+00:00 Daniel Jasper via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: djasper<br>
Date: Thu Mar 17 07:00:22 2016<br>
New Revision: 263709<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263709&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=263709&view=rev</a><br>
Log:<br>
clang-format: Slightly weaken AlignAfterOpenBracket=AlwaysBreak.<br>
<br>
If a call takes a single argument, using AlwaysBreak can lead to lots<br>
of wasted lines and additional indentation without improving the<br>
readability in a significant way.<br>
<br>
Before:<br>
caaaaaaaaaaaall(<br>
caaaaaaaaaaaall(<br>
caaaaaaaaaaaall(<br>
caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa))));<br>
<br>
After:<br>
caaaaaaaaaaaall(caaaaaaaaaaaall(caaaaaaaaaaaall(<br>
caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa))));<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/ContinuationIndenter.cpp<br>
cfe/trunk/unittests/Format/FormatTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=263709&r1=263708&r2=263709&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=263709&r1=263708&r2=263709&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)<br>
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Mar 17 07:00:22 2016<br>
@@ -356,7 +356,17 @@ void ContinuationIndenter::addTokenOnCur<br>
Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) &&<br>
State.Column > getNewLineColumn(State) &&<br>
(!Previous.Previous ||<br>
- !Previous.Previous->isOneOf(tok::kw_for, tok::kw_while, tok::kw_switch)))<br>
+ !Previous.Previous->isOneOf(tok::kw_for, tok::kw_while,<br>
+ tok::kw_switch)) &&<br>
+ // Don't do this for simple (no expressions) one-argument function calls<br>
+ // as that feels like needlessly wasting whitespace, e.g.:<br>
+ //<br>
+ // caaaaaaaaaaaall(<br>
+ // caaaaaaaaaaaall(<br>
+ // caaaaaaaaaaaall(<br>
+ // caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa))));<br>
+ Current.FakeLParens.size() > 0 &&<br>
+ Current.FakeLParens.back() > prec::Unknown)<br>
State.Stack.back().NoLineBreak = true;<br>
<br>
if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=263709&r1=263708&r2=263709&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=263709&r1=263708&r2=263709&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Mar 17 07:00:22 2016<br>
@@ -4461,12 +4461,31 @@ TEST_F(FormatTest, AlignsAfterOpenBracke<br>
" aaaaaaaaaaa aaaaaaaaa,\n"<br>
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);",<br>
Style);<br>
- verifyFormat("SomeLongVariableName->someFunction(\n"<br>
- " foooooooo(\n"<br>
- " aaaaaaaaaaaaaaa,\n"<br>
- " aaaaaaaaaaaaaaaaaaaaa,\n"<br>
- " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));",<br>
+ verifyFormat("SomeLongVariableName->someFunction(foooooooo(\n"<br>
+ " aaaaaaaaaaaaaaa,\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaa,\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));",<br>
Style);<br>
+ verifyFormat(<br>
+ "aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)));",<br>
+ Style);<br>
+ verifyFormat(<br>
+ "aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaa.aaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)));",<br>
+ Style);<br>
+ verifyFormat(<br>
+ "aaaaaaaaaaaaaaaaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)),\n"<br>
+ " aaaaaaaaaaaaaaaa);",<br>
+ Style);<br>
+ verifyFormat(<br>
+ "aaaaaaaaaaaaaaaaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaaa(\n"<br>
+ " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)) &&\n"<br>
+ " aaaaaaaaaaaaaaaa);",<br>
+ Style);<br>
}<br>
<br>
TEST_F(FormatTest, ParenthesesAndOperandAlignment) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>