Ternary operator: break after or before?

[Peter Kasting]

Summary

The Chromium style guide and clang-format disagree about how to linebreak around the "?:" operator, and we'd like to resolve things.  Your opinion about which way to go is sought.

Background

The Google style guide does not give much guidance about linebreaking around operators (other than "be consistent when breaking boolean expressions").  In the early days of Chrome, we added a rule to our style guide to clarify this: basically, "break after operators, not before".  Thus, when linebreaking around "?:", almost all code did something like:

int x = foo ?

    a :

    b;

When clang-format began to be used, it introduced a new style of breaking before the "?:" operator, e.g.:

int x = foo

    ? a

    : b;

This form, while previously fairly unknown in the codebase, is now up to ~30% of cases, compared with ~70% for the other form**, due to the increasing popularity of clang-format.

clang-format actually has a flag to control which way it breaks around these operators, but Chromium doesn't explicitly toggle the flag.

Choice

I'm seeking to harmonize our practice by making one of the two above the officially-encouraged style for Chromium.  Here are the pros for each as I see them:

Break after ?:

+ Consistent with the great majority of existing code

+ Obeys explicit Chromium style recommendation (which was deemed important enough to preserve even when we removed most other exceptions from Google style)

+ Consistent with most binary operators, e.g. +, -, &&, etc., where we break after, not before

+ Highly subjective: Seems more readable to me, somewhat so to Nico

Break before ?:

+ Insofar as clang-format is the representation of "official Google style", more consistent with Google style

+ Almost no teams in Google bother to change the clang-format behavior, indicating that most internal teams are either OK with or apathetic toward this style

+ Would cause us to remove a "Chromium-specific" style rule from our guide

+ Highly subjective: May make the "?" and ":" more obvious by aligning them

Neutral:

* Since the Google style guide is silent on the issue, I assume our decision either way is not as important as something that has an explicit Google style rule

In short: Is one form more readable than the other?  If not, do we choose consistency with the majority of existing code and historical practice, or consistency with Google style?

Please opine.

PK

**Probably-lame regexes I used to measure:

Break before: https://code.google.com/p/chromium/codesearch#search/&q=%5E%5Cs*%5C?%20-file:third_party%20-file:usr/%20-file:v8&all=1&sq=package:chromium&type=cs

Break after: https://code.google.com/p/chromium/codesearch#search/&q=%5CS%5Cs%2B%5C?$%20-file:third_party%20-file:usr/%20-file:v8&all=1&sq=package:chromium&type=cs

[Dana Jansens]

On Fri, Dec 19, 2014 at 1:40 PM, 'Peter Kasting' via Chromium-dev <chromi...@chromium.org> wrote:

Summary

The Chromium style guide and clang-format disagree about how to linebreak around the "?:" operator, and we'd like to resolve things.  Your opinion about which way to go is sought.

Background

The Google style guide does not give much guidance about linebreaking around operators (other than "be consistent when breaking boolean expressions").  In the early days of Chrome, we added a rule to our style guide to clarify this: basically, "break after operators, not before".  Thus, when linebreaking around "?:", almost all code did something like:

int x = foo ?

    a :

    b;

When clang-format began to be used, it introduced a new style of breaking before the "?:" operator, e.g.:

int x = foo

    ? a

    : b;

When this first happened in a CL I reviewed my first impression was pleasant surprise at the alignment. I like the way clang-format does it.

This form, while previously fairly unknown in the codebase, is now up to ~30% of cases, compared with ~70% for the other form**, due to the increasing popularity of clang-format.

clang-format actually has a flag to control which way it breaks around these operators, but Chromium doesn't explicitly toggle the flag.

Choice

I'm seeking to harmonize our practice by making one of the two above the officially-encouraged style for Chromium.  Here are the pros for each as I see them:

Break after ?:

+ Consistent with the great majority of existing code

