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

Enable use of terminal support under MacVim #416

Closed
bwells opened this issue Aug 8, 2017 · 15 comments
Closed

Enable use of terminal support under MacVim #416

bwells opened this issue Aug 8, 2017 · 15 comments

Comments

@bwells
Copy link

@bwells bwells commented Aug 8, 2017

  • Category
    • Question
    • Bug
    • Suggestion
  • OS
    • Linux
    • macOS
    • Windows
    • Etc.
  • Vim
    • Vim
    • Neovim

MacVim (as of snapshot 136) has added functional support for :terminal in line with upstream vim (8.0.891 for me). FZF.vim is working as desired under a vanilla vim with :terminal support, but under MacVim it still tries to launch xterm by default.

Suggestion: Enable use of vim's built in terminal support under macvim as the default option when available.

@kutsan

This comment has been minimized.

Copy link

@kutsan kutsan commented Aug 18, 2017

@marzdrel

This comment has been minimized.

Copy link

@marzdrel marzdrel commented Aug 18, 2017

I am completely new to vimscript but I did look around to see how hard it might be to get it running. Fzf detects if it's running in Neovim and then just uses build-in function termopen() to spawn inside the build-in terminal. Looks like MacVim so far offers no such function but a command :terminal. I added another path for has('gui_macvim') and used :terminal with parameters to spawn Fzf.

This worked fine so far, but then it turned out that Fzf just doesn't work at all in MacVim's terminal. There is a problem with drawing inside the terminal. App is running but the visuals are all wrong. Again, I have little experience with Go but this seems to be related to the fact Fzf uses tcell library. I tried some ncurses based apps and those work fine. I also tried other apps using tcell and those fail just like Fzf does.

So far this is a blocker for me. I have no idea how hard it will be to read the Fzf output back in Vim or to cut through rough edges to streamline the user experience (closing extra buffers and such). So far I am not even sure if there is even a way to spawn a terminal under (instead of above) current buffer. I am not sure how's the implementation, but the documentation for terminal functionality in MacVim is minimal.

Last but not least, daily snapshots of MacVim are very unstable. Simple actions crashes entire app (window disappears).

@prabirshrestha

This comment has been minimized.

Copy link

@prabirshrestha prabirshrestha commented Aug 26, 2017

Vim8 does support the equivalent of neovim's termopen() using term_start(). If you want to open terminal in the specified buffer you can pass curwin as options.

This is what vim-fz currently does:

  let ctx['buf'] = bufnr('%')
  if s:is_nvim
    call termopen(cmd, {'on_exit': function('s:exit_cb', [ctx])}) | startinsert
  else
    call term_start(cmd, {'term_name': 'Fz', 'curwin': ctx['buf'], 'exit_cb': function('s:exit_cb', [ctx])})
  endif

It would be also good to use inbuilt terminal on windows's gvim.

@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Aug 26, 2017

I have no objections on using builtin terminal if on GVim. Looks like it has improved a lot since the last time I checked, and I can finally run fzf inside it. One issue I immediately noticed is that it does not differentiate between CTRL-J and CTRL-M, mapped to "down" and "accept" respectively in fzf, so I have to remap CTRL-J just to be able to select an item: fzf --bind ctrl-j:accept.

@prabirshrestha

This comment has been minimized.

Copy link

@prabirshrestha prabirshrestha commented Aug 26, 2017

Might be file a bug in vim-dev? There has been lot of patches related to terminal. As more people use it more bugs are getting fixed.

Given that it would be good for fzf support older vim 8 too, might be have a flag to opt in for terminal now let g:fzf_prefer_vim_terminal = 1. As more people test it we could enable it by default in the future.

@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Aug 26, 2017

Might be file a bug in vim-dev?

Can any one of you do that? I'm not familiar with it.

might be have a flag to opt in for terminal now let g:fzf_prefer_vim_terminal = 1

For GVim, right? I don't see any benefit of using :terminal in terminal Vim.

@prabirshrestha

This comment has been minimized.

Copy link

@prabirshrestha prabirshrestha commented Aug 26, 2017

Now you can use github to file issues (https://github.com/vim/vim/issues/new) and it will automatically get synced to vim-dev (https://groups.google.com/forum/#!forum/vim_dev) which is google groups.

I would like to have it working in both gvim and terminal vim if possible but we could tackle terminal vim later. The same terminal api works in both gvim8 and terminal vim8 so I don't think there will much extra work.

This is currently what fzf looks inside terminal vim in windows. It takes the full screen.

image

This is how vim-fz looks inside terminal vim in windows. This allows me to see existing opened buffers.
image

@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Aug 26, 2017

Oh right, I don't use Windows so it didn't occur to me. On Linux and macOS, fzf can use --height option not to occupy full screen, and can display 24-bit colors in the terminal.

fzf-macos

And the code is simpler as we can just call system() synchronously.

So to recap, using builtin terminal makes sense for GVim (on all platforms) and terminal Vim on Windows.

/cc @janlazo

@janlazo

This comment has been minimized.

Copy link
Contributor

@janlazo janlazo commented Aug 27, 2017

Last time I tried :terminal on GVim, I had to manually close the terminal buffer after running exit on cmd.exe. It's not ready in terminal vim on ConEmu with the following hack for 256 colors and backspace key because it gets stuck and Ctrl + C didn't work:

  " Reference - https://conemu.github.io/en/VimXterm.html
  if $ConEmuANSI ==# 'ON' && !has('gui_running')
    if has('builtin_terms') && $ConEmuTask !~# 'Shells::cmd'
      set term=xterm
      set t_Co=256
      let &t_AB = "\e[48;5;%dm"
      let &t_AF = "\e[38;5;%dm"
    endif

    inoremap <Char-0x07F> <BS>
    nnoremap <Char-0x07F> <BS>
    vnoremap <Char-0x07F> <BS>
  endif

I can work on :terminal for GVim but I prefer to wait for bug fixes for terminal Vim because I never use terminal Vim without ConEmu and powershell. It looks like a lot of work to reconcile the incompatible APIs even if this is only for the GUIs.

@janlazo

This comment has been minimized.

Copy link
Contributor

@janlazo janlazo commented Aug 27, 2017

Basic support added in junegunn/fzf@3c6035d

@janlazo

This comment has been minimized.

Copy link
Contributor

@janlazo janlazo commented Aug 27, 2017

Anyone know why this line in fz.vim to explicitly close the terminal job channel is required in the exit callback for Vim 8?

if !s:is_nvim
    silent! call ch_close(job_getchannel(term_getjob(a:ctx['buf'])))
endif
@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Aug 29, 2017

Reported CTRL-J issue: vim/vim#2031

@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Aug 30, 2017

@janlazo

This comment has been minimized.

Copy link
Contributor

@janlazo janlazo commented Sep 7, 2017

fzf 0.17.0-2 uses Vim 8 terminal for MacVim if the requirements are met.
See junegunn/fzf@58b5be8

It's enabled in terminal Vim on Windows but it may have display issues on the terminal if not using powershell or a GUI.

@junegunn

This comment has been minimized.

Copy link
Owner

@junegunn junegunn commented Sep 8, 2017

Yes, I think we can close this now. Note that you have to build Macvim from source at the moment:

brew update
brew reinstall macvim --HEAD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.