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

Added fixer for Standard linter #781

Merged
merged 2 commits into from
Jul 22, 2017
Merged

Added fixer for Standard linter #781

merged 2 commits into from
Jul 22, 2017

Conversation

sumnerevans
Copy link
Contributor

Addresses #780.

@w0rp, is there anywhere that the available fixers are listed?

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Add some Vader tests to cover these functions.


function! ale#fixers#standard#GetExecutable(buffer) abort
return ale#node#FindExecutable(a:buffer, 'javascript_standard', [
\ 'node_modules/standard/bin/cmd.js',
Copy link
Member

Choose a reason for hiding this comment

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

You should probably ensure that the cmd.js version is executed with node in the Fix function below on Windows. See the eslint function for an example.

@w0rp
Copy link
Member

w0rp commented Jul 19, 2017

@sumnerevans The tools are just listed in the table with the linters. :ALEFixSuggest suggests some things from the registry. We don't really need to explicitly mention anywhere that you can also fix files with certain things, it should basically be a given.

@sumnerevans
Copy link
Contributor Author

I'm in the process of adding the Vader tests (I've never done them before so it's a good learning opportunity 😄). If all goes well, I'll push sometime today or tomorrow.

@w0rp
Copy link
Member

w0rp commented Jul 20, 2017

If you run into trouble installing Docker for running all of the tests, your problem is probably going to be that you need to add your user to a docker group and log out and in again. That's the one part of setting up Docker which isn't explained too well.

@sumnerevans
Copy link
Contributor Author

@w0rp, I actually already have Docker installed and I was happy to see how easy it was to run the tests. (I also use Linux which always helps things go smoother 😉.)

@w0rp
Copy link
Member

w0rp commented Jul 20, 2017

Cool.

@sumnerevans
Copy link
Contributor Author

@w0rp, how does it look now? I added some stuff to the eslint test directory. Should I create a different directory instead? I could also rename the eslint-test-files folder to js-test-files.

@w0rp
Copy link
Member

w0rp commented Jul 21, 2017

The Fix function looks fine. I don't think you committed the tests, though.

@sumnerevans
Copy link
Contributor Author

@w0rp, whoops. I totally forgot to git add .. I updated and force pushed.

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Cool, looks good. 👍

@w0rp w0rp merged commit 1221748 into dense-analysis:master Jul 22, 2017
@w0rp
Copy link
Member

w0rp commented Jul 22, 2017

Cheers! 🍻

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.

None yet

2 participants