+ Obeys explicit Chromium style recommendation (which was deemed important enough to preserve even when we removed most other exceptions from Google style)

+ Consistent with most binary operators, e.g. +, -, &&, etc., where we break after, not before

+ Highly subjective: Seems more readable to me, somewhat so to Nico

Break before ?:

+ Insofar as clang-format is the representation of "official Google style", more consistent with Google style

+ Almost no teams in Google bother to change the clang-format behavior, indicating that most internal teams are either OK with or apathetic toward this style

+ Would cause us to remove a "Chromium-specific" style rule from our guide

+ Highly subjective: May make the "?" and ":" more obvious by aligning them

Neutral:

* Since the Google style guide is silent on the issue, I assume our decision either way is not as important as something that has an explicit Google style rule

In short: Is one form more readable than the other?  If not, do we choose consistency with the majority of existing code and historical practice, or consistency with Google style?

Please opine.

PK

**Probably-lame regexes I used to measure:

Break before: https://code.google.com/p/chromium/codesearch#search/&q=%5E%5Cs*%5C?%20-file:third_party%20-file:usr/%20-file:v8&all=1&sq=package:chromium&type=cs

Break after: https://code.google.com/p/chromium/codesearch#search/&q=%5CS%5Cs%2B%5C?$%20-file:third_party%20-file:usr/%20-file:v8&all=1&sq=package:chromium&type=cs

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Ryan Sleevi]

On Fri, Dec 19, 2014 at 1:40 PM, 'Peter Kasting' via Chromium-dev <chromi...@chromium.org> wrote:

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Thanks for the excellent summary of the objective issues.

Since I have nothing to add to the objective criteria, but plenty of opine, my subjective criteria is that I prefer 1 (after) over 2.  

[Scott Graham]

I find the current clang-format style much more readable, even though it "feels" inconsistent with putting &&, etc. on the previous line.

On Fri, Dec 19, 2014 at 1:40 PM, 'Peter Kasting' via Chromium-dev <chromi...@chromium.org> wrote:

--

[Daniel Cheng]

I personally find ?: at the beginning to more readable, since it's immediately obvious it's part of a ternary operator expression.

Daniel

[Jeffrey Yasskin]

I like the "then" and "else" ("?" and ":") written next to the thing
that will happen "then" or "else", which is how "before" works, but I
also don't think this is important enough to send more than one
message about.

On Fri, Dec 19, 2014 at 1:40 PM, 'Peter Kasting' via Chromium-dev
<chromi...@chromium.org> wrote:

[James Cook]

I like ?: at the beginning.

Also, my understanding is that this behavior is to follow the formatting of infix-dot, like Builder().Foo().Bar(). I strongly prefer those dot operators to be at the beginning, as clang-format does it now. I presume the dot-operator formatting is not a subject of this discussion.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

[Peter Kasting]

On Fri, Dec 19, 2014 at 3:27 PM, James Cook <jame...@chromium.org> wrote:

Also, my understanding is that this behavior is to follow the formatting of infix-dot, like Builder().Foo().Bar(). I strongly prefer those dot operators to be at the beginning, as clang-format does it now. I presume the dot-operator formatting is not a subject of this discussion.

I don't think the ?: and . operators have anything to do with each other, so I don't think wrapping rules about them are, or should be, intended to be in sync (at least not more than with other operators).

PK 

[Chris Hopman]

I like ?: at the beginning. I think for the example here it doesn't really matter (though the example here should probably be on just one line). Maybe we should give some real, more complicated examples of what the formatting looks like.

--

[Thiago Farina]

On Fri, Dec 19, 2014 at 10:16 PM, Chris Hopman <cjho...@chromium.org> wrote:

I like ?: at the beginning. I think for the example here it doesn't really matter (though the example here should probably be on just one line). Maybe we should give some real, more complicated examples of what the formatting looks like.

I have one that fits pretty well for this thread:

https://codereview.chromium.org/815583004/diff/1/ui/app_list/pagination_model.cc#newcode240

