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

Running GoTest with g:go_term_enabled and splitright puts editor into insert mode #1412

Closed
mightyguava opened this issue Aug 16, 2017 · 12 comments
Assignees

Comments

@mightyguava
Copy link

Behavior

When running :GoTest or other terminal-enabled commands, I expect a split pane to open and enter "insert" mode, so that I can press q to exit the terminal.

Instead, I observe that the split pane opens, but the editor pane that my code is in enters insert mode instead, and I have to exit insert mode, go to the terminal pane, and then press :q to close it.

Steps to reproduce:

Run :GoTest in a Go file.

Configuration

vimrc

call plug#begin('~/.vim/plugged')
Plug 'fatih/vim-go'
call plug#end()

set splitright
let g:go_term_enabled = 1
  • vim version: NVIM v0.2.0
  • vim-go version: 525fe8af97433c4a661de9548be5ed1c15451a0c
  • go version: 1.8.3 darwin/amd64
@mocheryl
Copy link
Contributor

I'm having the same issue. This appeared due to changes in commit 4fda596 and consequentially 7915244. By reverting them, I get back the expected behaviour.

Also by reverting I could still not replicate the problem the first commit solved. The current working directory always stays the same. Using neovim v0.2.2

@fatih
Copy link
Owner

fatih commented Dec 14, 2017

I had something similiar back then, but it would put me inside insert mode. The following (didn't tested) should get you out of insert mode:

autocmd BufWinEnter,WinEnter term://* stopinsert

I think you could easily do anything else instead of calling stopinsert, such as calling feedkeys() or calling :q, etc.. Can you please try it and let us know ?

@mocheryl
Copy link
Contributor

Maybe I wasn't clear enough in my previous comment, because there is a bit of a misunderstanding here.

The issue is not in not exiting insert mode after running a Go command (in this case :GoTest). That has been resolved some time after the initial comment of this issue (second commit mentioned in my previous comment). It's the whole user experience when dealing with the terminal. When opened, it used to automatically switch control to it and enter terminal (insert) mode so that you can issue commands to it (in this case hitting q to exit, close and restore control back to the original window) just as it was originally intended.

Now this functionality has been, in my opinion, a bit crippled due to the first commit I mentioned in my comment. Now you always have to manually switch to these newly-opened terminal windows and that is just not very practical especially when you're issuing a lot of these Go commands. This, I believe, is also part of the complaint of the original comment. All these changes have been done to solve issue #1289, but the problem mentioned there doesn't exist in my case (maybe newer version of neovim has something to do with it), making this commit/fix moot.

Or maybe we just need to rethink how opening terminals should be handled (some commands should switch control, some not)?

@arp242
Copy link
Contributor

arp242 commented Dec 27, 2017

Note: #1611 touches this code, so this may fix this (... or it may not).

@bhcleek
Copy link
Collaborator

bhcleek commented Dec 27, 2017

Reverting 7915244 will fix this, but I'm not sure why it was done to begin with, so I'm reluctant to revert it without a better understanding of the implications.

@mocheryl
Copy link
Contributor

Note: #1611 touches this code, so this may fix this (... or it may not).

Thanks for the suggestion, but unfortunately it doesn't. I actually checked that out before writing my previous comment and found out that it doesn't change current behavior; only fixes/improves it.

Reverting 7915244 will fix this, but I'm not sure why it was done to begin with, so I'm reluctant to revert it without a better understanding of the implications.

This and more has already been explained in my first two comments of this issue, so please refer to those. To add, I guess we should get more neovim users to test the consequences of reverting these two changes, because I still can't reproduce the referenced issue in my environment.

@NicholasAsimov
Copy link

I'm a neovim user as well and was experiencing this problem for a long time as well but didn't take the time to debug it. Let me try to explain it as best as I can.

Current/desired behavior

What I would like to see (and my guess is that this is desired behavior):

  1. Use :GoRun command
  2. New terminal window opens using go_term_mode (so if it's vsplit and I have set splitright then new window on the right of the screen should appear)
  3. If the program exits with code 0 than window should remain active and in insert mode, so I can observe the output and press any key to close it.

This is a GIF showcasing the desired behavior:
desired2

What happens instead:

The main editing window becomes active.
In order to close the :GoRun window you have to switch to it and close it, which is not very productive.

This is a GIF showcasing the current behavior:
current_slow

What causes this

Basically as @mocheryl said it is caused mainly by the 4fda596. This commit switches the window back to the editor. 7915244 just makes it even less convenient because in normal mode you have to :q the window but in insert mode you can just press any key to exit the terminal window (at least in neovim).

How to fix

After #1611 you cannot revert 7915244 without conflicts so I'm just showing a patch that allows me to achieve the desired behavior in neovim. In essence it's same as reverting those 2 commits and solving the conflict.

diff --git autoload/go/term.vim autoload/go/term.vim
index 23ee18c..912b5d6 100644
--- autoload/go/term.vim
+++ autoload/go/term.vim
@@ -19,7 +19,6 @@ function! go#term#newmode(bang, cmd, mode) abort
   endif
 
   " execute go build in the files directory
-  let l:winnr = winnr()
   let cd = exists('*haslocaldir') && haslocaldir() ? 'lcd ' : 'cd '
   let dir = getcwd()
 
@@ -67,11 +66,6 @@ function! go#term#newmode(bang, cmd, mode) abort
   call jobresize(id, width, height)
 
   let s:jobs[id] = job
-  stopinsert
-
-  if l:winnr !=# winnr()
-    exe l:winnr . "wincmd w"
-  endif
 
   return id
 endfunction

Consequences

I don't see any for 4fda596. As @mocheryl said 4fda596 was meant to fix the #1289 issue but I cannot reproduce it even after reverting the fix. My guess is that something else fixed that down the road. Its hard for me to understand how is that change supposed to fix the cwd though, because the cd command is executed in termopen() context that should not lead to cwd change in main window anyway.

However I'm not sure why 7915244 was introduced so I cannot say for certain.

I hope this helped to understand the issue and I hope to see this fixed as it really hinders the productivity of a quick :GoRun after a code change.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 19, 2018

Thank you for the clear description of desired behaviors @NicholasAsimov. I expect to revisit this once #1721 is merged, because simply removing those lines won't be sufficient for a great user experience; we need to handle some non-zero exit statuses slightly differently, and introducing that logic now would conflict with #1721. As soon as it's merged, I'll make changes that include the patch you suggested.

I've also confirmed that removing those lines does not reintroduce #1289.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 20, 2018

When running :GoTest or other terminal-enabled commands, I expect a split pane to open and enter "insert" mode, so that I can press q to exit the terminal.

Instead, I observe that the split pane opens, but the editor pane that my code is in enters insert mode instead, and I have to exit insert mode, go to the terminal pane, and then press :q to close it.

@mightyguava I am no longer able to duplicate the original window getting put into insert mode. It's possible that in the past it was due to a race condition that has since been resolved.

To put the terminal mode into insert mode, you can add autocmd BufWinEnter,WinEnter term://* startinsert to your vimrc so that you can press any key to close the terminal window.

@NicholasAsimov I need a bit more time to think through some of the implications of implementing what you've suggested. The behavior you've described may make sense for fast running processes, but for longer running processes, it's preferrable to open the terminal window and put the user back in the original window to continue working. Additionally, leaving the terminal in insert mode will automatically close it as soon as any key is pressed; that would be very disruptive to a user that wants to see the scrollback in the terminal and doesn't realize the window is insert mode.

As I mentioned above, you can add an autocmd to your vimrc to put the terminal window into insert mode when you enter it.

@fatih
Copy link
Owner

fatih commented Mar 20, 2018

I agree with @bhcleek . There is a reason this was implemented in the first place like this. You should be able to code on your editor, the content of :GoRun is there to see the output, not to interact with it. The interaction should be always in the main buffer.

Additionally, leaving the terminal in insert mode will automatically close it as soon as any key is pressed;

This was very annoying when I start using it, hence we switch back to the buffer. Luckily, there are many tweaks you can use to make it useful for you. For example I have the followings:

" Terminal settings
if has('terminal')
  " Kill job and close terminal window
  tnoremap <Leader>q <C-w><C-C><C-w>c<cr>

  " switch to normal mode with esc
  tnoremap <Esc> <C-W>N

  " mappings to move out from terminal to other views
  tnoremap <C-h> <C-w>h
  tnoremap <C-j> <C-w>j
  tnoremap <C-k> <C-w>k
  tnoremap <C-l> <C-w>l
 
  " Open terminal in vertical, horizontal and new tab
  nnoremap <leader>tv :vsplit<cr>:term ++curwin<CR>
  nnoremap <leader>ts :split<cr>:term ++curwin<CR>
  nnoremap <leader>tt :tabnew<cr>:term ++curwin<CR>

  tnoremap <leader>tv <C-w>:vsplit<cr>:term ++curwin<CR>
  tnoremap <leader>ts <C-w>:split<cr>:term ++curwin<CR>
  tnoremap <leader>tt <C-w>:tabnew<cr>:term ++curwin<CR>

  " always start terminal in insert mode when I switch to it
  " NOTE(arslan): startinsert doesn't work in Terminal-normal mode.
  autocmd WinEnter * if &buftype == 'terminal' | call feedkeys("i") | endif
endif

I know that not most of them fixes every workflow. But we're trying our best. What we can do is though is to provide some more customizable settings for the terminal usage. Not many know Vim script in and out and prefer simple switches.

@bhcleek
Copy link
Collaborator

bhcleek commented Mar 20, 2018

The original reported issue is fixed. #1720, #1721, #1722, and #1725 addressed some of the other issues brought up here and it seems that there's a reasonable solution for those that want to put their terminal windows into insert mode.

Please feel free to open a new issue if you find any related bugs on master.

@bhcleek bhcleek closed this as completed Mar 20, 2018
@NicholasAsimov
Copy link

If somebody has the same use-case as me and wants a quick go run window for a fast-running process you can use this command: :w<CR>:vsplit <bar> terminal go run %<CR> and map as a keybind, for example: autocmd FileType go nmap <leader>r :w<CR>:vsplit <bar> terminal go run %<CR>.

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

No branches or pull requests

6 participants