<div dir="ltr">My thoughts:<div>- "<span style="font-family:arial,sans-serif;font-size:13px">more legible" is highly subjective. I for one like clang-format's choice as it uses fewer lines and (more importantly) fewer different indents. So if I had to manually choose between the two, I would choose the former.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">- We have explicitly changed this for Google style and we are not changing it back. So if this change to submit this change, we need to introduce an additional style option (and I have no idea what to call it ;-) ). Maybe the best choice would be to pull out the actual penalty into something that can be configured per style. E.g.: PenaltyWrapCallAfterParen.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><font face="arial, sans-serif">- There is an alternative to consider. In earlier days of clang-format we used hanging identation (we still do with all other binary operators). I.e. the snippet you mention would be formatted as:</font></div>
<div><font face="arial, sans-serif"><br></font></div><div><font face="courier new, monospace"><span style="font-size:13px">CharSourceRange LineRange = </span><span style="font-size:13px">CharSourceRange::</span><span style="font-size:13px">getTokenRange(</span></font></div>
<div><font face="courier new, monospace"><span style="font-size:13px"> Line.Tokens.</span><span style="font-size:13px">front().Tok.getLo(),</span><br style="font-size:13px"><span style="font-size:13px"> Line.Tokens.back().Tok.getLoc(</span><span style="font-size:13px">));</span></font><font face="arial, sans-serif"><br>
</font></div><div><font face="courier new, monospace"><span style="font-size:13px"><br></span></font></div><div><span style="font-size:13px"><font face="arial, helvetica, sans-serif">Now, arguably, this is preferable as it is more structured than what clang-format currently does and at the same time does not need three different indentations (which makes it look 'untidy'). That option, however, did not fly in Google style and we had to special-case all assignment expressions. Again, that is something we could change specifically for LLVM style. However, from offline discussions I assume that breaking after the opening parenthesis bothers you in more places. Just wanted to bring this back onto the table as I know Chandler is a big fan :-).</font></span></div>
<div><span style="font-size:13px"><font face="arial, helvetica, sans-serif"><br></font></span></div><div><span style="font-size:13px"><font face="arial, helvetica, sans-serif">Cheers,<br></font></span></div><div><span style="font-size:13px"><font face="arial, helvetica, sans-serif">Daniel</font></span></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 23, 2013 at 2:46 AM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Bah. The formatting didn't go in the email as I'd expected.<br><br>Before:<br><div><br></div><div>
<ol style="margin:0px 0px 0px 45px;padding:0px 0px 0px 1px;border-width:0px 0px 0px 1px;border-left-style:solid;border-left-color:rgb(204,204,204);font-family:InconsolataMedium,monospace;font-size:13px;line-height:16px;vertical-align:baseline;list-style-position:initial;color:rgb(40,40,40);background-color:rgb(248,248,248)">
<div class="im">
<li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
<div style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline"> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">CharSourceRange</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">LineRange</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;color:rgb(102,102,102)">=</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">CharSourceRange</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;color:rgb(102,102,102)">::</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getTokenRange</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">(</span></div>
</li></div><li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
<div style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline"> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Line</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tokens</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">front</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">().</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tok</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getLo</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">(),</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Line</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tokens</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">back</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">().</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tok</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getLoc</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">());</span></div>
</li><li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
</li></ol></div><div><br></div><div>After:</div><div><br></div><div><ol style="margin:0px 0px 0px 45px;padding:0px 0px 0px 1px;border-width:0px 0px 0px 1px;border-left-style:solid;border-left-color:rgb(204,204,204);font-family:InconsolataMedium,monospace;font-size:13px;line-height:16px;vertical-align:baseline;list-style-position:initial;color:rgb(40,40,40);background-color:rgb(248,248,248)">
<li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
<div style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline"> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">CharSourceRange</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">LineRange</span> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;color:rgb(102,102,102)">=</span></div>
</li><li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
<div style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline"> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">CharSourceRange</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;color:rgb(102,102,102)">::</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getTokenRange</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">(</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Line</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tokens</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">front</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">().</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tok</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getLo</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">(),</span></div>
</li><li style="margin:0px 0px 0px 3px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline;white-space:pre-wrap">
<div style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline"> <span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Line</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tokens</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">back</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">().</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">Tok</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">.</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">getLoc</span><span style="margin:0px;padding:0px;border:0px;font-family:inherit;font-size:inherit;font-style:inherit;font-variant:inherit;font-weight:inherit;line-height:inherit;vertical-align:baseline">());</span></div>
</li></ol></div><div><br></div><div>Whee.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-eric</div><div><br></div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br>
<br><div class="gmail_quote">On Tue, Oct 22, 2013 at 5:37 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Daniel,<br>
<br>
Thought I'd send it out and get some discussion if people care and<br>
then I can update the tests and commit if we think it's a good idea.<br>
<br>
It'll change code like in (one of the) failing tests:<br>
<br>
Value of: format(messUp(Code), Style)<br>
Actual: "CharSourceRange LineRange =\n<br>
CharSourceRange::getTokenRange(Line.Tokens.front().Tok.getLo(),\n<br>
Line.Tokens.back().Tok.getLoc());"<br>
Expected: Code.str()<br>
Which is: "CharSourceRange LineRange =<br>
CharSourceRange::getTokenRange(\n Line.Tokens.front().Tok.getLo(),<br>
Line.Tokens.back().Tok.getLoc());"<br>
[ FAILED ] FormatTest.BreaksAfterAssignments (5 ms)<br>
[----------] 1 test from FormatTest (5 ms total)<br>
<br>
this:<br>
<br>
<pre style='color:#000000;background:#ffffff;'><html><body<br>
style='color:#000000; background:#ffffff; '><pre><br>
CharSourceRange LineRange <span style='color:#808030; '>=</span><br>
CharSourceRange<span style='color:#800080;<br>
'>::</span>getTokenRange<span style='color:#808030;<br>
'>(</span>Line<span style='color:#808030; '>.</span>Tokens<span<br>
style='color:#808030; '>.</span>front<span style='color:#808030;<br>
'>(</span><span style='color:#808030; '>)</span><span<br>
style='color:#808030; '>.</span>Tok<span style='color:#808030;<br>
'>.</span>getLo<span style='color:#808030; '>(</span><span<br>
style='color:#808030; '>)</span><span style='color:#808030; '>,</span><br>
Line<span style='color:#808030;<br>
'>.</span>Tokens<span style='color:#808030; '>.</span>back<span<br>
style='color:#808030; '>(</span><span style='color:#808030;<br>
'>)</span><span style='color:#808030; '>.</span>Tok<span<br>
style='color:#808030; '>.</span>getLoc<span style='color:#808030;<br>
'>(</span><span style='color:#808030; '>)</span><span<br>
style='color:#808030; '>)</span><span style='color:#800080; '>;</span><br>
</pre><br>
<br>
versus:<br>
<br>
<pre style='color:#000000;background:#ffffff;'><html><body<br>
style='color:#000000; background:#ffffff; '><pre><br>
CharSourceRange LineRange <span style='color:#808030; '>=</span><br>
CharSourceRange<span style='color:#800080;<br>
'>::</span>getTokenRange<span style='color:#808030; '>(</span><br>
Line<span style='color:#808030; '>.</span>Tokens<span<br>
style='color:#808030; '>.</span>front<span style='color:#808030;<br>
'>(</span><span style='color:#808030; '>)</span><span<br>
style='color:#808030; '>.</span>Tok<span style='color:#808030;<br>
'>.</span>getLo<span style='color:#808030; '>(</span><span<br>
style='color:#808030; '>)</span><span style='color:#808030; '>,</span><br>
Line<span style='color:#808030; '>.</span>Tokens<span<br>
style='color:#808030; '>.</span>back<span style='color:#808030;<br>
'>(</span><span style='color:#808030; '>)</span><span<br>
style='color:#808030; '>.</span>Tok<span style='color:#808030;<br>
'>.</span>getLoc<span style='color:#808030; '>(</span><span<br>
style='color:#808030; '>)</span><span style='color:#808030;<br>
'>)</span><span style='color:#800080; '>;</span><br>
</pre><br>
<br>
which while the former is more lines I think it is a much more legible<br>
general formatting style.<br>
<br>
Thoughts?<br>
<span><font color="#888888"><br>
-eric<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>