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

Autofix support? (Javascript here, ESLint) #541

Closed
tunnckoCore opened this Issue May 8, 2017 · 51 comments

Comments

Projects
None yet
8 participants
@tunnckoCore
Copy link
Contributor

tunnckoCore commented May 8, 2017

Hi there! First of all, i'm new to Vim, so sorry me in advance :D

I'm curious is there a way to automatically fix the errors the some linters gives you? For example, I'm Nodejs dev and I'm using ESLint, so.. is it possible to use the ESLint's --fix flag somehow? It's probably not a job of this (ALE) plugin, but using totally different plugin (like Neoformat) or Vim internal things, sounds like too much to me.

Also I just noticed that if I use Neoformat for example it seems it is synchronous, so it's blocking the whole work, until "fixable" errors are fixed. And no to mention that the file is pretty small ~100lines and it's shocking that is so slow.

If it isn't possible with ALE, what you suggest?

@tunnckoCore tunnckoCore changed the title Autofix support? Autofix support? (Javascript here, ESLint) May 8, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 8, 2017

This plugin won't support fixing any problems, but I will eventually write a separate plugin that does.

@w0rp w0rp closed this May 8, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 8, 2017

Actually, I'm going to need a lot of the same code to do the same things any way. I might as well just make fixing problems part of the same project.

@w0rp w0rp reopened this May 8, 2017

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 8, 2017

Yea, I was thinking the same think if it is possible. Glance look over the codebase and seems it is, yea.

Thanks for the fast response in any way! It would be great feature here, or even if in another plugin anyway

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 9, 2017

@w0rp ping me at twitter (again @tunnckoCore) or somewhere here at github, please. No matter such feature is added to this plugin or totally another plugin is created. :)

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 20, 2017

@w0rp I think it would be great to allow the user to set an option so that ale autofixes issues on save only. I guess autofixing on save doesn't break anything and it's required for ale so that it can be a replacement for neomake.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 20, 2017

@sassanh Hold that thought for a few hours.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 20, 2017

@tunnckoCore Okay. I have been working on adding the ability to fix files with asynchronous jobs off and on for a few days, and I just merged it into master. Check out :help ale-fix for detailed information. I added eslint --fix and autopep8 for now, plus one function for removing blank lines at the end of files.

You can configure it like so:

let g:ale_fixers.javascript = [
\ 'eslint',
\ 'remove_trailing_lines',
\ {buffer, lines -> filter(lines, 'v:val !=~ ''^\s*//''')},
\]

The above will configure ALEFix for JavaScript so it runs eslint --fix with the usual node_modules detection and so on, run a function to remove trailing blank lines, and the lambda example above will remove all line comments. (It's the easiest example I can think of.)

ALEFix is kind of the inverse of ALELint, in that it fixes files, it's only invoked manually, and no tools are configured to be run by default. You have to choose how you want to fix your files.

@wavded

This comment has been minimized.

Copy link

wavded commented May 20, 2017

Hey @w0rp, thanks for working on this, it seems like it will replace fixmyjs.vim for me. Pulling master and using it with neovim and a global eslint install, it doesn't appear to be working for me. Here is my config:

let g:ale_fixers = {}
let g:ale_fixers.javascript = ['eslint']

Then I'm running it on a fixable file with :ALEFix but nothing is happening. Am I doing something wrong?

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 20, 2017

@wavded Can you provide an example eslint configuration and some lines you're trying to fix?

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 20, 2017

@wavded i believe you should not add let g:ale_fixers = {}, but not sure, since i'm new to vim(script)

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 20, 2017

@w0rp that's amazing!! i'll try it when i can! Thanks for the great work. 👏 🍻

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 20, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 20, 2017

Ah, I need to add a few more possible filenames in there. The configuration file can have many names.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 20, 2017

3532257 Okay try again.

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 20, 2017

Thanks.

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 20, 2017

Ah, I need to add a few more possible filenames in there.

@w0rp hahahah, yep, that was that i seen before and what i was searching last few minutes & secs: "daaamn i know there was only one thing... hm" ;d

Btw, is it possible to implement "run fixers on save" or such? Through option (variable or whatever it is called in vim world), like it is for the linting let g:ale_lint_on_save.

Or if you don't want to implement it, can it be easy fix? So i can add it manually?

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 20, 2017

@tunnckoCore you can add this to your vimrc/init.vim:

autocmd BufWritePost *.js,*.jsx,*.py ALEFix
@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 21, 2017

Oh yea, forgot that.

Actually the whole code and vimscript is pretty easy to understand so i implemented few things. I can PR when get some time. :)

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 21, 2017

I don't think I'd recommend fixing files automatically on save. I suppose you could modify a file after saving it, but fixing files should be mostly non-blocking, so it can't happen before a file is saved.

What we could do is save a file, trigger fixes, writefile(), maybe set nomodified, and reload the buffer. That would work. That might be the only way for some future tools to run. Plus, it might be possible to do all of that from another buffer.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 21, 2017

Plus, it might be possible to do all of that from another buffer.

I mention that because it's not possible to setline() from another buffer in Vim, so ALE either sets the lines immediately if the buffer is still open, or waits for the buffer to be shown again and then calls setline().

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented May 21, 2017

I don't think I'd recommend fixing files automatically on save. I suppose you could modify a file after saving it, but fixing files should be mostly non-blocking, so it can't happen before a file is saved.

  1. Why not recommending it?
  2. It works, it feels a lot better than the other similar things (neoformat for example). Don't know if it not work correctly if the file is opened and modified from multiple places. In any way, that's not my case, so it work for me.

I implemented 3) Prettier ESLint and it works and feels fantastic. 4) And also :

let g:ale_fix_on_save
let g:ale_fix_on_enter
let g:ale_fix_on_insert_leave
let g:ale_fix_on_text_changed

copy pasted from the ale_lint_*

Do you want separate PRs for 3 and 4?

@wavded

This comment has been minimized.

Copy link

wavded commented May 22, 2017

@w0rp sorry for the delay, whatever you did fixed it for me, I was using .eslintrc if that made a difference.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 22, 2017

For fixing files automatically after saving them, we need to write the changes to the file again, probably using writefile(), and deal with reloading the buffer. Then it will work. We also need to consider what happens when you use use :x or :wq.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 22, 2017

One option that might work for saving and closing files is remembering the filename when fixing starts, and writing to that file after fixing ends, if and only if fixing is being done for the "save" event.

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 23, 2017

@w0rp Would it be possible to have the fixer look for eslint_d?

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 23, 2017

Possibly. For now, you can set the executable path for that with the options.

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 23, 2017

@w0rp Sorry, just making sure I understand: You're talking about this option: let g:ale_javascript_eslint_executable = 'eslint_d' not let g:ale_fixers.javascript correct?

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 24, 2017

@joeybaker yeah, I'm using eslind.d with:

let g:ale_javascript_eslint_executable='eslint_d'

eslint_d is in the PATH.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 25, 2017

@joeybaker Yes, g:ale_javascript_eslint_executable.

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 25, 2017

@w0rp thanks. Any chance autoFixing isn't reading that correctly? I'm suspicious because when I call eslint_d manually, fixes are super fast, but ALEFix is noticeably slower.

@sassanh

This comment has been minimized.

Copy link

sassanh commented May 25, 2017

@joeybaker When I set g:ale_javascript_eslint_executable to an invalid string ALEFix doesn't raise an error but it doesn't fix anything at all. So if it's fixing it's running eslint_d. Why it runs slowly with ALEFix for you is still questionable. It runs fast on my machine.

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 25, 2017

@sassanh thanks! I'll dig around a bit more.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 25, 2017

Look at the output of :ALEInfo and see :help g:ale_javascript_eslint_use_global

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 25, 2017

@w0rp Thanks! I overlooked the need for g:ale_javascript_eslint_use_global that did the trick!

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 25, 2017

@joeybaker I just pushed a commit which will now detect eslint_d automatically in node_modules when available, so you should now be able to install eslint_d locally, and it will be detected without any options being set.

@joeybaker

This comment has been minimized.

Copy link

joeybaker commented May 25, 2017

Many thanks @w0rp!

@w0rp w0rp added this to the Version 1.4 milestone May 26, 2017

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented May 26, 2017

I'll close this issue, as basic support is there now.

@w0rp w0rp closed this May 26, 2017

@adamyonk

This comment has been minimized.

Copy link

adamyonk commented Jun 1, 2017

Could this be expanded to support prettier directly (instead of going through prettier-eslint), or is it already possible to configure that way?

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 1, 2017

@adamyonk Both are now supported on master. They are suggested by :ALEFixSuggest.

@adamyonk

This comment has been minimized.

Copy link

adamyonk commented Jun 1, 2017

Thanks for pointing me in the right direction! Here's the config I used to get it working, in case it's helpful to anyone else:

let g:ale_fixers = { 'javascript': ['prettier'] }
let g:ale_javascript_prettier_options = '--no-semi --trailing-comma es5'
@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 1, 2017

I'm just waiting until the prettier guy realises that "zero configuration" is a pipe dream, and adds configuration file support for prettier.

@adamyonk

This comment has been minimized.

Copy link

adamyonk commented Jun 1, 2017

It really is crazy that it's made it this far without it. It sure makes getting a whole team to use the same settings more difficult than it could be. 😐

@tunnckoCore

This comment has been minimized.

Copy link
Contributor

tunnckoCore commented Jun 1, 2017

"zero configuration"

It is, zero configuration. The problem is that community has different opinions :D

@adamyonk

This comment has been minimized.

Copy link

adamyonk commented Jun 1, 2017

Another quick question about fixers: Using BufWritePre to ALEFix results in a file that is dirty after the save, what do you all use to get around that?

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Jun 1, 2017

See :help g:ale_fix_on_save. Fixing files on save requires some care, so ALE can handle that for you.

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Aug 7, 2017

Cool, that's probably a good way to configure it. I might use that myself.

@sassanh

This comment has been minimized.

Copy link

sassanh commented Aug 7, 2017

What's the good way to configure it?

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Aug 7, 2017

Huh, the guy deleted his comment. I'll show you a shortened version of what he originally included here.

" Fix JavaScript code with ESlint
let g:ale_fixers = {}
let g:ale_fixers.javascript = ['eslint']
# ESlint config as YAML
# Use prettier as a plugin for eslint, and set up options for it here.
plugins:
  - prettier
rules:
  prettier/prettier:
    - error
    - singleQuote: true
      trailingComma: all
@sharils

This comment has been minimized.

Copy link
Contributor

sharils commented Aug 7, 2017

Thanks for reposting it, after my post, strange things happened and I thought it didn't work and also because I just saw that prettier has an RFC for config files. So I deleted it. Again thanks a lot for reposting it. 👍

@w0rp

This comment has been minimized.

Copy link
Owner

w0rp commented Aug 7, 2017

Sure, no problem.

@l5x

This comment has been minimized.

Copy link

l5x commented Dec 30, 2017

@w0rp just wanted to say thank you for all the hard work!

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