Closed Bug 1310615 Opened 3 years ago Closed 3 years ago

Force unix-style line endings across devtools files

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: jdescottes, Unassigned)

References

Details

Attachments

(1 file)

Lost a significant amount of time trying to understand why a patch attached to Bug 1305365 could not apply cleanly. 

Turns out it was because one of the edited files had windows style line endings but the patch file didn't have any. I am guessing the patch file had been manually edited/amended, and whatever editor agressively replaced all line endings.

I would like to discuss a change to our eslint configuration to allow only linux style endings (we currently explicitly allow any kind of linebreak).
CC'd jryans and pbro. Any reason we want to allow windows-style endings?
(In reply to Julian Descottes [:jdescottes] from comment #3)
> CC'd jryans and pbro. Any reason we want to allow windows-style endings?
Not that I'm aware of. This old etherpad backup is the doc we used when we implemented eslint originally and discussed all rules:
https://old.etherpad-mozilla.org/eslint-devtools
I don't think we spent much time on this particular one.
Seems best to make them all Unix only.  Seems like the only exception would be if there is some test case that's testing for specific line endings or something like that, but we may not have any currently.

I wonder if some people on Windows have Git / Mercurial configured to use Windows line endings on checkout but Unix when committing... (I recall that being an option in the Git installer for Windows.)  If they do, I suppose this change would flag all files as errors if they are running ESLint in their editor.

:pbro, do you still use Windows for development?  I'm just curious if there are side-effects of the change such as that.
Flags: needinfo?(pbrosset)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5)
> Seems best to make them all Unix only.
Agreed.
> I wonder if some people on Windows have Git / Mercurial configured to use
> Windows line endings on checkout but Unix when committing... (I recall that
> being an option in the Git installer for Windows.)  If they do, I suppose
> this change would flag all files as errors if they are running ESLint in
> their editor.
I'm not seeing any options in Mercurial for this. There is a eol extension that one can use to automatically convert line endings to a particular format.
> :pbro, do you still use Windows for development?  I'm just curious if there
> are side-effects of the change such as that.
Yes I do, and I'm not seeing any problems locally when applying Julian's patch.
Maybe I just don't know what to look for, but so far everything seems fine. ESLint doesn't report any issues. When I open files in Sublime Text (my editor), it tells me that line ending are UNIX style.
Flags: needinfo?(pbrosset)
One downside is that Sublime Linter is not able to enforce this particular rule (see issue [1]). The files stored by Sublime in memory are always using Unix style endings so developers relying on Sublime for eslint validation will not get early warnings about this violation.

However this setting seems to be the one chosen by most of the other eslint configurations found in mozilla central (see [2]) so I still think we should go ahead with this.

This changeset contains the exlint configuration change + the outcome of running eslint --fix after updating the conf.
(this is on top of inbound because a modification on an offending file was recently made but is only on this branch for now)

[1] https://github.com/roadhump/SublimeLinter-eslint/issues/143
[2] http://searchfox.org/mozilla-central/search?q=linebreak-style&path=
Priority: -- → P3
Comment on attachment 8801660 [details]
Bug 1310615 - eslint: force unix style linebreaks in devtools;

https://reviewboard.mozilla.org/r/86336/#review85632

Looks good, thanks for making the change!
Attachment #8801660 - Flags: review?(jryans) → review+
Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b464e34825
eslint: force unix style linebreaks in devtools;r=jryans
Thanks for the review! Try looks green, landing.
https://hg.mozilla.org/mozilla-central/rev/e3b464e34825
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.