Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Fix file import resolution #340

Merged
merged 1 commit into from
Nov 28, 2015
Merged

Fix file import resolution #340

merged 1 commit into from
Nov 28, 2015

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Nov 28, 2015

Fixes #335

I originally didn't realize that process.cwd() is always the directory of the file being linted. Because of this, it's possible to pass in only the basename of the file, and ESLint will assume cwd is set correctly, and will resolve configs and imports.

But, this breaks use of .eslintignore files, so if one of those are found, we can set cwd to the project root and pass the relative path to eslint.

filePath = Path.relative(eslintignoreDir, params.filePath)
process.chdir(eslintignoreDir)
} else {
filePath = Path.basename(params.filePath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be nice to move this here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, completely missed that. Yes, indeed it would. One moment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, moved.

By default, process.cwd is the directory containing the file being linted, so
normally it is only necessary to provide the name of the file itself, with no path.

But, if a .gitignore file is present, eslint will not properly ignore a file if
cwd is not set to the project root when executing with text and providing
a filename, as we are doing. So, if we find an .eslintignore, we can simply
change the cwd to the root directory (same directory as the .eslintignore).
@steelbrain
Copy link
Contributor

LGTM, merge if CI passes

@IanVS
Copy link
Member Author

IanVS commented Nov 28, 2015

I guess AppVeyor isn't working correctly? https://ci.appveyor.com/project/Arcanemagus/linter-eslint/build/51

@Arcanemagus
Copy link
Member

AppVeyor will be broken till v1.3.0 is released (see atom/atom#9277), hence why it isn't marked as required 😉.

@IanVS
Copy link
Member Author

IanVS commented Nov 28, 2015

Thanks @Arcanemagus. This look okay to you?

@@ -19,9 +19,10 @@
},
"devDependencies": {
"babel-eslint": "^4.1.5",
"eslint-plugin-react": "^3.10.0",
"eslint-config-airbnb": "latest",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetising them? 😛

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm i --save-dev does this automatically, apparently.

@Arcanemagus
Copy link
Member

Looks good to me 👍.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants