Should &&/|| really be at the end of lines?

[gsqu...@mozilla.com]

Question of the day:
When breaking overlong expressions, should &&/|| go at the end or the beginning of the line?

TL;DR: Coding style says 'end', I&others think we should change it to 'beginning' for better clarity, and consistency with other operators.


Our coding style reads:
"Break long conditions after && and || logical connectives. See below for the rule for other operators." [1]
"""
Overlong expressions not joined by && and || should break so the operator starts on the second line and starts in the same column as the start of the expression in the first line. This applies to ?:, binary arithmetic operators including +, and member-of operators (in particular the . operator in JavaScript, see the Rationale).

Rationale: operator at the front of the continuation line makes for faster visual scanning, because there is no need to read to end of line. Also there exists a context-sensitive keyword hazard in JavaScript; see bug 442099, comment 19, which can be avoided by putting . at the start of a continuation line in long member expression.
""" [2]


I initially focused on the rationale, so I thought *all* operators should go at the front of the line.

But it seems I've been living a lie!
&&/|| should apparently be at the end, while other operators (in some situations) should be at the beginning.


Now I personally think this just doesn't make sense:
- Why the distinction between &&/|| and other operators?
- Why would the excellent rationale not apply to &&/||?
- Pedantically, the style talks about 'expression *not* joined by &&/||, but what about expression that *are* joined by &&/||? (Undefined Behavior!)

Based on that, I believe &&/|| should be made consistent with *all* operators, and go at the beginning of lines, aligned with the first operand above.

And therefore I would propose the following changes to the coding style:
- Remove the lonely &&/|| sentence at [1].
- Rephrase the first sentence at [2] to something like: "Overlong expressions should break so that the operator starts on the following line, in the same column as the first operand for that operator. This applies to all binary operators, including member-of operators (in particular the . operator in JavaScript, see the Rationale), and extends to ?: where the 2nd and third operands should be on separate lines and start in the same column as the first operand."
- Keep the rationale at [2].

Also, I think we should add something about where to break expressions with operators of differing precedences, something like: "Overlong expressions containing operators of differing precedences should first be broken at the operator of lowest precedence. E.g.: 'a+b*c' should be split at '+' before '*'"


A bit more context:
Looking at the history of the coding style page, a certain "Brendan" wrote that section in August 2009 [3], shortly after a discussion here [4] that seemed to focus on the dot operator in Javascript. In that discussion, &&/|| appear in examples at the end of lines and nobody talks about that (because it was not the main subject, and/or everybody agreed with it?)

Discuss!


[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators
[3] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style$compare?locale=en-US&to=7315&from=7314
[4] https://groups.google.com/d/msg/mozilla.dev.platform/Ji9lxlLCYME/zabUmQI9S-sJ

[Jeff Gilbert]

I don't visually like ||/&& at start of line, but I can't fault the
reasoning, so I'm weakly for it.
I don't think it's important enough to change existing code though.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[gsqu...@mozilla.com]

There's an ongoing effort to use clang-format to automatically format All The Codes [1], so I think we should try and make sure we agree (or at least settle*) on a coding style when it's going to be enforced everywhere. ;-)

* I'm presenting my own preferences here, but in the end I'm fine with complying with whatever we can all agree to live with. Consistence is more important than personal preferences.

g.

[Jean-Yves Avenard]

Hi

On 16/02/17 23:56, Jeff Gilbert wrote:
> I don't visually like ||/&& at start of line, but I can't fault the
> reasoning, so I'm weakly for it.
> I don't think it's important enough to change existing code though.

Disclaimer: I'm now biased about this.

I've been writing C and C++ code for now most of my life. And during
that time you take strong habits. Not always good habits, but there are
there.

Now having operators at the end of the line, including logical ones like
&& and || is something I've always done, because it's just the way
things were done.
Never really thought much about it, you place the operator at the end of
the line when you're splitting a long logical operations.

but is it the proper things to do?
Just like Gerald above, I had misread (or more accurately understood
differently) what the rules were in that document, because there's
various discrepancies in it.

When you read code, especially during review, the review process is made
much more easily when you can at a glance understand one particular
logical calculation.

reading this: (turn on courier)

