Standardize Line Encodings
Closed this issue · 4 comments
Moved from #204
From @Cloud7050,
Using git ls-files --eol, this repo appears to sometimes use LF and sometimes use CRLF in the index (and some files are even mixed). For comparison, the main js-slang repo appears to mainly use LF.
This repo's base eslint config is based on a config I've been carrying with me for a while. As I'm on Windows, that rule's setting is less of a concious choice for this repo. If I'm understanding this correctly, maybe it'll be better to:
- Rely on each user's git core.autocrlf config to handle line endings across platforms (true on Windows, input on Linux/macOS?)
- Turn off the linebreak-style rule in the base eslint config. Users are free to locally checkout LF or CRLF
- Push a commit that converts all existing files to be indexed with LF (could use something like eol-converter-cli on npm)
From there I think the repo may sometimes get CRLF introduced, but they would sometimes get corrected to LF down the line. Any thoughts?
My perspective on this is informed by Prettier, which says:
When people collaborate on a project from different operating systems, it becomes easy to end up with mixed line endings in a shared git repository. It is also possible for Windows users to accidentally change line endings in a previously committed file from LF to CRLF. Doing so produces a large git diff and thus makes the line-by-line history for a file (git blame) harder to explore.
Because of the above reason, I don't really agree with allowing for mixed line endings.
IMO, the best way to fix this issue will be to run prettier or eslint --fix in a pre-commit hook and configuring .gitattributes to ignore line endings project-wide, and then we don't even need to push a commit to convert all existing files.
Any thoughts?
Prettier used to be in the repo, but was recently removed (#148). Having worked both with and without it in this repo, I find not having it to be less intrusive given its highly opinionated formatting, and support its removal; I feel eslint is sufficient.
Eslint is able to autofix the line endings, which is convenient, though it is somewhat annoying to autofix only one rule should the need arise. I'd forgotten that this repo had already received a pass over fixing all reported eslint problems recently, so it should be just fine to also use autofix as the means to do one-time conversion in the working directory.
My worry with leaving the eslint rule on now that this has been raised is its potential to annoy certain users with reported problems. If eslint wants one kind of line ending, and files are checked in fine with that line ending, but a user checks out locally with a different line ending due to platform/config, then eslint would spam warnings. If we are able to avoid this scenario even with the rule on using some combination of gitattributes/gitconfig, then that would be great. We would enjoy the benefit of avoiding mixed line endings being checked in.
If I'm following correctly, we could unset the text attribute for the whole repo (ignore conversion) and rely on eslint to autofix to a certain line ending. And this would ignore all users' local gitconfigs for core.eol and core.autocrlf? Then we wouldn't run into a problem like the rule wanting LF, but a Windows user ending up with CRLF checked out in the working directory with each git command (thus breaking the eslint rule, even if it is eventually LF that gets checked in).
I think that would still rely on running eslint's autofix to guarantee standardisation. Would it be simpler to configure git to handle the conversion automatically? Perhaps some combination of text=auto and eol?
I am not sure about how this setup would remove the need for some commit standardising all existing files in the index.
As an aside, I tend to disable some of the time-consuming hooks when coding on Windows. For example, automatic reinstallation of packages, the full lint+fix+build pipeline before committing (admittedly less of a concern now given the drastically reduced build times, but I still tend to split my stuff into multiple smaller commits), and the running of scripts that don't work on Windows. Instead I run those manually, and use VSCode's eslint extension to fix/autofix problems myself as I code.
Ah I see, I understand that prettier is pretty intrusive, so we can skip that then, I get your hesitation about time-consuming hooks as well.
In that case, I think setting the .gitattributes file is the best approach, and yep this would ignore all users' local gitconfigs, according to here.
Seems much simpler to configure git to handle it, I think something like this would be best:
.js text eof=crlf
.ts text eof=crlf
.tsx text eof=crlf
I am not sure about how this setup would remove the need for some commit standardising all existing files in the index.
Yeah, my bad, we'll still need to lump in a commit to standardize this, but this seems simple enough with the --renormalize option
I assume that the proposed solution above is sufficient. I've added a PR to fix this issue #210
I may be looking into overall eslint improvements soon. I will try to factor this in and incorporate the PR's changes into that.