Fwd: Upcoming changes to our JS coding style
Magnus Melin wrote on 29.08.19 09:01:
I think we want to use the same rules as mozilla-central, and they use
80ch, just like Thunderbird has always used.
Thunderbird has always encouraged to stay within 80 chars, but it was
never a hard limit. A little more was OK, if justified, like in the
cases I mentioned. The hard limit was somewhere around 120 chars.
All I'm suggesting is to carry on the existing rules: "SHOULD 80, MUST
120", to use IETF language :)
You'd have major problems otherwise especially comparing/maintaining
semi-forked files that would have to look totally different just
because they were copied.
That won't be a problem, if our rules are more relaxed, as I'm proposing.
Ben
Jörg Knobloch wrote on 29.08.19 10:51:
We have many lines exceeding 80 chars in JS code, so there well be a lot of unnecessary churn
+1
C++ mail code is affected, too. We've never adhered to the 80 chars rule in a strict sense.
We'd really need to see some formatting examples to compare to see whether the code would end up "overly compact". Maybe that's really the case.
Old code:
var {MailServices} = ChromeUtils.import(<a class="moz-txt-link-rfc2396E" href="resource:///modules/MailServices.jsm">"resource:///modules/MailServices.jsm"</a>);
New code:
var { MailServices } = ChromeUtils.import(
<a class="moz-txt-link-rfc2396E" href="resource:///modules/MailServices.jsm">"resource:///modules/MailServices.jsm"</a>
);
This is not an exception. This exact line we have thousands of times. There are many other examples like that, aside from imports.
Whoever claims that this is better for "readability"... must be reading in a different way than I am (Japanese top to bottom?).
I'm not against the idea of code reformating. Just some specific rules are overzealous and destructive. And these few rules do as much damage as the formatting does on the positive side. I'm only talking about a few very specific rules here, esp. around ifs.
Hands up who is still using vi in a terminal window ;-)
You can even resize terminal windows.
Hands up who is still using vi on physical green 80x25 char terminal! ;-)
For those young ones: Something like this: http://cts.dmu.ac.uk/study/images/Viewpoint-1A-dumb-terminal-large.JPG
Ben
On 29 Aug 2019 14:53, Paul Morris wrote:
Should we delay this work 6 weeks until just before the next merge
(and beta release) in order to have more time for discussion to reach
a consensus decision?
Seems like an option. We have three releases next week, some of which
are important, like 68.1 which will fix some issues in 68.0, and 60.9
which is hopefully the last one in the 60.x cycle.
We really need to get all people on board, and the examples Ben showed
don't look pretty.
Jörg.
Currently calendar line width is 100 characters and there hasn't been a
decision to change that when using Prettier's auto-formatting. So
here's a proposal:
-
Go ahead and do the work to use Prettier with calendar code so that it
can land before the merge on Monday. (With 100 character line width.)
No reason to delay that based on discussions of TB line width. -
Delay the work to start formatting TB code with Prettier for 6 weeks
to allow time for more discussion of optimal line width with Prettier, etc.
This gives us a more incremental approach, which is a good idea in any
case. (It will just take a little juggling of eslint rules, no big deal.)
Please let me know your thoughts on this proposal. I would like to move
forward with it today, if possible.
-Paul
OK with me
Note that my issues are not just with line width, but specific other
rules like "lonely else" and "one if condition per line", but I don't
care much about Lightning, so your proposal is reasonable.
Paul Morris wrote on 29.08.19 16:29:
Currently calendar line width is 100 characters and there hasn't been
a decision to change that when using Prettier's auto-formatting. So
here's a proposal:
-
Go ahead and do the work to use Prettier with calendar code so that
it can land before the merge on Monday. (With 100 character line
width.) No reason to delay that based on discussions of TB line width. -
Delay the work to start formatting TB code with Prettier for 6 weeks
to allow time for more discussion of optimal line width with Prettier,
etc.
This gives us a more incremental approach, which is a good idea in any
case. (It will just take a little juggling of eslint rules, no big
deal.)
Please let me know your thoughts on this proposal. I would like to
move forward with it today, if possible.
-Paul
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
On 29 Aug 2019 16:29, Paul Morris wrote:
Please let me know your thoughts on this proposal. I would like to
move forward with it today, if possible.
Sounds good.
Despite 80 resulting in some unusual formats, I vote for consistency.
Unlike much more serious issues with following Fx (toolbars), this is
definitely not one of them and definitely not worth the inconsistency.
On 08/29/2019 02:51 AM, Jörg Knobloch wrote:
On 29 Aug 2019 09:01, Magnus Melin wrote:
I think we want to use the same rules as mozilla-central, and they
use 80ch, just like Thunderbird has always used. You'd have major
problems otherwise especially comparing/maintaining semi-forked files
that would have to look totally different just because they were copied.
There are very few "semi-forked" files, mostly in the area of
preferences and the about:* pages.
If you really need to compare, you can format the M-C page to C-C
standards first.
We have many lines exceeding 80 chars in JS code, so there well be a
lot of unnecessary churn to reach a design of the TTY era of the 1980s.
Hands up who is still using vi in a terminal window ;-)
Jörg.
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
Hey Folks,
if all this is about is line width, we can certainly bikeshed about this for another 6 weeks. Everyone likely has their preference, as you can also see in the discussion it sparked on dev.platform.
Honestly, I feel this is something that we won't be able to agree on collectively. I think the best way forward would be to skip the discussion and have Paul and respective module owners decide now.
There will certainly be things that will be horrible with prettier, but the whole point was to not have to worry about specific rules and how they would interact. You can find more reasoning on dev.platform.
This is just my recommendation though, if you feel postponing it for 6 weeks helps, do it :)
Philipp
On 29. Aug 2019, at 4:41 PM, Jörg Knobloch jorgk@jorgk.com wrote:
On 29 Aug 2019 16:29, Paul Morris wrote:
Please let me know your thoughts on this proposal. I would like to move forward with it today, if possible.
Sounds good.
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
Philipp,
everybody apart from Magnus (and maybe Paul) has said 100 chars (or
more, I said 120). So, there seems to be broad very consensus for 100
instead of 80.
As for myself, it's not just about line width, but also a few selected
other rules about ifs that go wrong and are destructive for readability.
Ben
Philipp Kewisch wrote on 29.08.19 20:35:
Hey Folks,
if all this is about is line width, we can certainly bikeshed about this for another 6 weeks. Everyone likely has their preference, as you can also see in the discussion it sparked on dev.platform.
Honestly, I feel this is something that we won't be able to agree on collectively. I think the best way forward would be to skip the discussion and have Paul and respective module owners decide now.
There will certainly be things that will be horrible with prettier, but the whole point was to not have to worry about specific rules and how they would interact. You can find more reasoning on dev.platform.
This is just my recommendation though, if you feel postponing it for 6 weeks helps, do it :)
Philipp
On 29. Aug 2019, at 4:41 PM, Jörg Knobloch jorgk@jorgk.com wrote:
On 29 Aug 2019 16:29, Paul Morris wrote:
Please let me know your thoughts on this proposal. I would like to move forward with it today, if possible.
Sounds good.
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net
Please go ahead with the mail side too.
Prettier recommends 80ch, mozilla-central user 80ch, Thunderbird always
used 80ch. We don't need to bikeshed another 6 weeks to realize that
we'll use 80ch.
-Magnus
On 29-08-2019 17:29, Paul Morris wrote:
Currently calendar line width is 100 characters and there hasn't been
a decision to change that when using Prettier's auto-formatting. So
here's a proposal:
-
Go ahead and do the work to use Prettier with calendar code so that
it can land before the merge on Monday. (With 100 character line
width.) No reason to delay that based on discussions of TB line width. -
Delay the work to start formatting TB code with Prettier for 6 weeks
to allow time for more discussion of optimal line width with Prettier,
etc.
This gives us a more incremental approach, which is a good idea in any
case. (It will just take a little juggling of eslint rules, no big
deal.)
Please let me know your thoughts on this proposal. I would like to
move forward with it today, if possible.
-Paul
Maildev mailing list
Maildev@lists.thunderbird.net
http://lists.thunderbird.net/mailman/listinfo/maildev_lists.thunderbird.net