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

Added support for rustfmt before save #49

Closed
wants to merge 3 commits into from

Conversation

grncdr
Copy link
Contributor

@grncdr grncdr commented Oct 3, 2015

I really enjoyed this feature from vim-go
so I copied it for rust & rustfmt.

I really enjoyed this feature from [vim-go](https://github.com/fatih/vim-go) so
I copied it for rust & rustfmt.
@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. 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.

@@ -0,0 +1,80 @@
" WTF License
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a "real" license, so I wonder if we can take it...

@steveklabnik
Copy link
Member

General 👍 on this idea, for sure.

@grncdr
Copy link
Contributor Author

grncdr commented Oct 3, 2015

I'm happy to update the license, was just a bit lazy when putting this together. The vim-go plugin I was working off of is BSD licensed, this repo has no license, and I guess rust itself is licensed under the Apache & MIT licenses? Would appreciate a recommendation as to the correct license to satisfy all of the above.

A more annoying (for me anyways 😉) problem: the vim-go plugin works by writing the buffer contents to a temporary file, then calling gofmt on that. This doesn't work for rust crates with multiple files, because rustfmt will fail when it can't find other modules in the crate. I'm working out a solution, but for now I'd slap a big "WIP" label on this.

@steveklabnik
Copy link
Member

All Rust stuff is MIT/ Apache2, we forgot to add the license locally when we moved it out of the main repo

Sent from my iPhone

On Oct 3, 2015, at 12:53, Stephen Sugden notifications@github.com wrote:

I'm happy to update the license, was just a bit lazy when putting this together. The vim-go plugin I was working off of is BSD licensed, this repo has no license, and I guess rust itself is licensed under the Apache & MIT licenses? Would appreciate a recommendation as to the correct license to satisfy all of the above.

A more annoying (for me anyways ) problem: the vim-go plugin works by writing the buffer contents to a temporary file, then calling gofmt on that. This doesn't work for rust crates with multiple files, because rustfmt will fail when it can't find other modules in the crate. I'm working out a solution, but for now I'd slap a big "WIP" label on this.


Reply to this email directly or view it on GitHub.

@grncdr
Copy link
Contributor Author

grncdr commented Oct 3, 2015

Given that there isn't a license on anything else, I've simply removed that line.

@IanConnolly
Copy link
Contributor

Was doing this myself!

@grncdr Could you add a :RustFmt command that calls the function? No reason why the user couldn't define it themselves, but I feel like rust.vim has the right to do it for them :P

@steveklabnik What's outstanding for @grncdr to do to get this merged? Happy to help.

@steveklabnik
Copy link
Member

I'm horrible at vimscript, so I'm not sure. Let's say that yeah, we need that :RustFmt command and then let's get this merged. I like the idea of the command so that other stuff can build on top of it, and it seems consistent with the rest of the bindings.

@IanConnolly
Copy link
Contributor

@grncdr I know the vim-go function adds errors to the quickfix list, but do you think the location list might be a better option? That way you can keep the errors for multiple windows in their respective location lists. Syntastic adds errors to the loclist for this reason.

EDIT: Sidenote as they're not linked, this'll fix #43.

@oblitum
Copy link

oblitum commented Apr 24, 2016

So, I guess this feature has been merged in #52, but this pull was left dangling?

@mbyio
Copy link
Contributor

mbyio commented Jul 27, 2016

Yeah, it looks that way to me.

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

6 participants