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

run script on temp file #61

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

run script on temp file #61

wants to merge 4 commits into from

Conversation

kufii
Copy link

@kufii kufii commented Mar 6, 2018

Previously it would only run on the saved file, meaning if you ran it and had unsaved changes, those changes would be lost. This copies the text to a temp file, runs eslint --fix on that, and returns the results

Previously it would only run on the saved file, meaning if you ran it and had unsaved changes, those changes would be lost. This copies the text to a temp file, runs eslint --fix on that, and returns the results
@elicwhite
Copy link
Owner

This would cause ESLint to be running on the file from tmp, right? Since eslint configs can be set using overrides, this could cause files to be linted with the wrong configuration since it might no longer match the correct glob, right?

@kufii
Copy link
Author

kufii commented Mar 7, 2018

oh yeah good point, give me a bit

the correct eslint config is now used. stilll uses the os temp directory on unsaved files
@kufii
Copy link
Author

kufii commented Mar 7, 2018

Ok check it now, fixed the issue

@kufii
Copy link
Author

kufii commented Apr 16, 2018

Hey, any updates or comments on this PR?

@skeggse
Copy link
Collaborator

skeggse commented May 19, 2020

It seems to me like there's a risk that the eslint configuration will ignore certain patterns of files, and that it's difficult to guarantee that the temporary file created will match that pattern. What do you think about fixing this via #34 instead?

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

Successfully merging this pull request may close these issues.

3 participants