-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
:GoAlternate and related documentation/mappings #704
Conversation
I think its better to have a variable to control the default positioning of the alternate file like GoRun rather then how I pass arguments. I'm gonna change it. |
Ok so I changed it and added a setting named g:go_alternate_mode. I think we should rename these settings (g:go_term_mode) to instead use a suffix of cmd as thats what they really are. They aren't a mode. Its just simply the command used to open the new file. I've updated the documentation on g:go_term_mode to reflect that. Its much more clear. There was also a mistake I corrected in the README.md where you cannot set g:go_term_mode to tab as that isn't the command to open a new tab. Its much more clear to say its a command. It also escapes the filename and other stuff. |
|
||
" By default create the test file if it does not exist | ||
if !exists("g:go_create_test_file") | ||
let g:go_create_test_file = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you disable this? It should give an error if the file doesn't exists. Having it enabled by default will cause to create unwanted files. We should be very simple in the features and not disturb people's workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I think we don't need this setting at all. Let's do this way. If e bang is passed it should force open the test file, if not passed it should give an error. Example:
Tries to open foo_test.go
, if it doesn't exists it echoes an error:
:GoAlternate
Tries to open foo_test.go
, if it doesn't exists it creates the foo_test.go
file.
:GoAlternate!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thing is, its not really creating a file, it just create a buffer with the name of the file. Didn't want to do that? Pretty simple to just delete the new buffer. I'm using the bang right now so that if the current buffer isn't saved you can switch with the bang, so it matches other commands and their behaviour. Plus its nice for mappings so you don't have to type the entire command just to create the new file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change that. The bang should create a file if passed if not it should give an error: no test file found. Call :GoAlternate! to create one
. With that also remove the let g:go_create_test_file = 1
option completely.
Hi @nhooyr Good job 👍 I have a some comments, which I think makes it much more simpler. Let me know once you finished and I'll take another stab. |
I also updated a comment for the syntax highlighting to reflect some new stuff. |
@fatih its been a few days, what do you think now. |
@nhooyr I also want to use it, but I've just arrived home (Turkey). I didnt' see my family for 6 weeks, and because of my full time job recently it's hard to look ;) |
to open the file. | ||
|
||
If [!] is given then it switches to the new file even if the current file | ||
isn't saved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be updated with the new behavior.
@fatih sorry I just wanted to voice my concerns before I implemented it as you wanted. It's done now. |
function! go#alternate#Switch(bang, cmd) | ||
let l:file = go#alternate#Filename(fnameescape(expand("%"))) | ||
if !filereadable(l:file) && !a:bang | ||
echoerr "couldn't find " . file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fatih Fixed
In #684 a golang alternate plugin was presented in which it was easy to alternate between the implementation and testing code. This commit adds support for that in vim-go. It also adds related documentation and mappings. I also fixed some documentation issues to make g:go_term_mode more clear.
@fatih fixed the error message. |
LGTM. Thanks @nhooyr nice work 👍 |
:GoAlternate and related documentation/mappings
@fatih I already noticed a bug. Well i think it is a bug, |
@nhooyr great catch. And no worries. This is why it's ok to merge to master. We can use it and fix other bugs until we release a new version. |
In #684 a golang alternate plugin was presented in which it was easy to
alternate between the implementation and testing code. This commit adds
support for that in vim-go. It also adds related documentation and mappings.