<div dir="ltr">As for structure: With simple cases and all operands having the same length, it always looks structured. But consider the case from the original code review:<div><pre style="white-space:pre-wrap"><font face="courier new, monospace">return (ui::GetDisplayLayout() == ui::LAYOUT_TOUCH) ?
GetItemPadding() / 2 : 0;</font></pre><pre><pre style="white-space:pre-wrap"><font face="courier new, monospace">return (ui::GetDisplayLayout() == ui::LAYOUT_TOUCH) ? GetItemPadding() / 2
: 0;</font></pre><font face="arial, helvetica, sans-serif">I know that a lot of this boils down to personal preference and what patterns one is used to, but I find the second version significantly more structured. The semi-objective reasons I can come up with are:</font></pre>
<pre><font face="arial, helvetica, sans-serif">a) The alignment and close proximity of ? and : aid in understanding that this is a conditional expression and clearly separate / show the boundaries of the three operands.</font></pre>
<pre><font face="arial, helvetica, sans-serif">b) A conditional expression is a branch and branches in code are usually vertical.</font></pre><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 13, 2013 at 12:08 AM, Peter Kasting <span dir="ltr"><<a href="mailto:pkasting@chromium.org" target="_blank" class="cremed">pkasting@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>On Mon, Aug 12, 2013 at 2:40 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank" class="cremed">thakis@chromium.org</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div><span style="color:rgb(34,34,34)">I told Daniel that we generally follow google-internal style unless there are strong reasons not to. This issue has been discussed at length for google-internal style, and I personally don't see a strong reason to deviate here.</span></div>
</div></div></blockquote><div><br></div></div><div>"All the existing code does it another way" is a strong enough reason to me.</div></div></div></div></blockquote><div><br></div><div>It would be, but that is simply not the case. It is hard to come up with good numbers, but e.g. compare the numbers for ":" on the new and old lines:</div>
<div><br></div><div><a href="https://code.google.com/p/chromium/codesearch#search/&q=(?m:%5C?.*%5Cn%5C%20*%5C:)%20-file:v8%20-file:usr/include%20lang:cc%20pcre:yes&all=1&sq=package:chromium&type=cs" target="_blank" class="cremed">https://code.google.com/p/chromium/codesearch#search/&q=(?m:%5C?.*%5Cn%5C%20*%5C:)%20-file:v8%20-file:usr/include%20lang:cc%20pcre:yes&all=1&sq=package:chromium&type=cs</a></div>
<div><br></div><div><a href="https://code.google.com/p/chromium/codesearch#search/&q=(?m:%5C?%5Cn%7B0,1%7D.*%5C%20%5C:$)%20-file:v8%20-file:usr/include%20lang:cc%20pcre:yes&sq=package:chromium&type=cs&all=1" target="_blank" class="cremed">https://code.google.com/p/chromium/codesearch#search/&q=(?m:%5C?%5Cn%7B0,1%7D.*%5C%20%5C:$)%20-file:v8%20-file:usr/include%20lang:cc%20pcre:yes&sq=package:chromium&type=cs&all=1</a><br>
</div><div><br></div><div>I have tried to tune the regular expressions to include all cases where the conditional expression is broken around the ":". The result is about 400:1000 (this time excluding v8 and usr/include). Yes, it is a preference, but clang-format's style is far from unprecedented.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div>I continue to believe that the Chromium-style-specific formatter should not wrap this way. I also believe this about any other style question where there is a consistent pattern within the Chrome codebase, regardless of how many options are legal.</div>
</div></div></div></blockquote><div><br></div><div>I think this is always a balancing act between accepting slightly different (but legal and readable style) and aiming for perfect consistency. IMO, both Chromium's and Google's style guides are quite elaborate and the wiggle room they leave should not be taken away completely by many implicit rules based on the consistency-argument. For me, conditional expressions are right on the edge. I could probably read code that alternates between ?/: on the new/old line just fine. On the other hand, I find the variety and inconsistency in indentation that either of the two links above show quite distracting.</div>
<div><br></div><div>I don't think escalating to chromium-dev is a good idea. That seems like a waste of a lot of people's time on a corner issue (with only a few thousand instances in the codebase). Why am I not just making the change? Several reasons:</div>
<div>1) I don't want to flip-flop. You are the first Chromium developer to complain about this. Thus, there seem to be many others that are ok with this or even like it. Now, as I said, I don't know who makes the decisions for Chromium, but I'd like to have a final decision before changing anything.</div>
<div>2) To me, this seems to make formatting strictly worse. As I am not working on the Chromium codebase, that does not count much and will certainly not influence the final solution, but it somehow adds to my reservations.</div>
<div>3) It is additional implementation effort (although patches are always welcome :-) ).</div><div>4) It is additional maintenance effort as we'd need to create an additional style option and ensuring that the combinatorial explosion of style options always do the right thing is difficult (e.g. how to indent conditional expressions that are parameters to ObjC functions..).</div>
<div><br></div></div>Cheers,<br>Daniel</div></div></div>