junegunn / fzf.vim

fzf :heart: vim
MIT License
9.68k stars 585 forks source link

Enable use of terminal support under MacVim #416

Closed bwells closed 7 years ago

bwells commented 7 years ago

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 commented 7 years ago

CC @marzdrel

marzdrel commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

Basic support added in https://github.com/junegunn/fzf/pull/1019/commits/3c6035d01e8be812741d3ecadaaa7a74d602ec3b

janlazo commented 7 years ago

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 commented 7 years ago

Reported CTRL-J issue: https://github.com/vim/vim/issues/2031

junegunn commented 7 years ago

https://github.com/junegunn/fzf/pull/1019#issuecomment-326022757

janlazo commented 7 years ago

fzf 0.17.0-2 uses Vim 8 terminal for MacVim if the requirements are met. See https://github.com/junegunn/fzf/commit/58b5be8ab6a99937a872e2ce4713f9ac79ff7d4e

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 commented 7 years ago

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