Hi Jean-Philippe, first off, sorry that this is causing you trouble.
I am always happy to reconsider. However, I do think that this is a good change based on hundreds of examples I have looked at. Obviously, this is subjective and I won't even start to make an argument on whether this is wasted or not. We could add a separate option, but it is not quite as easy. AlwaysBreak has never been *always* break. There have always been exceptions. Two immediately spring to mind: 1. When the function name (plus open parenthesis) is shorter or equal to the ContinuationIndentWidth. 2. For nested blocks such as lambdas, at least in some of the cases. Both are also about wasting space. I think single argument function calls are closely related to those two. So, we could add an "StrictAlwaysBreak" mode and then also not have the two exemptions above, but I am not sure it is worth it. How often do these cases occur in your codebase? Do you really feel like there is a significant readability disadvantage? Cheers, Daniel On Sun, Mar 20, 2016 at 12:00 PM, Jean-philippe Dufraigne < [email protected]> wrote: > Hi Daniel, > > I'm not sure I understand the logic behind modifying 'AlwaysBreak' to be > 'AlwaysBreakExceptForSingleArgument'. > > AlwaysBreak is always going to lead to "wasted space". > Some consider consistently break to not be a waste but actually contribute > to readability, it seems that was what 'AlwaysBreak' was about. > > Would you reconsider this change, or make it an option different from > 'AlwaysBreak'? > > > > We just completed the roll out clang-format at my company. > We are using: AlignAfterOpenBracket: AlwaysBreak > 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. > > Kind regards, > Jean-Philippe. > > > 2016-03-17 12:00 GMT+00:00 Daniel Jasper via cfe-commits < > [email protected]>: > >> Author: djasper >> Date: Thu Mar 17 07:00:22 2016 >> New Revision: 263709 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=263709&view=rev >> Log: >> clang-format: Slightly weaken AlignAfterOpenBracket=AlwaysBreak. >> >> If a call takes a single argument, using AlwaysBreak can lead to lots >> of wasted lines and additional indentation without improving the >> readability in a significant way. >> >> Before: >> caaaaaaaaaaaall( >> caaaaaaaaaaaall( >> caaaaaaaaaaaall( >> caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa)))); >> >> After: >> caaaaaaaaaaaall(caaaaaaaaaaaall(caaaaaaaaaaaall( >> caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, aaaaaaaaa)))); >> >> Modified: >> cfe/trunk/lib/Format/ContinuationIndenter.cpp >> cfe/trunk/unittests/Format/FormatTest.cpp >> >> Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=263709&r1=263708&r2=263709&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) >> +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Mar 17 07:00:22 2016 >> @@ -356,7 +356,17 @@ void ContinuationIndenter::addTokenOnCur >> Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) && >> State.Column > getNewLineColumn(State) && >> (!Previous.Previous || >> - !Previous.Previous->isOneOf(tok::kw_for, tok::kw_while, >> tok::kw_switch))) >> + !Previous.Previous->isOneOf(tok::kw_for, tok::kw_while, >> + tok::kw_switch)) && >> + // Don't do this for simple (no expressions) one-argument function >> calls >> + // as that feels like needlessly wasting whitespace, e.g.: >> + // >> + // caaaaaaaaaaaall( >> + // caaaaaaaaaaaall( >> + // caaaaaaaaaaaall( >> + // caaaaaaaaaaaaaaaaaaaaaaall(aaaaaaaaaaaaaa, >> aaaaaaaaa)))); >> + Current.FakeLParens.size() > 0 && >> + Current.FakeLParens.back() > prec::Unknown) >> State.Stack.back().NoLineBreak = true; >> >> if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign && >> >> Modified: cfe/trunk/unittests/Format/FormatTest.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=263709&r1=263708&r2=263709&view=diff >> >> ============================================================================== >> --- cfe/trunk/unittests/Format/FormatTest.cpp (original) >> +++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Mar 17 07:00:22 2016 >> @@ -4461,12 +4461,31 @@ TEST_F(FormatTest, AlignsAfterOpenBracke >> " aaaaaaaaaaa aaaaaaaaa,\n" >> " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);", >> Style); >> - verifyFormat("SomeLongVariableName->someFunction(\n" >> - " foooooooo(\n" >> - " aaaaaaaaaaaaaaa,\n" >> - " aaaaaaaaaaaaaaaaaaaaa,\n" >> - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));", >> + verifyFormat("SomeLongVariableName->someFunction(foooooooo(\n" >> + " aaaaaaaaaaaaaaa,\n" >> + " aaaaaaaaaaaaaaaaaaaaa,\n" >> + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));", >> Style); >> + verifyFormat( >> + "aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)));", >> + Style); >> + verifyFormat( >> + "aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaa.aaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa)));", >> + Style); >> + verifyFormat( >> + "aaaaaaaaaaaaaaaaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, >> aaaaaaaaaaaaaaaa)),\n" >> + " aaaaaaaaaaaaaaaa);", >> + Style); >> + verifyFormat( >> + "aaaaaaaaaaaaaaaaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaaa(\n" >> + " aaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaa, >> aaaaaaaaaaaaaaaa)) &&\n" >> + " aaaaaaaaaaaaaaaa);", >> + Style); >> } >> >> TEST_F(FormatTest, ParenthesesAndOperandAlignment) { >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
