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

support eslint --fix #51

Closed
nhooyr opened this issue Jan 29, 2017 · 14 comments
Closed

support eslint --fix #51

nhooyr opened this issue Jan 29, 2017 · 14 comments

Comments

@nhooyr
Copy link

nhooyr commented Jan 29, 2017

It would be nice to support http://eslint.org/docs/user-guide/command-line-interface#fix

@sbdchd
Copy link
Owner

sbdchd commented Jan 31, 2017

Okay, I have added eslint --fix to the dev branch.

Can you check that it works accordingly?

Thanks!

@nhooyr
Copy link
Author

nhooyr commented Jan 31, 2017

It's not working because eslint cannot find its configuration file. It cannot use a tempfile, it needs the path to the actual file. Or if you want to use a temp file, make the temp file in the same directory as the original file, this will allow eslint to find the configuration file.

The configuration file is a dotfile that is stored with the original js file.

@nhooyr
Copy link
Author

nhooyr commented Jan 31, 2017

Also, eslint --fix formats even with errors. You mentioned that it did not as a caveat in the commit message.

@sbdchd
Copy link
Owner

sbdchd commented Jan 31, 2017

When eslint --fix encounters an error, it outputs a non-zero status code, which means Neoformat won't update the buffer with the changed data.

@fwip
Copy link

fwip commented Feb 4, 2017

Another option is to let neoformat look for the config file and supply it to eslint with the --config option. You'd probably have to implement the complicated rules they use to find the right configuration file, though: http://eslint.org/docs/user-guide/configuring#configuration-file-formats

@manveru
Copy link
Contributor

manveru commented Feb 10, 2017

Just tried the dev branch, but even with

    let g:neoformat_enabled_javascript = ['prettier', 'eslint']

NeoFormat just ignores the eslint formatter and only runs the prettier one. There's no output regarding eslint with debug on either. Any idea why that might happen?

@nhooyr
Copy link
Author

nhooyr commented Feb 10, 2017

@manveru #51 (comment)

Also, prettier is first so it will be run first and since it succeeds, eslint will not be run.

@manveru
Copy link
Contributor

manveru commented Feb 10, 2017

OK, but shouldn't it at least give some output of trying to run eslint? I'm going to build a formatter based on https://www.npmjs.com/package/prettier-eslint-cli now, that should also get around the config file problem.

@manveru
Copy link
Contributor

manveru commented Feb 10, 2017

Seems like that works:

function! neoformat#formatters#javascript#prettiereslint() abort
    return {
        \ 'exe': 'prettier-eslint',
        \ 'args': ['--stdin'],
        \ 'stdin': 1,
        \ }
endfunction

@jondot
Copy link
Contributor

jondot commented Feb 11, 2017

thanks @manveru that works. I would still wait out for just eslint --fix since prettier has a strong opinion about how code that is linted OK should be formatted which I don't like, and it adds a considerable overhead for CPU that eslint already takes :(

@manveru
Copy link
Contributor

manveru commented Feb 11, 2017

Yeah, I totally understand, just thought it might help other people like me that need both combined anyway. On my machine at least, I don't notice any performance issues, saving (which is when I run the formatter) is almost instant. It might depend on the rules you specify for eslint?

@BenoitZugmeyer
Copy link
Contributor

BenoitZugmeyer commented Mar 9, 2017

After reading about this issue, I made a ESLint fork to support fixing code from stdin, allowing it to be used as a neoformat formatter. It worked great for me so far. This may not be a long term solution but it may interest those who want to use ESLint as a formatter without Prettier. I'm open to suggestions!

@daikikohara
Copy link

I'm using the prettier-eslint solution.
But stdin option doesn't seem to ignore files in .eslintignore.
Does anyone know the workaround for that?

@nhooyr
Copy link
Author

nhooyr commented Nov 22, 2019

Going to close as it has been a long time and I'm not sure if this is relevant anymore.

@nhooyr nhooyr closed this as completed Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants