Fix eslint errors on Windows

[addisonElliott]

On Windows, when running npm run lint, a LOT of the following errors
appeared:

 error  Expected linebreaks to be 'LF' but found 'CRLF'  linebreak-style

This error only occurs on Windows because the linebreak-style eslint
setting was set to "unix". This is fixed by making the eslint config
file a Javascript script and setting the platform based on the build
OS.

Thus, the linebreak-style will be "windows" or "unix" depending on the
users platform.

[codecov]

Codecov Report

Merging #4364 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4364      +/-   ##
==========================================
- Coverage   92.77%   92.75%   -0.03%     
==========================================
  Files         119      119              
  Lines        8448     8448              
==========================================
- Hits         7838     7836       -2     
- Misses        610      612       +2

Impacted Files	Coverage Δ
src/RestWrite.js	93.5% <0%> (-0.19%)	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	96.76% <0%> (-0.11%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7944e2b...b6f1acf. Read the comment docs.

[montymxb]

Can confirm, way too many linebreak errors during lint.

[flovilmart]

which is quite bad, as we'll end-up with mixed line breaks if we allow both.

[montymxb]

To add I think this may be better addressed via a .gitattributes and/or a .editorconfig file to force LF. If we merge this in we're not really guaranteeing that someone will be checking out CRLF and then committing LF style. They could even be checking out LF on windows, and then this would cause problems.

@addisonElliott can you instead make this function via gitattributes on a per-repo basis as mentioned above? We could also do editorconfig, but not really sure that's needed if gitattributes is properly set.

It could be nice, if we can, to auto-set checkout CRLF commit LF only on windows so nothing there gets mad at the files. In that case we could keep the change you've proposed here.

[montymxb]

@flovilmart the CI should catch PRs with CRLF style line endings with the same error as was reported initially here. I don't believe we'd have an issue considering that, even if someone changed the line endings in a commit, should just fail.

[flovilmart]

The issue is with git: For example, the default behavior of git on Windows systems is to convert LF linebreaks to CRLF when checking out files, but to store the linebreaks as LF when committing a change. This will cause the linebreak-style rule to report errors if configured with the "unix" setting, because the files that ESLint sees will have CRLF linebreaks. If you use git, you may want to add a line to your .gitattributes file to prevent git from converting linebreaks in .js files:

…

On Nov 21, 2017, 16:12 -0500, Benjamin Wilson Friedman ***@***.***>, wrote: @flovilmart the CI should catch PRs with CRLF style line endings with the same error as was reported initially here. I don't believe we'd have an issue considering that, even if someone changed the line endings in a commit, should just fail. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[flovilmart]

https://eslint.org/docs/rules/linebreak-style

[flovilmart]

All should be explicitely eol=lf, we don’t have any bat file, so please remove this line.

[flovilmart]

This should be reverted

[addisonElliott]

If we implement the gitattributes file, then the line endings will be converted to LF upon pushing to repository. I removed this line because I found it to be a redundant check in lint.

Do you think it's still worthwhile to have in?

[montymxb]

We'll need this to ensure that our lint is appropriate. Even if it may be redundant, without it we're not actually linting properly.

[addisonElliott]

I added a gitattributes file. There is more information about it here.

Basically, when cloning the repository, all text files will be converted to the host platform's line endings. The exception is for files where the EOL is specified. I set *.sh files to use LF because I had issues running bootstrap.sh in Cygwin due to this. In addition, I made *.bat files use CRLF files regardless.

When committing files to the repository, the text files will be converted to LF. This will not affect your working directory line endings.

Since the conversion to the correct line endings will be done upon pushing, I removed the eslint check.

Let me check on this but I think the eol is how it will be stored for your local repository.

[flovilmart]

As per ESLint doc, you have to force all JS not to convert to CRLF, removing the lint line is not acceptable. See previous comments.

[addisonElliott]

@flovilmart I'm not sure that there should be an eol=lf for each of the filetypes. Maybe I'm misunderstanding what the docs are saying, but this is what I'm reading:

eol
This attribute sets a specific line-ending style to be used in the working directory. It enables end-of-line conversion without any content checks, effectively setting the text attribute. Note that setting this attribute on paths which are in the index with CRLF line endings may make the paths to be considered dirty. Adding the path to the index again will normalize the line endings in the index.

So setting an eol for each of the filetypes would force it to be LF line endings when cloning the repository which is not what you want for users on Windows since they will need CRLF.

The text=auto part will normalize the line endings for checking it into the repository to be LF regardless.

[addisonElliott]

Update: I did some testing with this branch to ensure that the gitattributes is doing what is desired.

I cloned the repository and branch and all files were converted to CRLF except the bootstrap.sh which had LF endings.

Next, I uploaded a Javascript file with CRLF line endings on Windows to a new branch. I cloned that on a Linux repository and it had LF line endings. That leads me to believe that it is being uploaded as LF. I can confirm it was uploaded as LF since I downloaded the file straight from Github and it had LF endings.

[flovilmart]

But please, leave the lint this way you'll know for sure if nothing as been wrongly changed when cloning on a windows machine.

[flovilmart]

Also, worth asking, what's your development environment on windows? editor, shell etc... perhaps we should recommend https://msdn.microsoft.com/en-us/commandline/wsl/about

[addisonElliott]

@flovilmart Sure thing, I restored the lint check. The line endings are not converted from CRLF to LF until after pushing to the repository so the lint check determines OS type and sets line break style from there.

I am using Cygwin as my shell and Notepad++ as my editor.

[montymxb]

@addisonElliott You would have to explicitly indicate text eol=lf for every file type to override your local checkout CRLF and commit LF style. It's a bit below the example here.

The rule @flovilmart was mentioning is this further down the page. Basically, as it stands we are strictly adhering to LF. I'm pretty sure Notepad++ plays nice with LF line endings too. If you modify the .gitattributes appropriately to preserve LF on checkout (against your global settings) your editor and others should respect it accordingly.

[addisonElliott]

I see the confusion now! My intention was to have the repository be checked out in whatever line ending suits the OS but convert to LF when committing. Then, my initial change to eslint could stay. I was following based on what you said:

It could be nice, if we can, to auto-set checkout CRLF commit LF only on windows so nothing there gets mad at the files. In that case we could keep the change you've proposed here.

However, it seems @flovilmart was thinking we should just have LF line endings upon checkout and then have the eslint check stay the same.

I'm fine with either, just let me know what would be preferred and I'll make the changes accordingly.

[montymxb]

Ahhh, sorry for the confusion in my comment there! Yes, I think it would be nice to have CRLF upon checkout (general habit), but most IDEs and editors and will also play nice with LF nowadays on windows. On that note, and considering we strictly want LF in all cases, I would also like us to go LF on checkout and revert the eslint check 👍 .

[addisonElliott]

@flovilmart @montymxb Okay, so hopefully this is the final commit!

Instead of doing eol=lf for each individual filetype, I did it for each file that is considered a text file and then I declare some file extensions that are text files.

On Windows, if I did eol=lf for each text file, I would receive a warning in git like Warning: LF file endings will be converted to CRLF for .eslintrc.json. This is because it is not one of the listed filetypes that have a forced EOL. So, instead, I set the eol=lf part for all files that are considered text and that warning went away on my side.

I tested this by cloning my repository and specific branch. All files (including ones like PATENTS which has no file extension) were LF line endings. In addition, running lint check received no errors!

[montymxb]

I took a look at this on windows and I can confirm this looks good! No more crazy amounts of lint errors. If you can just add those few extra types that would be fantastic.

A note for anyone else on windows that these changes only take effect on the initial clone (correct me if I'm wrong). If you already have an existing local repo and then receive these changes your line endings will remain as they were before, and that should be just fine.

[montymxb]

We should also add:

• *.txt
• *.yml
• *.sql
  Just to be thorough and cover our travis yml, a text test file and some .sql files we use for Postgres. Wouldn't want anything unexpected to sneak through on those.

[addisonElliott]

Good idea. Added

[flovilmart]

I've restarted the failed test, unrelated to those changes and will merge.

[montymxb]

Awesome, thanks again for helping us out with the windows setup @addisonElliott !