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 linter for textlint #1449

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@januswel
Copy link

januswel commented Mar 25, 2018

  • Fix not working
  • Use a nearest executable
  • Add tests for command_callback
Fix linter for textlint
- Fix not working
- Use a nearest executable
- Add tests for command_callback
@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Mar 26, 2018

Both what was on master and what you have here aren't correct. The local executable wasn't being used on master, and here you removed the fallback path for executing the script which will be needed on Windows. Neither version used node.exe to run the .js file, which will be needed for Windows. Your version uses %s, which means linting while you type wouldn't work any more, and the master version didn't have tests for the command. Both built the executable string in ugly ways.

I pushed a commit now to address these issues: 8b34a4b

@w0rp w0rp closed this Mar 26, 2018

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Mar 26, 2018

I just played around a bit, and everyone has missed a trick too. We can get rid of the -c config loading and use --stdin --stdin-filename %s just like we do with ESLint.

@januswel

This comment has been minimized.

Copy link

januswel commented Mar 26, 2018

Thank you for your comment and modifications.
I am so happy to be able to use textlint with ALE finally.

@januswel januswel deleted the januswel:fix-linter-textlint branch Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment