What should we do about imports that exceed the 120-char line limit?

[pixelzoom]

What should we do about imports that exceed the 120-char line limit?

Slack developer channel:

Jesse Greenberg 7:44 AM
WebStorm wants to format this import statment like
import EnergySkateParkColorScheme
from '../../../../../energy-skate-park/js/energy-skate-park/common/view/EnergySkateParkColorScheme.js';
Is that right? I thought I disabled all auto wrapping but maybe I am missing a setting somewhere?

Sam Reid 8:31 AM
checking…
8:33
That happened for me too
8:35
I changed the “Settings=>Editor=>Code STyle => Javascript => Wrapping and Braces => Hard Wrap at” from 120 to 999 and the problem went away:
8:35

Jesse Greenberg 8:52 AM
Thanks! That fixed it for me too.

Sam Reid 9:27 AM
Perhaps we should check in that change to the code style?

Jesse Greenberg 9:40 AM
Before checking in, I noticed increasing the "Hard wrap at" setting pushed the visual guide (ruler) out that many characters too. I added another visual guide at 120 characters. I don't want to make everyone's visual guides go away.

Michael Kauzmann:octopus: 10:13 AM
Yes I don't think that changing that setting is worth it for just the wrapping import statement.

Chris Malley 10:33 AM
The 120-char line limit has not been a hard limit, up to developer discretion, and I'd hate to change that. I see the limit being applied when I auto-import or Organize imports. I do not see the limit applied when I format code. My vote would be to live with it for imports, since (most) are added by the IDE, and typically collapsed.

Jesse Greenberg 10:35 AM
Cool, I agree with that and it sounds good to me

[zepumph]

My vote would be to live with it for imports, since (most) are added by the IDE, and typically collapsed.

I'm not sure what @pixelzoom's opinion is from this. Does "live with it" mean to be okay with having a long import split on two lines?

I personally feel like having the "hard wrap" setting in my IDE, and I would rather not turn it off.

[pixelzoom]

Sorry... To clarify, "live with it" means I'm fine with the 120-char line limit being applied to imports "since (most) are added by the IDE, and typically collapsed."

[jonathanolson]

It's tricker to design a lot of automated things (reordering imports, parsing imports for string detection, etc.) around multi-line imports, so some code would need to change. I have a preference to have single-line imports.

[samreid]

I agree, single lines seem preferable for tooling support.

[chrisklus]

From 4/2/20 dev meeting:

We agree single line seems preferable. @jessegreenberg is going to update phet-code-style.xml to not hard wrap at 120, but still have a visual guide at 120 characters. He's also going to create a lint rule that flags two-line import statements.

[jessegreenberg]

This is done. A new lint rule "single-line-import" has been added that enforces this, and above commits outside of chipper were made where the lint rule found import statements on multiple lines. Over slack I said

Hello all - the phet-idea-codestyle.xml was changed for #139, so please be aware of the following.

• If you use WebStorm/IntelliJ, please pull phet-info, then re-import phet-idea-codestyle.xml so your IDE conforms to this change.
• A new lint rule was introduced to keep imports on one line.
• Commits are being pushed to 19 repos with this formatting change.

Closing.

[pixelzoom]

A note about the above commit, for posterity...

I was curious about what specifically changed, so looked at 5e263c1. The entire file had changed! @samreid pointed out that it was a formatting issue. (The formatting of the code-style file is wrong. Oh, the irony.). So I reformatted the file, confirmed that only 2 lines had really changed, and pushed.