return ((aCodecMask & VPXDecoder::VP8)
&& aMimeType.EqualsLiteral("video/webm; codecs=vp8"))
|| ((aCodecMask & VPXDecoder::VP9)
&& aMimeType.EqualsLiteral("video/webm; codecs=vp9"))
|| ((aCodecMask & VPXDecoder::VP9)
&& aMimeType.EqualsLiteral("video/vp9"));

than:
return ((aCodecMask & VPXDecoder::VP8) &&
aMimeType.EqualsLiteral("video/webm; codecs=vp8")) ||
((aCodecMask & VPXDecoder::VP9) &&
aMimeType.EqualsLiteral("video/webm; codecs=vp9")) ||
((aCodecMask & VPXDecoder::VP9) &&
aMimeType.EqualsLiteral("video/vp9"));

where does the || apply, where does the && ?
I must recompose the entire expression in my mind to understand what's
going on.
The previous one require no such mental process.

If we are talking about new code, the question becomes, can this become
the rule *and* can we add such rule to clang-format
seeing that we're in the process of applying clang-format to the entire
source tree (with exceptions), modifying existing code would become a
moot issue.

Cheers
JY

> On Thu, Feb 16, 2017 at 1:47 PM, <gsqu...@mozilla.com> wrote:

[Jeff Gilbert]

I would not assume that's necessarily going to happen without
contention. Consistency is not a goal in and of itself.

[gsqu...@mozilla.com]