--

Thiago Farina

[Peter Kasting]

On Fri, Dec 19, 2014 at 4:16 PM, Chris Hopman <cjho...@chromium.org> wrote:

I like ?: at the beginning. I think for the example here it doesn't really matter (though the example here should probably be on just one line). Maybe we should give some real, more complicated examples of what the formatting looks like.

Here are examples from code reviews I did today:

clang-format (break before) vs. my suggestion (break after) Example 1:

  infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(

      scoped_ptr<ConfirmInfoBarDelegate>(new OutdatedPluginInfoBarDelegate(

          installer, plugin_metadata.Pass(),

          l10n_util::GetStringFUTF16(

              (installer->state() == PluginInstaller::INSTALLER_STATE_IDLE)

                  ? IDS_PLUGIN_OUTDATED_PROMPT

                  : IDS_PLUGIN_DOWNLOADING,

name)))));

  infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(

      scoped_ptr<ConfirmInfoBarDelegate>(new OutdatedPluginInfoBarDelegate(

          installer, plugin_metadata.Pass(),

          l10n_util::GetStringFUTF16(

              (installer->state() == PluginInstaller::INSTALLER_STATE_IDLE) ?

                  IDS_PLUGIN_OUTDATED_PROMPT : IDS_PLUGIN_DOWNLOADING,

name)))));

Example 2:

  const int x_pos =

      (base::i18n::IsRTL()

          ? (screen_bounds.x() + bubble_half_width + kFullscreenPaddingEnd)

          : (screen_bounds.right() - bubble_half_width -

             kFullscreenPaddingEnd));

  const int x_pos = base::i18n::IsRTL() ?

      (screen_bounds.x() + bubble_half_width + kFullscreenPaddingEnd) :

      (screen_bounds.right() - bubble_half_width - kFullscreenPaddingEnd);

PK

[Chris Hopman]

Here's some examples of ?: at the end:

1. new_trace.set_characters_preloaded(preload->preload_is_current_ ?
2. preload->preload_characters_ :
3. 0);
4.  
5.  
6.  
7. return (ContrastRatio(RelativeLuminance(foreground), background_luminance) >=
8. ContrastRatio(RelativeLuminance(foreground2), background_luminance)) ?
9. foreground : foreground2;
10.  
11.  
12.  
13. bottom_left_painter_->SetClipRect(
14. LayerExceedsSize(bottom_left_layer_.get(), gfx::Size(left, bottom)) ?
15. gfx::Rect(0, bottom_left_layer_->bounds().height() - bottom,
16. left, bottom) :
17. gfx::Rect(),
18. bottom_left_layer_.get());
19.  
20.  
21.  
22. LOperand* temp3 =
23. ((divisor > 0 && !instr->CheckFlag(HValue::kLeftCanBeNegative)) ||
24. (divisor < 0 && !instr->CheckFlag(HValue::kLeftCanBePositive))) ?
25. NULL : TempRegister();
26.  
27.  
28.  
29. i::Handle<i::Script> script_object = compiled_script->IsJSFunction() ?
30. i::Handle<i::Script>(i::Script::cast(
31. i::JSFunction::cast(*compiled_script)->shared()->script())) :
32. i::Handle<i::Script>(i::Script::cast(
33. i::SharedFunctionInfo::cast(*compiled_script)->script()));
34.  
35.  
36.  
37. return obj->IsString() ?
38. scope.Escape(Local<String>::Cast(Utils::ToLocal(obj))) :
39. Local<String>();
40.  
41.  
42.  
43. int call_function_arg_count = is_call_ic ?
44. CallICStub::ExtractArgcFromMinorKey(CodeStub::MinorKeyFromKey(key)) :
45. CallFunctionStub::ExtractArgcFromMinorKey(
46. CodeStub::MinorKeyFromKey(key));
47.  
48.  
49.  
50. FunctionLiteral::IsParenthesizedFlag parenthesized = parenthesized_function_ ?
51. FunctionLiteral::kIsParenthesized :
52. FunctionLiteral::kNotParenthesized;

