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

Add jshint linter #2

Merged
merged 3 commits into from
Sep 16, 2016
Merged

Add jshint linter #2

merged 3 commits into from
Sep 16, 2016

Conversation

ckyrouac
Copy link
Contributor

No description provided.


call ALEAddLinter('javascript', {
\ 'executable': 'jshint',
\ 'command': 'jshint --reporter unix --config ' . g:ale_jshint_config_loc . ' -',
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 use a default for the config location or make the config option in some way. I now offer a command_callback option which lets you generate the command string which will be run in a job. The function takes a buffer number as its only argument. You can look at the shell linter I just added around an hour ago for an example of its use.

Thank you for adding more tools! You contribution is much appreciated. I need to add an option for selecting which linters you want, which is part of why I now require a 'name' option in the dictionary here for identifying which linter it is for which filetype. I'll add that once my RSI clears up. I've been having some issues recently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry - I forgot to include plugin/ale/aaflags.vim in this commit. Force pushed an update. Let me know if you like it.

Thank you for writing this! I've been using it all day.. it's very useful and speedy.

Copy link
Member

Choose a reason for hiding this comment

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

No problem. I'll keep adding more to it especially after I am able to recover from some recent RSI issues I have been experiencing.

@w0rp
Copy link
Member

w0rp commented Sep 15, 2016

One thing I'm wondering about is that I might make all of the linters change directory through some means to the directory of the file before running, to make sure they search up parent directories to find a configuration file. A good default for JSHint might be to not specify any configuration files at all in the command, and let JSHint look up parent directories to find one. That's what I used to do with JSHint before I switched to eslint. I think it works in Vim for me at the moment because I separately change directories automatically when I can.

" Set this to the location of the jshint configuration file
if !exists('g:ale_jshint_config_loc')
let g:ale_jshint_config_loc = '.jshintrc'
endif
Copy link
Member

Choose a reason for hiding this comment

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

Oh sorry, I only just saw this now. This conflicts with some recent changes I did. Check my comments about reading from parent directories as default. I would maybe move this flag into the linter file itself, and keep only the options which apply to all linters in this file.

@ckyrouac
Copy link
Contributor Author

Looking up a config recursively sounds cool. JSHint is a little annoying because it doesn't look for a default configuration file when in stdin mode. That's why I added the flag. I'll clean it up more tonight.

@w0rp
Copy link
Member

w0rp commented Sep 15, 2016

Ah, I see. That sounds unfortunate. eslint does do that with the stdin option. I wonder if it might be related to a need to switch directories, if it's just always based off of the filename you pass in. eslint also has a filename option for a hint. Maybe there is something there.

I might look into always somehow changing directories when running jobs, maybe something like cd dirname % && ... for commands or similar.

@w0rp
Copy link
Member

w0rp commented Sep 15, 2016

You could also write the "search up parent directories" part yourself, as you can now write callbacks for the command to run which will let you do that, if you look at what ALEAddLinter will now accept on master. I have been thinking I'll probably need a utility function at some point for doing that. Maybe something like ale#util#FindConfig(buffer, filename_callback).

@ckyrouac ckyrouac force-pushed the jshint-linter branch 2 times, most recently from 81bad73 to 071c5c4 Compare September 16, 2016 00:29
@ckyrouac
Copy link
Contributor Author

I force pushed the change to move the jshint_config_loc flag to the jshint.vim file.

@w0rp
Copy link
Member

w0rp commented Sep 16, 2016

I'll take this as it is for now. If you or others want to improve it, they can go ahead and do that. I'll write that function for finding files in parent directories at some point soon.

@w0rp w0rp merged commit c84bafe into dense-analysis:master Sep 16, 2016
justinyhuang pushed a commit to justinyhuang/ale that referenced this pull request Feb 14, 2020
hsanson pushed a commit that referenced this pull request Feb 3, 2022
* enable using cpplint for c

* fix tag alignment

* fix tag alignment trial #2

Co-authored-by: Justin Huang <justin.huang@perceive.io>
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