Forgot the link:
[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1188202

[Jeff Gilbert]

Both are readable with judicious indentation on wrapping. If you must
recompose the entire expression in your mind, the code is too terse,
and we should consider cleaning it up and making it more readable.
&&/|| position will not make or break this.

On Thu, Feb 16, 2017 at 3:10 PM, Jean-Yves Avenard
<jyav...@mozilla.com> wrote:
> Hi
>
>

> On 16/02/17 23:56, Jeff Gilbert wrote:
>>
>> I don't visually like ||/&& at start of line, but I can't fault the
>> reasoning, so I'm weakly for it.
>> I don't think it's important enough to change existing code though.
>
>

>> On Thu, Feb 16, 2017 at 1:47 PM, <gsqu...@mozilla.com> wrote:
>>>

[Mike Hommey]

On Fri, Feb 17, 2017 at 12:10:32AM +0100, Jean-Yves Avenard wrote:
> reading this: (turn on courier)
>
> return ((aCodecMask & VPXDecoder::VP8)
> && aMimeType.EqualsLiteral("video/webm; codecs=vp8"))
> || ((aCodecMask & VPXDecoder::VP9)
> && aMimeType.EqualsLiteral("video/webm; codecs=vp9"))
> || ((aCodecMask & VPXDecoder::VP9)
> && aMimeType.EqualsLiteral("video/vp9"));
>
> than:
> return ((aCodecMask & VPXDecoder::VP8) &&
> aMimeType.EqualsLiteral("video/webm; codecs=vp8")) ||
> ((aCodecMask & VPXDecoder::VP9) &&
> aMimeType.EqualsLiteral("video/webm; codecs=vp9")) ||
> ((aCodecMask & VPXDecoder::VP9) &&
> aMimeType.EqualsLiteral("video/vp9"));
>
> where does the || apply, where does the && ?
> I must recompose the entire expression in my mind to understand what's going
> on.
> The previous one require no such mental process.

... assuming the indentation and use of parens is correct, which it
might not be, so you still have to look closely.

Which brings an interesting observation: the review tools are not
helpful in that situation, while a local editor would allow you to know
which closing parens corresponds to which opening one.

Mike

[Jeff Gilbert]

If there are so many parens that we require an editor to disambiguate
them, it's too dense. Add whitespace or decompose the expression until
readable.

[gsqu...@mozilla.com]

Jeff, I see you're opinionated against consistency. :-)
Or you think there are other more important things?
Anyway, I'm not exactly sure what you're advocating. Could you please elaborate?

[Jean-Yves Avenard]

On 17/02/17 00:17, Jeff Gilbert wrote:
> If there are so many parens that we require an editor to disambiguate
> them, it's too dense. Add whitespace or decompose the expression until
> readable.
>

I'd love to know how adding any whitespaces could make any logical
expressions clearer that what I posted above.

That's the point, there's no more whitespace to add, nor a need to
decompose further, it's already been fully expanded.

[L. David Baron]

On Thursday 2017-02-16 15:11 -0800, Jeff Gilbert wrote:
> I would not assume that's necessarily going to happen without
> contention. Consistency is not a goal in and of itself.

Using clang-format on the entire tree has the massive value of:

* reviewers not needing to point out whitespace and indentation issues

* reduced friction for new contributors (being able to use a tool
instead of repeatedly iterating on review comments)

* reduced friction for existing contributors working in new areas
of the codebase

which outweighs indvidual style preferences or historic module
differences here.

We should try to do something as close as reasonably possible to the
existing dominant style so that we disrupt annotate/blame the least.


As for the proposal to switch to start-of-line && and || -- I
wouldn't want to see us try to make such a change *before* we
clang-format the entire tree, because we'll just create an
inconsistent mess that makes it harder to figure out what the local
style is. I'd be slightly more willing to consider it *after* we
clang-format the whole tree (or simultaneously, which is perhaps
better for blame since there's only one revision to skip), but then
we need to balance it against the cost of the increased disruption
to blame that makes understanding the history of our code harder.

-David

> On Thu, Feb 16, 2017 at 3:04 PM, <gsqu...@mozilla.com> wrote:

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla https://www.mozilla.org/ 𝄂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)

[Jeff Gilbert]

I'll not get into why I disagree with your points about
format-the-world here. Let's leave that for elsewhere.

Here's my take on the example at hand:
https://pastebin.mozilla.org/8979527
In this case, I do like the latter two best. (Old-style and simplification)

[Robert O'Callahan]

On Fri, Feb 17, 2017 at 12:25 PM, L. David Baron <dba...@dbaron.org> wrote:
> Using clang-format on the entire tree has the massive value of:
>
> * reviewers not needing to point out whitespace and indentation issues
>
> * reduced friction for new contributors (being able to use a tool
> instead of repeatedly iterating on review comments)
>
> * reduced friction for existing contributors working in new areas
> of the codebase
>
> which outweighs indvidual style preferences or historic module
> differences here.

Also, it's very liberating not having to think about formatting while
writing code, knowing it will be automatically fixed up later. This is
especially helpful for partially-automated changes such as mass
find-and-replace.

Rob
--
lbir ye,ea yer.tnietoehr rdn rdsme,anea lurpr edna e hnysnenh hhe uresyf toD
selthor stor edna siewaoeodm or v sstvr esBa kbvted,t rdsme,aoreseoouoto
o l euetiuruewFa kbn e hnystoivateweh uresyf tulsa rehr rdm or rnea lurpr
.a war hsrer holsa rodvted,t nenh hneireseoouot.tniesiewaoeivatewt sstvr esn

[gsqu...@mozilla.com]

Thank you David.
Your opinion about any *future* change to &&/|| style would be appreciated. ;-)

About "increased disruption to blame that makes understanding the history of our code harder":
I think some tools like http://searchfox.org/ have considerably lessened the impact to blame, because they make it so easy to skip over style changes.
E.g., in SearchFox, hover over the gray blame bar to the left, and click "Show latest version without this line", done!

[Martin Thomson]

On Fri, Feb 17, 2017 at 10:39 AM, Robert O'Callahan
<rob...@ocallahan.org> wrote:
>> Using clang-format on the entire tree has the massive value of:
>

> Also, it's very liberating not having to think about formatting while
> writing code, knowing it will be automatically fixed up later. This is
> especially helpful for partially-automated changes such as mass
> find-and-replace.

NSS recently reached this point. Formatting is checked in CI and both
David and roc are right about the benefits. We even have a pre-commit
hook now that catches errors early.

It was only mildly disruptive getting there, but worth it. Firefox
is, of course, bigger and more diverse, but I don't see why it
couldn't eventually reach the same point.

[Jeff Gilbert]

Let's drop this side-topic from this thread.

[Nicholas Nethercote]

I personally have a strong preference for operators at the end of lines.
The codebase seems to agree with me, judging by some rough grepping ('fff'
is an alias I have that's roughly equivalent to rgrep):

$ fff "&&$" | wc -l
28907
$ fff "^ *&&" | wc -l
3751

$ fff "||$" | wc -l
26429
$ fff "^ *||" | wc -l
2977

$ fff " =$" | wc -l
39379
$ fff "^ *= " | wc -l
629

$ fff " +$" | wc -l
31909
$ fff "^ *+$" | wc -l
491

$ fff " -$" | wc -l
2083
$ fff "^ *-$" | wc -l
52

$ fff " ==$" | wc -l
1501
$ fff "^ *== " | wc -l
161

$ fff " !=$" | wc -l
699
$ fff "^ *!= " | wc -l
129

They are rough regexps and probably have some false positives, but the
numbers aren't even close; operators at the end of the line clearly
dominate.

I will conform for the greater good but silently weep inside every time I
see it.

Nick

[gsqu...@mozilla.com]

Of course grepping agrees with you, since it's been the coding style so far! ;-)
I was hoping to change that, if most people agreed.

While it's good to know how many people are for or against it, so that we get a sense of where the majority swings, I'd really like to know *why* people have their position. (I could learn something!)

So Nick, would you have some reasons for your "strong preference"? And what do you think of the opposite rationale as found in [2]?

(But I realize it's more work, so no troubles if you don't have the time to expand on your position here&now; thank you for your feedback so far.)

[David Major]

One thing I like about trailing operators is that they tend to match
what you'd find in bullet-point prose. Here's a made-up example:

You can apply for a refund of your travel insurance policy if:
* You cancel within 7 days of purchase, and
* You have not yet begun your journey, and
* You have not used any benefits of the plan.

Over time my eyes have come to expect the conjunction on the right.

[Botond Ballo]

On Fri, Feb 17, 2017 at 1:28 AM, <gsqu...@mozilla.com> wrote:
> While it's good to know how many people are for or against it, so that we get a sense of where the majority swings, I'd really like to know *why* people have their position. (I could learn something!)

I prefer having the operator at the beginning of the line (i.e. what
you're proposing). I'll try to explain my reasoning.

I view code's innate structure as being a tree, and the thing we edit
as just a serialization of that tree into a sequence of characters for
convenient editing and storage.

In this conceptual tree, I view a binary operator expression such as
"a && b" as having the following structure:

&&
├── a
└── b

That is, the operator is the root of the subtree, and the operands are
the leaves.

For me, the most salient piece of information in the subtree is "what
type of subtree is this?" - that is, the operator.

Having the operator at the beginning of the line, and especially that
combined with indenting both operands further than the operator, puts
this most salient information front and centre. By contrast, having it
at the end of the line means your eyes need to travel over and parse
the entire first operand to find it.

Hopefully that made sense :)

Cheers,
Botond

[Nicholas Nethercote]

On Fri, Feb 17, 2017 at 5:28 PM, <gsqu...@mozilla.com> wrote:

> Of course grepping agrees with you, since it's been the coding style so
> far! ;-)
> I was hoping to change that, if most people agreed.
>

Isn't the current style that && and || go at the end but other operators go
at the start? And yet in practice going at the end dominates for all
operators.

As for [2]:

"Rationale: operator at the front of the continuation line makes for faster
visual scanning, because there is no need to read to end of line. Also
there exists a context-sensitive keyword hazard in JavaScript; see bug
442099, comment 19, which can be avoided by putting . at the start of a
continuation line in long member expression."

The first sentence isn't compelling to me; reading to the end of a line
isn't that hard. Having an exception for . in JS might be reasonable. I
don't write JS much so that's not a high priority for me.

Nick

[Eric Rescorla]

On Thu, Feb 16, 2017 at 11:39 PM, David Major <dma...@mozilla.com> wrote:

> One thing I like about trailing operators is that they tend to match
> what you'd find in bullet-point prose. Here's a made-up example:
>
> You can apply for a refund of your travel insurance policy if:
> * You cancel within 7 days of purchase, and
> * You have not yet begun your journey, and
> * You have not used any benefits of the plan.
>
> Over time my eyes have come to expect the conjunction on the right.
>

I tend to agree with this, though it's just opinion.

In any case, absent some pretty compelling reason, I don't see a good
reason to change the style here, given that we clearly don't have consensus
to do so.

-Ekr

> On Fri, Feb 17, 2017, at 07:28 PM, gsqu...@mozilla.com wrote:

[gsqu...@mozilla.com]

Hi again Nick,

Someone made me realize that I didn't fully read your message, sorry for that.

I now see that as well as &&/||, you have grepped for other operators, and shown that the overwhelming usage is to put all of them at the end of lines!

In light of this, and from what others here&elsewhere have discussed, I'm now thinking that we probably should indeed just update our coding style with whatever is used the most out there, and model our "Official Formatter" on it.


Another thought, if technically possible:
If our main repository is going to always be under the control of some official clang-format style, it should be possible for anybody to pull the repository, and use a different formatter locally with their favorite style. And when pushing, their modified code could be automatically reformatted with the official formatter -- Everybody feels good when programming! :-)

Cheers,
Gerald

On Friday, February 17, 2017 at 5:16:42 PM UTC+11, Nicholas Nethercote wrote:

[Bobby Holley]

On Fri, Feb 17, 2017 at 2:18 PM, <gsqu...@mozilla.com> wrote:

> Hi again Nick,
>
> Someone made me realize that I didn't fully read your message, sorry for
> that.
>
> I now see that as well as &&/||, you have grepped for other operators, and
> shown that the overwhelming usage is to put all of them at the end of lines!
>
> In light of this, and from what others here&elsewhere have discussed, I'm
> now thinking that we probably should indeed just update our coding style
> with whatever is used the most out there, and model our "Official
> Formatter" on it.
>
>
> Another thought, if technically possible:
> If our main repository is going to always be under the control of some
> official clang-format style, it should be possible for anybody to pull the
> repository, and use a different formatter locally with their favorite
> style. And when pushing, their modified code could be automatically
> reformatted with the official formatter -- Everybody feels good when
> programming! :-)
>