And the same code with them at the beginning:

1. new_trace.set_characters_preloaded(preload->preload_is_current_
2. ? preload->preload_characters_
3. : 0);
4.  
5.  
6.  
7. return (ContrastRatio(RelativeLuminance(foreground), background_luminance) >=
8. ContrastRatio(RelativeLuminance(foreground2), background_luminance))
9. ? foreground : foreground2;
10.  
11.  
12.  
13. bottom_left_painter_->SetClipRect(
14. LayerExceedsSize(bottom_left_layer_.get(), gfx::Size(left, bottom))
15. ? gfx::Rect(0, bottom_left_layer_->bounds().height() - bottom,
16. left, bottom)
17. : gfx::Rect(),
18. bottom_left_layer_.get());
19.  
20.  
21.  
22. LOperand* temp3 =
23. ((divisor > 0 && !instr->CheckFlag(HValue::kLeftCanBeNegative)) ||
24. (divisor < 0 && !instr->CheckFlag(HValue::kLeftCanBePositive)))
25. ? NULL : TempRegister();
26.  
27.  
28.  
29. i::Handle<i::Script> script_object = compiled_script->IsJSFunction()
30. ? i::Handle<i::Script>(i::Script::cast(
31. i::JSFunction::cast(*compiled_script)->shared()->script()))
32. : i::Handle<i::Script>(i::Script::cast(
33. i::SharedFunctionInfo::cast(*compiled_script)->script()));
34.  
35.  
36.  
37. return obj->IsString()
38. ? scope.Escape(Local<String>::Cast(Utils::ToLocal(obj)))
39. : Local<String>();
40.  
41.  
42.  
43. int call_function_arg_count = is_call_ic
44. ? CallICStub::ExtractArgcFromMinorKey(CodeStub::MinorKeyFromKey(key))
45. : CallFunctionStub::ExtractArgcFromMinorKey(
46. CodeStub::MinorKeyFromKey(key));
47.  
48.  
49.  
50. FunctionLiteral::IsParenthesizedFlag parenthesized = parenthesized_function_
51. ? FunctionLiteral::kIsParenthesized
52. : FunctionLiteral::kNotParenthesized;

After actually looking at a bunch of real example, the ?: at the beginning looks really bad to me in several cases. I think in some cases I prefer ?: at the beginning slightly more, but in others I greatly prefer ?: at the end.

My vote would now be for ?: to go at the end.

[Chris Hopman]

In your suggestion first example (and in several of mine), the two cases are on the same line (but broken from the condition). I really prefer that when it fits, do we know if clang-format supports that?

[Peter Kasting]

On Fri, Dec 19, 2014 at 4:33 PM, Chris Hopman <cjho...@chromium.org> wrote:

In your suggestion first example (and in several of mine), the two cases are on the same line (but broken from the condition). I really prefer that when it fits, do we know if clang-format supports that?

As far as I can tell, when clang-format decides to break before '?', it always also breaks before ':', to line the two characters up.

This is one of the reasons I personally strongly dislike clang-format's behavior.  I don't know offhand what it would do if we toggled the behavior switch.

PK

[Ryan Hamilton]

On Fri, Dec 19, 2014 at 4:30 PM, Chris Hopman <cjho...@chromium.org> wrote:

After actually looking at a bunch of real example, the ?: at the beginning looks really bad to me in several cases. I think in some cases I prefer ?: at the beginning slightly more, but in others I greatly prefer ?: at the end.

My vote would now be for ?: to go at the end.

Personally, I prefer ?: at the beginning in all of the examples you included ​(modulo the weird line wrapping which I suspect is an artifact of some sort)​

.​

​

[Peter Kasting]

I believe the line wrapping in all his examples is exactly what clang-format does today.  What did you find "weird"? 

