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

Error when there's a \r in .eslintrc #1172

Closed
gyandeeps opened this issue Aug 18, 2014 · 10 comments
Closed

Error when there's a \r in .eslintrc #1172

gyandeeps opened this issue Aug 18, 2014 · 10 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features

Comments

@gyandeeps
Copy link
Member

One of the test case failed on my machine. I just forked the repo and it failed.

The test case is:

var filepath = path.resolve(__dirname, "..", "fixtures", ".eslintignore2");
it("should return true for file matching a child of an ignore pattern", function() {
    var ignoredPaths = IgnoredPaths.load({ ignore: true, ignorePath: filepath });
    assert.ok(ignoredPaths.contains("undef.js/subdir/grandsubdir"));
});

The content of eslintignore2 is:

undef.js

Am I doing something wrong or is it a problem?

UPDATE
I think the eslintignore2 file has 2 rows. second row is empty. Thats why it fails.
I used notepad++ to open the file.
Can any one confirm this on windows 7 machine?

Also after split("\n") operation is done on the file content it gives an array like this

[ 'undef.js\r' , '' ]

Also after doing filter(nonEmpty) on it, it refines the array to be like this

[ 'undef.js\r' ]

The above split and filter is done here: https://github.com/eslint/eslint/blob/master/lib/ignored-paths.js#L44

I think the mystery is \r thing.
http://stackoverflow.com/questions/1279779/what-is-the-difference-between-r-and-n

Discussions around this area within eslint:

@nzakas nzakas changed the title Test failing: ignored-paths file Error when there's a \r in .eslintrc Aug 18, 2014
@nzakas
Copy link
Member

nzakas commented Aug 18, 2014

This looks like we just need to split on /\r?\n/ instead of just "\n".

@gyandeeps
Copy link
Member Author

will have fix with PR ready in few.
Thanks

@michaelficarra
Copy link
Member

How did a failing test get on master?

@gyandeeps
Copy link
Member Author

I think it only happens on windows machine. Travis build never failed.
For more info http://stackoverflow.com/questions/1279779/what-is-the-difference-between-r-and-n

@nzakas
Copy link
Member

nzakas commented Aug 19, 2014

The test isn't failing on master. I'm also on windows 7 and have no problem.

@gyandeeps you should double check that git isn't automatically adding windows-style line endings on your machine.

@gyandeeps
Copy link
Member Author

@nzakas You are correct. It has something to do with my git config.
I tried it on my friends machine and none of the test failed.
Do you think i should close this issue and the PR #1174

@nzakas
Copy link
Member

nzakas commented Aug 19, 2014

No, I think this is a legitimate issue where we will be handling Windows-escaped files incorrectly. That should be fixed.

@gyandeeps
Copy link
Member Author

Any idea how I can make a file to have windows style escape and then create test against it. I will have to save that file inside test. How can i force a file to have \r escapes in it?
Any help would be appreciated. Thanks.

@michaelficarra
Copy link
Member

nzakas added a commit that referenced this issue Sep 5, 2014
Fix: Error when there's a \r in .eslintrc (#1172)
@nzakas
Copy link
Member

nzakas commented Sep 6, 2014

Commit message format error left this issue open, but this has been fixed.

@nzakas nzakas closed this as completed Sep 6, 2014
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects
None yet
Development

No branches or pull requests

3 participants