Please no. That would make reviewing a nightmare.

[Martin Thomson]

On Sat, Feb 18, 2017 at 9:28 AM, Bobby Holley <bobby...@gmail.com> wrote:
>> If our main repository is going to always be under the control of some
>> official clang-format style, it should be possible for anybody to pull the
>> repository, and use a different formatter locally with their favorite
>> style. And when pushing, their modified code could be automatically
>> reformatted with the official formatter -- Everybody feels good when
>> programming! :-)
>>
>

> Please no. That would make reviewing a nightmare.

Yes, though this is totally technically possible, it means that it is
also possible to generate the most awesome confusion. "Line 517 is
broken, looks fine to me."

[gsqu...@mozilla.com]

On Saturday, February 18, 2017 at 9:29:06 AM UTC+11, Bobby Holley wrote:
> On Fri, Feb 17, 2017 at 2:18 PM, <gsqu...@mozilla.com> wrote:
>
> > Hi again Nick,
> >
> > Someone made me realize that I didn't fully read your message, sorry for
> > that.
> >
> > I now see that as well as &&/||, you have grepped for other operators, and
> > shown that the overwhelming usage is to put all of them at the end of lines!
> >
> > In light of this, and from what others here&elsewhere have discussed, I'm
> > now thinking that we probably should indeed just update our coding style
> > with whatever is used the most out there, and model our "Official
> > Formatter" on it.
> >
> >
> > Another thought, if technically possible:
> > If our main repository is going to always be under the control of some
> > official clang-format style, it should be possible for anybody to pull the
> > repository, and use a different formatter locally with their favorite
> > style. And when pushing, their modified code could be automatically
> > reformatted with the official formatter -- Everybody feels good when
> > programming! :-)
> >
>
> Please no. That would make reviewing a nightmare.

Not sure what your concern is:
- People who stick with the official parser will only ever see that (pushing to mozreview will reformat everything to the official style).
- People who prefer a different style at home will have to adapt to the different style when reviewing others' code, or when their code gets review comments in a different style -- the formatter shouldn't change things so wildly that it would be so hard to find the corresponding location in their local repo.

So it's everybody's choice to avoid a nightmare, or deal with it. ;-)

[Jared Wein]

After dealing with bugs that only happen in PGO builds, I would be worried
about bugs in the code-formatting-rewriter. The value doesn't seem worth
the risk to me.

- jared

[Boris Zbarsky]

On 2/17/17 5:44 PM, gsqu...@mozilla.com wrote:
> - People who stick with the official parser will only ever see that (pushing to mozreview will reformat everything to the official style).

Not all reviews go through mozreview.

-Boris

[gsqu...@mozilla.com]

Not yet, if the mozreview crowd has their way. ;-)

But anyway, here's a possible solution: Instead of even modifying local files, your favorite editor could reformat to your favorite style on load, and reformat back to the official style on save.

-Gerald

[ISHIKAWA,chiaki]

On 2017/02/18 7:28, Bobby Holley wrote:
> On Fri, Feb 17, 2017 at 2:18 PM, <gsqu...@mozilla.com> wrote:
>