[Chris Hopman]

The two cases should have the same line wrapping. I blame various bits of email infrastructure if they don't.

You can also see them here:

http://ideone.com/DPM9d5

http://ideone.com/IDFOnI

[Ryan Hamilton]

​I'm not sure if this is a screen width thing, but I see lines being wrapped (in the email) before the end of the line (based on line number):

​I assume this is some artifact of email. But mentally undoing this, I prefer the ?: at the beginning of the line :>​

[Peter Kasting]

On Fri, Dec 19, 2014 at 4:42 PM, Ryan Hamilton <r...@chromium.org> wrote:

​I'm not sure if this is a screen width thing, but I see lines being wrapped (in the email) before the end of the line (based on line number):

Ah.  Yeah, I'm not seeing that.  I would suspect screen width or similar.

PK 

[Peter Kasting]

Thread summary so far:

* Break before: 6 votes (danakj, sgraham, dcheng, jyasskin, jamescook, rch)

* Break after: 4 votes (pkasting, rsleevi, tsepez, chopman)

Interestingly, no one has opined on things like "consistency with Google style" or "consistency with Chromium style" or in general anything other than "I like way X better".  This makes it somewhat hard for me to decide what to do if the vote ends up being reasonably even.  It would be nice to hear which of these principles people think should govern.  For example, Nico has opined outside this thread that he thinks "consistency with Google style" should trump everything, while I've suggested that "Chromium codebase and style guide say X" is more important than anything else.

After all, if everyone thinks X is better than Y, we could just do X.  But if people think that we should hew to <some other principle> unless all other things are equal, then that principle can guide us when the preferred style isn't broadly agreed-upon.

PK

[Hendrik Wagenaar]

If there's a vote, put me down for +1 before.

infobar_service->AddInfoBar(infobar_service->CreateConfirmInfoBar(

      scoped_ptr<ConfirmInfoBarDelegate>(new OutdatedPluginInfoBarDelegate(

          installer, plugin_metadata.Pass(),

          l10n_util::GetStringFUTF16(

              (installer->state() == PluginInstaller::INSTALLER_STATE_IDLE) ?

                  IDS_PLUGIN_OUTDATED_PROMPT : IDS_PLUGIN_DOWNLOADING,

name)))));

This would be clearer if it wasn't written as one giant statement. The location of the ?: won't make much of an improvement.

[Ryan Hamilton]

On Fri, Dec 19, 2014 at 4:52 PM, 'Peter Kasting' via Chromium-dev <chromi...@chromium.org> wrote:

Thread summary so far:

* Break before: 6 votes (danakj, sgraham, dcheng, jyasskin, jamescook, rch)

* Break after: 4 votes (pkasting, rsleevi, tsepez, chopman)

Interestingly, no one has opined on things like "consistency with Google style" or "consistency with Chromium style" or in general anything other than "I like way X better".  This makes it somewhat hard for me to decide what to do if the vote ends up being reasonably even.  It would be nice to hear which of these principles people think should govern.  For example, Nico has opined outside this thread that he thinks "consistency with Google style" should trump everything, while I've suggested that "Chromium codebase and style guide say X" is more important than anything else.

​Ah. Well, good point. :> As someone who is responsible for code which is shared between the internal code repository and Chromium, I'm strongly prefer that Google style and Chromium style align as much as possible. Where we have clang-format pre-submit checks which attempt to enforce style which is contradictory to Google clang-format style it introduces significant maintenance headaches.

​

 

After all, if everyone thinks X is better than Y, we could just do X.  But if people think that we should hew to <some other principle> unless all other things are equal, then that principle can guide us when the preferred style isn't broadly agreed-upon.

​Fine, fine... rain in my "lets do what I want because I want to" parade :>​

[Daniel Cheng]

In general, I don't think "consistency with the existing style" should be a governing principle, because the style guide has, and will continue, to change over time. I'd also prefer to see fewer diffs between Google and Chromium style, both in the actual style guide text and the implementation of clang-format.

Daniel

--

[Steve Kobes]

In Google C++ code (outside of Chromium), breaking after ?: is slightly more common, but not dramatically so.

' (\?) *$'    57%

'^ *(\?) '    43%

Since the Google style guide is silent on the issue, "consistency" dictates that Chromium should be silent on it also.

[Nico Weber]

On Fri, Dec 19, 2014 at 6:00 PM, Steve Kobes <sko...@chromium.org> wrote:

In Google C++ code (outside of Chromium), breaking after ?: is slightly more common, but not dramatically so.

' (\?) *$'    57%

'^ *(\?) '    43%

Since the Google style guide is silent on the issue, "consistency" dictates that Chromium should be silent on it also.

(FWIW, here's an explicit "here's why clang-format does what it does" FAQ entry at https://goto.google.com/ternary (internal-only link, sorry) that's been ok'd by the style arbiters. So it's not an omission by accident.)

[Ryan Hamilton]

On Fri, Dec 19, 2014 at 6:16 PM, Nico Weber <tha...@chromium.org> wrote:

On Fri, Dec 19, 2014 at 6:00 PM, Steve Kobes <sko...@chromium.org> wrote:

In Google C++ code (outside of Chromium), breaking after ?: is slightly more common, but not dramatically so.

' (\?) *$'    57%

'^ *(\?) '    43%

Since the Google style guide is silent on the issue, "consistency" dictates that Chromium should be silent on it also.

(FWIW, here's an explicit "here's why clang-format does what it does" FAQ entry at https://goto.google.com/ternary (internal-only link, sorry) that's been ok'd by the style arbiters. So it's not an omission by accident.)

​To confirm, both "Google" clang-format and Chromium clang-format will do the same thing? SGTM.​

 

[Dana Jansens]

On Dec 19, 2014 4:52 PM, "'Peter Kasting' via Chromium-dev" <chromi...@chromium.org> wrote:
>
> Thread summary so far:
>
> * Break before: 6 votes (danakj, sgraham, dcheng, jyasskin, jamescook, rch)
> * Break after: 4 votes (pkasting, rsleevi, tsepez, chopman)
>
> Interestingly, no one has opined on things like "consistency with Google style" or "consistency with Chromium style" or in general anything other than "I like way X better".  This makes it somewhat hard for me to decide what to do if the vote ends up being reasonably even.  It would be nice to hear which of these principles people think should govern.  For example, Nico has opined outside this thread that he thinks "consistency with Google style" should trump everything, while I've suggested that "Chromium codebase and style guide say X" is more important than anything else.

I found the "consistency with other operators" and "consistency with google code" to have approximately equal value, and don't put as much value on the rest. That left me not feeling strongly in either direction, so I just commented on my personal preference.

>
> After all, if everyone thinks X is better than Y, we could just do X.  But if people think that we should hew to <some other principle> unless all other things are equal, then that principle can guide us when the preferred style isn't broadly agreed-upon.
>
> PK
>

[Krzysztof Olczyk]

I don't think it's possible to come up with objective facts supporting any of the options here.

 

So just another subjective voice for putting ? and : at the beginning, before the respective lines. This makes the code, personally to me, much more readable.

For me, 

auto x = foo ?

             a :

             b;

looks like

if (foo)

  x = a; else

  x = b;

-- 

Best regards,

Krzysztof Olczyk

Opera Software ASA

TV & Devices

[Devlin Cronin]

FWIW, my preference is for 1) (breaking after the ternary).  I don't think it's massively more readable either way, but I'd like to be consistent with the style that we've had (and still have in the majority of code).  Further, this isn't a C++-only style, and I know it's come up before in, e.g., JS reviews.  Since we can't just take the "do git cl format and forget about it" approach with all code, it seems strange to decide to move away from the status quo when there isn't (in my opinion) a truly concrete reason to do so.

[Dana Jansens]

On Dec 22, 2014 8:29 AM, "Devlin Cronin" <rdevlin...@chromium.org> wrote:
>
> FWIW, my preference is for 1) (breaking after the ternary).  I don't think it's massively more readable either way, but I'd like to be consistent with the style that we've had (and still have in the majority of code).  Further, this isn't a C++-only style, and I know it's come up before in, e.g., JS reviews.  Since we can't just take the "do git cl format and forget about it" approach with all code, it seems strange to decide to move away from the status quo when there isn't (in my opinion) a truly concrete reason to do so.

I think "this is how most google code is formatted" is pretty concrete?

[Devlin Cronin]

On Mon, Dec 22, 2014 at 8:36 AM, Dana Jansens <dan...@chromium.org> wrote:

On Dec 22, 2014 8:29 AM, "Devlin Cronin" <rdevlin...@chromium.org> wrote:
>
> FWIW, my preference is for 1) (breaking after the ternary).  I don't think it's massively more readable either way, but I'd like to be consistent with the style that we've had (and still have in the majority of code).  Further, this isn't a C++-only style, and I know it's come up before in, e.g., JS reviews.  Since we can't just take the "do git cl format and forget about it" approach with all code, it seems strange to decide to move away from the status quo when there isn't (in my opinion) a truly concrete reason to do so.

I think "this is how most google code is formatted" is pretty concrete?

Well, I did specify it with "in my opinion." :)  Perhaps "concrete" wasn't the right word - there are objective and measurable reasons for both cases (chromium code consistency vs google code consistency), but I don't think that "this is how Google does it" is a very compelling reason.  Chromium has had its own style for years, even if it mostly matches Google style, and I don't think that matching Google style exactly is necessary (again, my opinion).  I can appreciate the benefit of one unified style throughout, but each style change like this we make to the chromium style has an inherit cost that's far greater than just putting a break in a different spot.  We all have to adjust to the new style, push back against reviewers who haven't heard the latest revision, ensure that we know to do this for all languages (not just those that clang format can manage), etc.  As such, I will generally weigh in favor of maintaining the current style within Chromium code, unless there's a reason beyond "this is what other teams do".

[Nico Weber]

On Mon, Dec 22, 2014 at 8:28 AM, Devlin Cronin <rdevlin...@chromium.org> wrote:

FWIW, my preference is for 1) (breaking after the ternary).  I don't think it's massively more readable either way, but I'd like to be consistent with the style that we've had (and still have in the majority of code).  Further, this isn't a C++-only style, and I know it's come up before in, e.g., JS reviews.  Since we can't just take the "do git cl format and forget about it" approach with all code

(clang-format can format .js in theory, all it takes is adding '.js' to CLANG_EXTS in depot_tools/git_cl.py. I haven't tried how it does on our js, but it's possible that it's less work to get it working for js code than it was to get it working for java code. It also already does .proto files in addition to c++, objective-c, and java.)

 

, it seems strange to decide to move away from the status quo when there isn't (in my opinion) a truly concrete reason to do so.

--

[Ricardo Vargas]

+1 for #1 because... as far as I remember there used to be an explicit rule on the style guide saying that lines should break after an operator, not before (when needed). Now the only mention about it is for boolean operators, but I think it is better to have a single exception, if needed, instead of either multiple exceptions or do anything.

So unless we want to move all operators to the start of a line, having ? : at the end is more consistent with the rest of the code.

[Daniel Cheng]

I think it's worth pointing out that there's already for the dot operator, the arrow operator (this one is a bit contentious IIRC), and <<.

Daniel

[Daniel Cheng]

I think it's worth pointing out that there's already /exceptions/ for the dot operator, the arrow operator (this one is a bit contentious IIRC), and <<.

randomly out words today, sorry

Daniel

[Peter Kasting]

Thread summary so far:

* Break before: 8 (danakj, sgraham, dcheng, jyasskin, jamescook, rch, hendrikw, kolczyk@opera)

* Break after: 6 (pkasting, rsleevi, tsepez, chopman, rdevlin.cronin, rvargas)

* Don't say anything (doesn't answer what clang-format should do): 1 (skobes)

