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

Fix 2816 - Standard fix does not work. #3046

Merged

Conversation

hsanson
Copy link
Contributor

@hsanson hsanson commented Mar 12, 2020

The standard linter --fix fails if the file being input is not relative
to the project root (standard/standard#1384).

This MR attempts to fix this by changing the command so the input file
is relative to the project root and the output is to a temporary file.

Preliminary tests with toy javascript projects seem to indicate this
works fine.

Where are the tests? Have you added tests? Have you updated the tests? Read the
comment above and the documentation referenced in it first. Write tests!

Seriously, read :help ale-dev and write tests.

The standard linter --fix fails if the file being input is not relative
to the project root (standard/standard#1384).

This MR attempts to fix this by changing the command so the input file
is relative to the project root and the output is to a temporary file.

Preliminary tests with toy javascript projects seem to indicate this
works fine.
Copy link

@yveslange yveslange left a comment

Choose a reason for hiding this comment

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

Seems correct to specify the stdin

@hsanson hsanson requested a review from w0rp April 17, 2020 13:31
@Frando
Copy link

Frando commented Apr 24, 2020

Found this after being biten by #2816. I tested and can confirm that this PR fixes :ALEFix with StandardJS 14.3.3.

@stale
Copy link

stale bot commented Aug 13, 2020

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs a bot will close automatically label Aug 13, 2020
@stale stale bot closed this Aug 15, 2020
@hsanson hsanson reopened this Aug 15, 2020
@stale stale bot removed the stale PRs a bot will close automatically label Aug 15, 2020
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.

I'll merge this.

I recommend everyone use ESLint instead of standard, with a standard configuration file if you really want all of the rules for StandardJS. I've never understood why there's a separate executable that behaves differently when it's really just ESLint under the hood.

@w0rp w0rp merged commit ac56574 into dense-analysis:master Aug 17, 2020
@w0rp
Copy link
Member

w0rp commented Aug 17, 2020

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

4 participants