>> Hi again Nick,
>>
>> Someone made me realize that I didn't fully read your message, sorry for
>> that.
>>
>> I now see that as well as &&/||, you have grepped for other operators, and
>> shown that the overwhelming usage is to put all of them at the end of lines!
>>
>> In light of this, and from what others here&elsewhere have discussed, I'm
>> now thinking that we probably should indeed just update our coding style
>> with whatever is used the most out there, and model our "Official
>> Formatter" on it.
>>
>>
>> Another thought, if technically possible:
>> If our main repository is going to always be under the control of some
>> official clang-format style, it should be possible for anybody to pull the
>> repository, and use a different formatter locally with their favorite
>> style. And when pushing, their modified code could be automatically
>> reformatted with the official formatter -- Everybody feels good when
>> programming! :-)
>>
>

> Please no. That would make reviewing a nightmare.
>

I think the idea is to produce the patch after (virtually) reformatting
the local tree into the officially adopted format and create the patch
against the tree.
The reviewers will get into

checkout
Central repo
-> local repo in the official format
check out a file or files to be modified.
Use a clang-format or whatever by passing options to it
to produce one's preferred format and check it (them) into
a working repo.
-> local repo in the preferred format: local mods take place
We work on the files in one's preferred format.

checkin/creating a patch

local repo in the preferred format: local mods take place
Locally use clang-format or whatever to mutate the changed
file into official format and check into the local repo in the
official format.
<--
local repo in the official format
Either create a patch (in the official format source)
or check into the central repo.
<--
Central repo

Point 1:
I think creating a wrapper for conversion between the two local repos
as above should not be that difficult.

Point 2:
I prefer all the operators including "&&" and "||"
at the beginning since such a format makes the tree-like structure of
multi-line easier to understand and edit, too.
Adding a condition or removing a condition is much easier in this form.
I forgot where the rational was explained well. Superficially it is
covered in GNU coding starnds:
https://www.gnu.org/prep/standards/html_node/Formatting.html

Point 3: Having a tool that does such conversion AND having an editor
support such formating style is very important (In my case, it is Emacs.)

Point 4: Have no fear. Such conversion tools have existed at least for
C, Fortran, Cobol and other languages
and been used extensively.
For example, more than a dozen years ago,
in the days of version 2.1.1xx, the whole SCSI subsystem source tree of
linux kernel were converted using "indent" program.
The only change of SCSI subsystem in that revision was formating change.
This was to untangle the spaghetti code of SCSI subsystem.

Point 5: We should set up a "Flag Day" to convert the source tree into
the official format THAT IS SUPPORTED by the mechanical
converter/formater, and change the source code in one sweep.
For example, the data can be 6 months from now. August 18, 2017.
(Such a flag day / intention was announced for SCSI subsystem format
change of linux kernel, and the "Flag Day" moniker was used when Multics
changed its internal character code from what I read in the glossary
file which later became Hacker's Dictionary.)

Mechanical conversion/formating is nice: we can do this locally before
producing an official patch, and there will be no disagreements among
the reviewers :-)


expressions

> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>

[Jean-Yves Avenard]

On 17/02/17 23:18, gsqu...@mozilla.com wrote:
> Hi again Nick,
>
> Someone made me realize that I didn't fully read your message, sorry for that.
>
> I now see that as well as &&/||, you have grepped for other operators, and shown that the overwhelming usage is to put all of them at the end of lines!
>
> In light of this, and from what others here&elsewhere have discussed, I'm now thinking that we probably should indeed just update our coding style with whatever is used the most out there, and model our "Official Formatter" on it.
>
>
> Another thought, if technically possible:
> If our main repository is going to always be under the control of some official clang-format style, it should be possible for anybody to pull the repository, and use a different formatter locally with their favorite style. And when pushing, their modified code could be automatically reformatted with the official formatter -- Everybody feels good when programming! :-)
>

What worries me here, is that I'm yet to read a compelling argument on
the *why* people were doing things a particular way (with the exception
of David Major who provided a logical explanation for it).

That you've done something out of habits for the last XX years is no
argument. Neither a "liking" for a form over another.

How things have been is also no enticing argument, as if we were to base
our future use on what currently exists (like the grep command provided
did), then I vote for absolute free coding style left to the developer
wishes. After all it certainly feels that way. There's not a single
directory in our code tree that has a consistent coding style. Braces,
position of the return type vary wildly across the code.