Comments in favor of various principles:

* Be consistent with Google code/style: 3? (rch, dcheng, danakj; possibly skobes)

* Be consistent with Chromium code: 2 (pkasting, rdevlin.cronin)

* Be consistent with other operators: 3 (pkasting, danakj, rvargas)

* Be consistent with other languages, e.g. JS: 2 (tsepez, rdevlin.cronin)

So far, as I somewhat expected, there is no clear runaway viewpoint, which is awkward, because then almost anything we decide to do is going to make some people unhappy.  More people have supported "break before" directly, but principle-wise, more people have expressed support for principles that would promote "break after".

(And as with all bikesheds, this has probably already taken up more time and energy than it's worth.)

PK

[Peter Kasting]

On Mon, Dec 22, 2014 at 4:01 PM, Daniel Cheng <dch...@chromium.org> wrote:

I think it's worth pointing out that there's already for the dot operator, the arrow operator (this one is a bit contentious IIRC), and <<.

(I think dot and arrow are equally contentious -- for example, I find clang-format's behavior equally wrong with both of them.  That said, I'm much more willing to live with the bizarre behavior there than for ?:.  But if I had my druthers, yes, we'd break after every operator including dot and arrow, except "<<", which would be different only because our style guide has long had an explicit rule saying to align "<<"s on subsequent lines, in which case it's not only the breaking but the indentation as well that's special for that operator.)

PK

[Rachel Blum]

Since we're tallying votes: preferring break after ?. 

It matters to me in terms of readability: I know what the code does right on line 1 - it's a ternary - and can choose to mentally parse line 2&3 depending on need. Break before forces me to read a 2nd line. (My brain seems to parse line-based). With one exception. If we can't fix clang-format to obey break-after, I'd rather break-before than not being able to auto-format. 

- Rachel

--

[Peter Kasting]

I never wrapped up this thread.

Sentiment overall is almost evenly split, so I'm going to make an executive decision:

Clang-format behavior will be left as-is (break before).  Both styles will be officially allowed in Chromium code.  The Chromium style guide gives examples.

Thanks for everyone's feedback.

PK

[Yuri Wiitala]

A third option (the ? after the test expression w/o whitespace).  Example:

  int x = is_value_set? value : 0;

I've seen a lot of code, albeit outside Google, that formats this way.

--

[Nico Weber]

On Thu, Feb 19, 2015 at 8:56 PM, Yuri Wiitala <m...@chromium.org> wrote:

A third option (the ? after the test expression w/o whitespace).  Example:

  int x = is_value_set? value : 0;

I've seen a lot of code, albeit outside Google, that formats this way.

While this is an option in that it's theoretically possible to format ?: this way, it's not a valid option for formatting chromium code :-)

(There's virtually zero usage of that in Chromium code: https://code.google.com/p/chromium/codesearch#search/&q=%5E%5B%5E/%22%5D*%5Cw%5C?.*:%20file:%5C.cc&sq=package:chromium&type=cs – where "virtually zero" is "two places", plus a handful of external thingies. These look like oversights.)

[Matthew Dempsky]

On Fri, Feb 20, 2015 at 1:56 PM, Yuri Wiitala <m...@chromium.org> wrote:

A third option (the ? after the test expression w/o whitespace).  Example:

  int x = is_value_set? value : 0;

I've seen a lot of code, albeit outside Google, that formats this way.

Indeed, the advantage of omitting the space before the question mark is it looks more like written English.  Except obviously then it should also be followed by two spaces, and spaces don't belong before colons either:

[Alex Vakulenko]

Let's just stick with what Peter wrote :)

--