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

Override the :A command to toggle between go implementation code and test code. #684

Closed
peterhellberg opened this issue Jan 13, 2016 · 40 comments
Labels

Comments

@peterhellberg
Copy link

I’d like vim-go to have the functionality of https://github.com/benmills/vim-golang-alternate and possibly add support for optionally creating missing test files when toggling.

@fatih
Copy link
Owner

fatih commented Jan 18, 2016

Thanks @peterhellberg for the suggestion. This is a nice feature we can add 👍

@fatih fatih added the feature label Jan 18, 2016
@nhooyr
Copy link
Contributor

nhooyr commented Jan 28, 2016

@fatih Do we ask the author for permission or can we just take it and credit him?

@peterhellberg
Copy link
Author

The only issue on that repo is a question about what license it should have… opened on 24 Jan 2014

I wouldn’t hold my breath for a reply from @benmills, but we should at least try to contact him.

@nhooyr
Copy link
Contributor

nhooyr commented Jan 28, 2016

Alright, whenever you feel is good I can create the PR with the docs and everything.

@benmills
Copy link

Sounds good to me! Thanks for thinking of this @peterhellberg

@nhooyr
Copy link
Contributor

nhooyr commented Jan 28, 2016

Okie, i'll create a PR and stuff.

@nhooyr
Copy link
Contributor

nhooyr commented Jan 30, 2016

@peterhellberg do you really think we need an option to create the test file? Do you you think it should be universally enabled?

@peterhellberg
Copy link
Author

@nhooyr I’d be happy if the test file was automatically created.

It might be a good idea to be able to disable that feature using a let, what do you think?

Edit: And now when I looked at the PR it seems like you already did that :)

@peterhellberg
Copy link
Author

@nhooyr Is your code enabling the traditional commands :A :AV and :AS? (alternate, alternate vsplit and alternate split)

@nhooyr
Copy link
Contributor

nhooyr commented Jan 30, 2016

@peterhellberg oh I didn't know thats a actual thing. I just used :GoAlternate and a bunch of mappings. I can switch it to that but that might cause problems with other peoples mappings (:A is the mapping in fzf.vim for ag). Should I add a switch to turn this on then?

@nhooyr
Copy link
Contributor

nhooyr commented Jan 30, 2016

It's just that the other mappings (GoRun) use the same setup I implemented instead of overriding :A. There is no GoRunVertical, you instead use the mapping (go-run-vertical). And it uses a variable to decide the default way you want it. I mean if you really think its necessary, its not that hard to just add over the current code, its like 3 lines.

@nhooyr
Copy link
Contributor

nhooyr commented Jan 30, 2016

You can also just add this to your .vimrc

augroup go
    autocmd!
    autocmd Filetype go command! -bang A call go#alternate#Switch('edit<bang>')
    autocmd Filetype go command! AV call go#alternate#Switch('vsplit')
    autocmd Filetype go command! AS call go#alternate#Switch('split')
augroup END

@peterhellberg
Copy link
Author

I’m just so used to using a.vim (and for vim-golang-alternate for go) for so many years now that :A and :AV is lodged in my muscle memory.

I can definitely change my .nvimrc if you want to keep the default command as GoAlternate.

@nhooyr
Copy link
Contributor

nhooyr commented Jan 30, 2016

lets wait and see what @fatih thinks.

@peterhellberg
Copy link
Author

I’m going to Vietnam tomorrow and will leave my laptop at home, will be back in Sweden in a month.

@nhooyr I’m happy with letting you and @fatih decide on how to best implement this feature.

@fatih
Copy link
Owner

fatih commented Feb 1, 2016

I've made some comments to the PR #704 I think we shouldn't create a new file at all but it should be created if a bang ! is passed. Such as :GoAlternate! , this would force to create the file. Let's discuss it in the PR.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 4, 2016

@peterhellberg it was merged. here are the commands you need to put in your vimrc if you want your alternate commands to work.

augroup go
    autocmd!
    autocmd Filetype go command! -bang A call go#alternate#Switch(<bang>0, 'edit')
    autocmd Filetype go command! -bang AV call go#alternate#Switch(<bang>0, 'vsplit')
    autocmd Filetype go command! -bang AS call go#alternate#Switch(<bang>0, 'split')
augroup END

@fatih
Copy link
Owner

fatih commented Feb 4, 2016

@nhooyr I've just added myself for A. I think we can add them to our doc as this is something used by many already. Mind opening a PR ;) ?

@fatih fatih closed this as completed Feb 4, 2016
@fatih
Copy link
Owner

fatih commented Feb 4, 2016

Closed as we have this feature now.

@nhooyr
Copy link
Contributor

nhooyr commented Feb 4, 2016

@fatih should I add this to the readme. It sounds like something more suited for it.

@fatih
Copy link
Owner

fatih commented Feb 4, 2016

Readme is ok. But also add to the doc. The thing is README is getting long everytime we add a new feature and I don't want it to be very long. That's why I do a cleanup every 2-3 months. Vim users should look anything from the doc as that's is the place we should put.

@peterhellberg
Copy link
Author

Thank you guys! I’ll update my config when I get back to Europe :)

@peterhellberg
Copy link
Author

I’ve now updated my config and this feature works really well, but I get a vim-go: PANIC PANIC PANIC message when calling :A!

Is this the intended behavior?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

No, I'll look into it.

@peterhellberg
Copy link
Author

Note that it functions just fine, it is just an unexpected error message (possibly due to the created empty buffer not having a package line)

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

Yea I understood. I'm in class atm, i'll look into it at lunch.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

So it only happens in empty buffers?

@peterhellberg
Copy link
Author

@nhooyr Take your time, no stress :)

And yes, it only happens when an empty file/buffer is created.
Calling :A! on a file that has a corresponding test work just fine.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

I'm not stressed, its my way of having fun :P

alright thanks.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

PANIC PANIC PANIC is what gocode returns if there is an error, do you have autocomplete?

@peterhellberg
Copy link
Author

Yes, and I guess gocode is called when the buffer is switched.

Calling :A (with no trailing !) first shows the message vim-go: couldn't find foo_test.go and then after a few hundred ms vim-go: PANIC PANIC PANIC if foo.go is empty.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

I don't think that can be fixed then, its a harmless error message though.

@peterhellberg
Copy link
Author

No, unless populating the buffer with package <name> :)

I think I found the underlying issue now, and it was that I have let g:go_auto_type_info = 1 in my config.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

ah yes, that calls gocode as well.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

Maybe we shouldn't print the PANIC PANIC PANIC in that case?

@peterhellberg
Copy link
Author

It might too much of an edge case to really care about?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

I meant, ignore the PANIC PANIC PANIC if g:go_auto_type_info = 1

@peterhellberg
Copy link
Author

Probably not, but @fatih maybe want to decide on this?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

Yup. but I did find a bug in the implementation of this thanks to you. it doesn't handle empty filenames.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 10, 2016

Alright my improvements are in #758 and improvements to GoInfo in #759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants