Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo contains 107 files with windows CRLF line endings #1024

Closed
jspillers opened this issue Apr 27, 2020 · 7 comments
Closed

repo contains 107 files with windows CRLF line endings #1024

jspillers opened this issue Apr 27, 2020 · 7 comments
Labels
bug Concrete, reproducible bugs good first issue A relatively simple issue

Comments

@jspillers
Copy link
Contributor

jspillers commented Apr 27, 2020

Bug Report

There are 107 files in the repo that contain CRLF line endings as opposed to LF.

Steps to Reproduce:

  1. cd perspective
  2. find . | grep -v dist | grep -v node_modules | xargs file | grep 'CRLF' | sed 's/:.*$//'

Expected Result:

above one-liner should return zero lines

Actual Result:

it returns 107 files with windows line endings

@jspillers jspillers changed the title repo contains 66 files with windows CRLF line endings repo contains 107 files with windows CRLF line endings Apr 27, 2020
@MisterAwesome23
Copy link

Hello. I am new to perspective and would love to contribute.
I would like to fix this issue, if no one is already on it.

windows line endings

If I got this right- there are 107 files ending with "\r\n" instead of a "\n"
More on this here
So we need files ending in the unix format of a "\n" right?

@timkpaine timkpaine added bug Concrete, reproducible bugs good first issue A relatively simple issue labels Apr 28, 2020
@timkpaine
Copy link
Member

@MisterAwesome23 correct

@MisterAwesome23
Copy link

MisterAwesome23 commented May 1, 2020

I've replaced all the ending by unix endings now.

find . | grep -v dist | grep -v node_modules | xargs file | grep 'CRLF' | sed 's/:.*$//'

This now results in 0 files.

Link to entire changes-
https://pastebin.com/A7Pjn3wB

Creating a pull for this.
#1032

"we require a CLA for the following people : (@MisterAwesome23)"
Can someone guide me to be added to CLA so this pull can be merged?

@texodus
Copy link
Member

texodus commented May 8, 2020

Thanks for the report!

We've gotten a few "ctrl-f" style fixes for this, so it's worth clarifying what we would consider a proper fix for this issue. It's not quite enough to simply remove the CRLF characters - we don't want future ones committed either.

Our linter ESLint supports linebreak-style and the documentation indicates this should be set to \n by default. It is clearly not working - but we should find out why and fix it, then this will be asserted as a git hook via yarn lint, preventing future commits from containing bad characters, and the existing errors can be trivially repaired with yarn fix (which will prevent any PRs for this meeting copious merge conflicts).

@darjama
Copy link
Contributor

darjama commented Jun 3, 2020

I've submitted a pull request #1075 to modify the eslint settings and change the .gitattributes file so that .js files aren't altered to have CRLF line endings on checkout on a windows machine as documented here.

The pull request doesn't change the existing files, and at least on my machine using eslint --stdin --fix brings my processor to its knees with so many changes. It works if I go directory by directory though. This tool for VSCode is helpful for batching the changes for all files.
I've submitted an ICLA.

@darjama
Copy link
Contributor

darjama commented Jun 3, 2020

Also, some additional configuration on windows machines might be needed to avoid it overall: https://warlord0blog.wordpress.com/2019/09/04/vscode-crlf-vs-lf-battle/

@texodus
Copy link
Member

texodus commented Jun 9, 2020

Thanks @darjama, I think #1078 covers this perfectly!

@texodus texodus closed this as completed Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Concrete, reproducible bugs good first issue A relatively simple issue
Projects
None yet
Development

No branches or pull requests

5 participants