On 2017/02/18 7:28, Bobby Holley wrote:
On Fri, Feb 17, 2017 at 2:18 PM, <gsquel...@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
Cheers,
Gerald
On Friday, February 17, 2017 at 5:16:42 PM UTC+11, Nicholas Nethercote
wrote:
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
On Fri, Feb 17, 2017 at 8:47 AM, <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.
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
____________________________________________
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