Regardless, we need to make a decision. Little of the most commonly seen
style in our tree is reflected in the coding style wiki page.
So either we decide that we amend the official coding style to reflect
what's already there, tweak the coding style so there's no more
ambiguities (like the distinction between the || and && operators and
all the others), or modify the code to comply with those rules.

I'll just re-iterate that operators be placed at the end of a logical
expression can't be reasonably and logically argued for. But maybe I've
been using a RPN calculators for too long.

I do hope that we end up with a single rule for all, because the current
state of things is terrible.

My $0.02
JY

[Michael Froman]

> On Feb 17, 2017, at 6:24 PM, ISHIKAWA,chiaki <ishi...@yk.rim.or.jp> wrote:
>
> Point 2:
> I prefer all the operators including "&&" and "||"
> at the beginning since such a format makes the tree-like structure of multi-line easier to understand and edit, too.
> Adding a condition or removing a condition is much easier in this form.
> I forgot where the rational was explained well. Superficially it is covered in GNU coding starnds:
> https://www.gnu.org/prep/standards/html_node/Formatting.html

If we’re considering changing the coding standard, my vote would be for all the operators including "&&" and "||” at the beginning for the reasons above.

-Michael.
[:mjf]

[Eric Rescorla]

On Fri, Feb 17, 2017 at 4:24 PM, ISHIKAWA,chiaki <ishi...@yk.rim.or.jp>
wrote:
>

> Point 5: We should set up a "Flag Day" to convert the source tree into the
> official format THAT IS SUPPORTED by the mechanical converter/formater, and
> change the source code in one sweep.
>

This seems like it places an enormous amount of trust in the tools not to
accidentally make semantic changes.

FWIW, when we converted NSS entirely to clang-format, we actually checked
that the compiler was producing the same output for the pre- and
post-formatted
versions and at least did some cursory review of the patches themselves. It
might make more sense to do this in stages.

-Ekr

>>>> dev-pl...@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform

[Eric Rescorla]

On Sat, Feb 18, 2017 at 1:10 AM, Jean-Yves Avenard <jyav...@mozilla.com>
wrote:

>
>
> On 17/02/17 23:18, gsqu...@mozilla.com wrote:
>

>> Hi again Nick,
>>
>> Someone made me realize that I didn't fully read your message, sorry for
>> that.
>>
>> I now see that as well as &&/||, you have grepped for other operators,
>> and shown that the overwhelming usage is to put all of them at the end of
>> lines!
>>
>> In light of this, and from what others here&elsewhere have discussed, I'm
>> now thinking that we probably should indeed just update our coding style
>> with whatever is used the most out there, and model our "Official
>> Formatter" on it.
>>
>>
>> Another thought, if technically possible:
>> If our main repository is going to always be under the control of some
>> official clang-format style, it should be possible for anybody to pull the
>> repository, and use a different formatter locally with their favorite
>> style. And when pushing, their modified code could be automatically
>> reformatted with the official formatter -- Everybody feels good when
>> programming! :-)
>>
>>

> What worries me here, is that I'm yet to read a compelling argument on the
> *why* people were doing things a particular way (with the exception of
> David Major who provided a logical explanation for it).
>
> That you've done something out of habits for the last XX years is no
> argument. Neither a "liking" for a form over another.
>

Actually, for matters of taste these seem like fairly compelling arguments,
especially
given the relative lack of evidence that most of these formatting questions
actually materially impact code readability.

I'd also note that if we're not going to use "this is what we have done
historically"
as a guide, then it seems like much bigger changes are on the table and we
would probably be better off adopting some other well-defined coding
standard
wholesale (e.g., the Google C++ guide), with the decision largely being made
on what has the best tooling available. Personally, I'd prefer this, but I
haven't
heard much enthusiasm for discarding a number of our style choices, however
idiosyncratic they may be (aFoo, I'm looking at you).

-Ekr

[Kyle Lahnakoski]

There is lots of whitespace to add: If the expression is over a line
long, then use indentation so the block structure follows the logical
structure:

____return
________(
____________(aCodecMask & VPXDecoder::VP8)
____________&&
____________aMimeType.EqualsLiteral("video/webm; codecs=vp8")
________)
________||
________(
____________(aCodecMask & VPXDecoder::VP9)
____________&&
____________aMimeType.EqualsLiteral("video/webm; codecs=vp9")
________)
________||
________(
____________(aCodecMask & VPXDecoder::VP9)
____________&&
____________aMimeType.EqualsLiteral("video/vp9")
________)
____;

[smaug]

On 02/18/2017 07:08 PM, Eric Rescorla wrote:

> I'd also note that if we're not going to use "this is what we have done
> historically"
> as a guide, then it seems like much bigger changes are on the table and we
> would probably be better off adopting some other well-defined coding
> standard
> wholesale (e.g., the Google C++ guide), with the decision largely being made
> on what has the best tooling available. Personally, I'd prefer this, but I
> haven't
> heard much enthusiasm for discarding a number of our style choices, however
> idiosyncratic they may be (aFoo, I'm looking at you).
>
> -Ekr
>

I don't care too much about &&/|| coding style, though the current style does feel easier to
read, per the reasoning dmajor gave.



But I greatly care about having different naming for different kinds of variables.
That reduces risk for errors and eases reviewing. So, removing aFoo? Please no! ;)

[Martin Thomson]

On Mon, Feb 20, 2017 at 3:18 AM, smaug <sm...@welho.com> wrote:
> I don't care too much about &&/|| coding style, though the current style
> does feel easier to
> read, per the reasoning dmajor gave.

I suspect that a lot of people think this way. While it's tempting to
suggest that arguments like "that's the way I've always done it" are
bogus, there's a value in maintaining the current set of wiring in
your brain. I've learned to eyeball code pretty quickly over time and
changing layout risks reducing my efficiency.

I don't know if there's a material difference in this case, and like
smaug, I don't feel passionately about this. Absent good evidence
that we're losing somehow, there is always at least some value in
retaining the current practice.

[Bobby Holley]

On Sun, Feb 19, 2017 at 8:07 PM, Martin Thomson <m...@mozilla.com> wrote:

> On Mon, Feb 20, 2017 at 3:18 AM, smaug <sm...@welho.com> wrote:

> > I don't care too much about &&/|| coding style, though the current style
> > does feel easier to
> > read, per the reasoning dmajor gave.
>

> I suspect that a lot of people think this way. While it's tempting to
> suggest that arguments like "that's the way I've always done it" are
> bogus, there's a value in maintaining the current set of wiring in
> your brain. I've learned to eyeball code pretty quickly over time and
> changing layout risks reducing my efficiency.
>
> I don't know if there's a material difference in this case, and like
> smaug, I don't feel passionately about this. Absent good evidence
> that we're losing somehow, there is always at least some value in
> retaining the current practice.
>

+1.

[Benjamin Smedberg]

On Thu, Feb 16, 2017 at 4:47 PM, <gsqu...@mozilla.com> wrote:

> Question of the day:
> When breaking overlong expressions, should &&/|| go at the end or the
> beginning of the line?
>
> TL;DR: Coding style says 'end', I&others think we should change it to
> 'beginning' for better clarity, and consistency with other operators.
>

I will avoid having an opinion here, and merely state for the record that
the final owner of this decision is Ehsan, as module owner of the C++/Rust
style module [1]. Since the discussion appears to be winding down, I hope
that Ehsan can make his decision known to finalize this email thread.

--BDS

[1]
https://wiki.mozilla.org/Modules/All#C.2B.2B.2FRust_usage.2C_tools.2C_and_style

[Emilio Cobos Álvarez]

On Wed, Feb 22, 2017 at 10:27:33AM -0500, Benjamin Smedberg wrote:
> On Thu, Feb 16, 2017 at 4:47 PM, <gsqu...@mozilla.com> wrote:
>
> > Question of the day:
> > When breaking overlong expressions, should &&/|| go at the end or the
> > beginning of the line?
> >
> > TL;DR: Coding style says 'end', I&others think we should change it to
> > 'beginning' for better clarity, and consistency with other operators.
> >
>
> I will avoid having an opinion here, and merely state for the record that
> the final owner of this decision is Ehsan, as module owner of the C++/Rust
> style module [1]. Since the discussion appears to be winding down, I hope
> that Ehsan can make his decision known to finalize this email thread.

Oh, I didn't know we had a common module for both Rust and C++ style.

Given we do, I'd argue that probably it's a good idea to be consistent
across both, and the common Rust style is having those operators at the
end of lines[1].

-- Emilio

[1]: https://github.com/rust-lang-nursery/fmt-rfcs/blob/1bd5b49dc38a48aaabe5d90c87e463351db42091/example/lists.rs#L302