<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 12, 2013 at 9:46 PM, 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 class="im">On Sun, Aug 11, 2013 at 11:58 PM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<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">This was decided to be the best way to go forward with Google's C++ style. I am reasonably certain that the style guide does not contain anything that says otherwise. The section you quote:<div>
a) applies to boolean expressions, which a conditional expression is not.</div>
<div>b) explicitly says that you can also wrap them to the new line.</div><div><br></div><div>If you find anything else in the style guide, please let me know, I am happy to get that changed.</div></div></blockquote><div>
<br></div></div><div>The Google style guide used to explicitly ban wrapping before an operator (not just a boolean operator, but any operator) rather than after. IIRC it no longer does so, but Chrome code generally dates from before this change and is very consistent about this wrapping, not just for ?:, but importantly for other operators as well (where we also need consistency).</div>
</div></div></div></blockquote><div><br></div><div>The Chromium style guide explicitly says: "Coding style for the Chromium projects generally follows the Google C++ Style Guide. The notes below are usually just extensions beyond what the Google style guide already says. If this document doesn't mention a rule, follow the Google C++ style.". IMO, this includes changes to the style guide (which happen happen frequently).</div>
<div><br></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 class="im"><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>As for "This is not done in Chromium":<br>
</div>
<div><a href="https://code.google.com/p/chromium/codesearch#search/&q=%5E%5C%20*%5C?&all=1&sq=package:chromium&type=cs" target="_blank" class="cremed">https://code.google.com/p/chromium/codesearch#search/&q=%5E%5C%20*%5C?&all=1&sq=package:chromium&type=cs</a><br>
</div><div>and</div><div><a href="https://code.google.com/p/chromium/codesearch#search/&q=(?m:%5C?.*%5Cn%5Cs*%5C:)%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%5Cs*%5C:)%20pcre:yes&all=1&sq=package:chromium&type=cs</a></div>
</div></blockquote><div><br></div></div><div>Your search includes things like v8 and third_party, which have their own conventions and are not "Chromium" code. Restrict to Chromium code, and I suspect the ratio will change.</div>
</div></div></div></blockquote><div><br></div><div>Fair enough. I have never worked on the Chromium codebase, so I don't know which parts follow the style guide and which parts to disregard.</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 class="im"><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>The basic reasoning for going with the slightly less popular style is this very basic example:<br>
</div>
<div><br></div><div><font face="courier new, monospace">aaaaa ? bbbbb :</font></div><div><font face="courier new, monospace"> ccccc;</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace">aaaaa ? bbbbb</font></div>
<div><font face="courier new, monospace"> : ccccc;</font></div><div><br></div><div>Here, the second formatting looks more structured and is easier to grasp at first sight. If the operands get more complex, this gets even more obvious. And for consistency with that formatting, ? and : should both be wrapped to the new line where necessary.</div>
</div></blockquote><div><br></div></div><div>The Google style guide generally directs line wrapping to begin at a 4-space indent. Indenting even with something on a previous line is generally used for lines of function arguments. So the correct way of formatting the first example would be:</div>
<div><br></div><div><font face="courier new, monospace">aaaaa ? bbbbb :</font></div><div><font face="courier new, monospace"> ccccc;</font></div><div><br></div><div>...which is generally less appealing than</div><div>
<br>
</div><div><font face="courier new, monospace">aaaaa ?</font></div><div><font face="courier new, monospace"> bbbbb : ccccc;</font></div><div><br></div><div>...which is why most Chrome code that needs to linebreak these at all does it this way. Whereas your indenting is not only unusual for breaking before an operator, it's also unusual for indenting even.</div>
</div></div></div></blockquote><div><br></div><div>That is not correct. The style guide says at a few very specific places that you have to indent four spaces. However, it does not even say 4 spaces with respect to what in all but simple cases. E.g. clang-format (and many other people - e.g. search for && at the end of a line), align the two operands of a binary expression no matter what. Again, this has been developed in cooperation with the style arbiters and a LOT of users.</div>
<div><br></div><div>Your own examples clarify why this is a good choice.</div><div><br></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>Finally, regardless of all other arguments, and even ignoring the fact that the above codesearch links included code they shouldn't: there's a way of formatting these in Chromium code that is clearly more common even by a pessimistic search, and is clearly compliant with the style guide. Therefore, the auto-formatter for Chromium code should use this pattern. It doesn't matter if your way is legal or not, the most important rule in the whole style guide is "be consistent", which this does not do as well as it could.</div>
</div></div></div></blockquote><div><br></div><div>I understand your argument and it is perfectly valid. However, also consider other sorts of consistency: E.g. there are a lot of users that have to develop in both Chromium- and Google-style code. For them, any (unnecessary) inconsistency is harmful.</div>
<div><br></div><div>I personally don't have any strong feelings about this (I for one would be happy with disallowing all multi-line conditional expressions). The decision to go this way is mostly that complex conditional expressions need as much structure as they can get. I know that we have other Chromium engineers that are happy enough with this. Is there a decision making process for Chromium style?</div>
<div><br></div><div>Cheers,</div><div>Daniel</div><div><br></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"><span class=""><font color="#888888"><div>PK </div></font></span></div></div></div>
</blockquote></div><br></div></div>