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 a cargo checker for syntastic #132

Merged
merged 3 commits into from
Aug 14, 2017
Merged

Conversation

jlevesy
Copy link
Contributor

@jlevesy jlevesy commented Dec 30, 2016

Hi there !

As cargo check has been merged in cargo (see rust-lang/cargo#3296), and the current rustc checker is facing some issues with external crates (see #130), I gave a try implementing a cargo checker for syntastic.

And here is the result. Please note it is my first time writing a syntastic checker, and I'm relatively new to the rust ecosystem, consequently I might have made mistakes, so any feedback will be greatly appreciated !

It currently works with cargo 0.16.0 nightly.

Thanks !

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rhys-vdw
Copy link

rhys-vdw commented Jan 7, 2017

Hey @jlevesy, I'm experiencing the same issue as #130 (I believe). I'm installed your branch but am not seeing a difference. Should cargo appear in the list of available checkers on a call to SyntasticInfo? (I have only a vague understanding of how Syntastic works, excuse my ignorance.)

@jlevesy
Copy link
Contributor Author

jlevesy commented Jan 8, 2017

Hey @rhys-vdw, thanks for giving a try !

cargo is indeed supposed to appear as an available checker.
What version of cargo are you using ?

@rhys-vdw
Copy link

rhys-vdw commented Jan 8, 2017

I had done a totally fresh rustup install in case that was the issue. I'm on:

cargo 0.15.0-nightly (298a012 2016-12-20)

:SyntasticInfo output:

Syntastic version: 3.8.0-14 (Vim 704, Neovim, Darwin)                                                                                                                                                                                             
Info for filetype: rust                                                                                                                                                                                                                           
Global mode: passive                                                                                                                                                                                                                              
Active filetypes: javascript ruby rust                                                                                                                                                                                                            
Filetype rust is active                                                                                                                                                                                                                           
The current file will be checked automatically                                                                                                                                                                                                    
Available checker: rustc                                                                                                                                                                                                                          
Currently enabled checkers: -   

Also I went into my bundles and confirmed that syntax_checkers/rust/cargo.vim is present.

@jlevesy
Copy link
Contributor Author

jlevesy commented Jan 9, 2017

Hmmm, can you change your toolchain to nightly by runing rustup default nightly or rustup override set nightly (only to override toolchain for a specific project) ?

I think it will do the job, now cargo is 0.17.0-nightly.

Actually cargo check is embedded from version 0.16.0, below that you have to install an external plugin. So I chose to disable this checker, for cargo versions < 0.16.0 (see here).

@rhys-vdw
Copy link

rhys-vdw commented Jan 9, 2017

Thanks @jlevesy. Working nicely! I can now see cargo in the available checkers, and have enabled it with:

let g:syntastic_rust_checkers = ['cargo']

@svenstaro
Copy link

I'm using this new checker locally and it's working really well. One thing I'd really like is if syntastic somehow automatically switched to the correct checker (rustc for single rust files, cargo for projects with a Cargo.toml) but that might be out of scope. Anyway, this checker is good to go!

@hasufell
Copy link

hasufell commented Mar 11, 2017

Doesn't work for me. I get no errors and no reported problems with cargo as checker, although the syntax is completely broken. rustc still works (via syntastic), but has the mentioned bugs.

https://github.com/mckinnsb/rust.vim/blob/master/syntax_checkers/rust/cargo.vim This one works perfectly fine for me though.

@torkleyy
Copy link

👍 Should also work with stable Rust as of Rust 1.16

@drozdziak1
Copy link

Why isn't this merged yet?

@jlevesy
Copy link
Contributor Author

jlevesy commented Jun 15, 2017

Sorry, I was away from my computer for a few months...

@hasufell, can you give me your rustc, cargo, vim and Syntastic versions please ?
Thanks

\ '%C %#--> %f:%l:%c'

return SyntasticMake({
\ 'makeprg': makeprg,
Copy link
Contributor

@hcpl hcpl Jun 28, 2017

Choose a reason for hiding this comment

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

@jlevesy apparently, 'cwd': expand('%:p:h') here is what solved the problem for @hasufell (taken from https://github.com/mckinnsb/rust.vim/blob/master/syntax_checkers/rust/cargo.vim#L40).

The current version doesn't work if you open a file from outside of the project it resides, because:

  • cargo check needs to be called within the project,
  • but Syntastic inherits vim environment when SyntasticMake is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, thank you for the review.

Choose a reason for hiding this comment

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

yes, that works

@jacwah
Copy link

jacwah commented Jul 29, 2017

This seems like a great patch! It's essential to any Vim user actually -- the rustc formatter is very limited. I'm currently testing it out, and the checker has worked well so far.

@snboisen
Copy link

What prevents this from being merged? Anything we can do to help? The rustc checker is useless for cargo-based projects and I would expect that to be the overwhelming majority of projects?

@steveklabnik
Copy link
Member

What prevents this from being merged?

  1. I don't use syntastic
  2. there are tons and tons of comments that I don't know how to evaluate

so, I've been kinda paralyzed. However, it seems this does work for lots of people, so I'm just going to merge it, and if it breaks stuff, well, people can send in new PRs to fix them.

Sorry to everyone in this thread for not taking this on sooner, as I've said elsewhere, if people want to help maintain rust.vim, please get in touch.

@steveklabnik steveklabnik merged commit a54bc38 into rust-lang:master Aug 14, 2017
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.