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

Options altered in Vim window after using popup #2041

Closed
5 of 10 tasks
lacygoill opened this issue May 15, 2020 · 2 comments
Closed
5 of 10 tasks

Options altered in Vim window after using popup #2041

lacygoill opened this issue May 15, 2020 · 2 comments

Comments

@lacygoill
Copy link
Contributor

lacygoill commented May 15, 2020

  • I have read through the manual page (man fzf)
  • I have the latest version of fzf
  • I have searched through the existing issues

Info

  • OS
    • Linux
    • Mac OS X
    • Windows
    • Etc.
  • Shell
    • bash
    • zsh
    • fish

Problem / Steps to reproduce

After using fzf in a Vim popup window, the options 'bufhidden', 'winfixwidth', 'winfixheight' are not properly restored. This can cause unexpected behaviors. For example, the focused window is sometimes wrongly restored when quitting a window.

To Reproduce

Run this shell command:

$ vim -Nu <(cat <<'EOF'
    set rtp^=~/.fzf rtp-=~/.vim rtp-=~/.vim/after hidden
    let g:fzf_layout = {'window': #{width: 0.9, height: 0.6}}
    au VimEnter * wincmd b
EOF
) -o /tmp/file{1..2}

Run this Ex command:

:call fzf#run(fzf#wrap(#{source: ['a'], sink: {-> 0}}))

Press Escape to quit the fzf popup, and run these Ex commands:

:sp
:q

Vim focuses the window 1. This is wrong; it should focus the window 2.
I think that's because the option 'winfixheight' has been wrongly set in the window 2.

Opening the fzf popup also causes 'winfixwidth' to be set, and 'bufhidden' to be set to hide in the current Vim window.

Expected behavior

The options 'winfixwidth', 'winfixheight' and 'bufhidden' are not altered in the current Vim window after using the fzf popup.

Screenshots

gif

Environment

  • Vim version: 8.2 Included patches: 1-752
  • OS: Ubuntu 16.04.6 LTS
  • Terminal: XTerm(322)

Additional context

This is a regression introduced in d631c76.

/cc @ichizok

@lacygoill lacygoill changed the title Options not correctly restored in Vim window after using popup Options altered in Vim window after using popup May 15, 2020
@lacygoill
Copy link
Contributor Author

lacygoill commented May 15, 2020

The issue comes from here:

fzf/plugin/fzf.vim

Lines 838 to 844 in d631c76

let s:popup_create = {buf -> popup_create(buf, #{
\ line: a:opts.row,
\ col: a:opts.col,
\ minwidth: a:opts.width,
\ minheight: a:opts.height,
\ zindex: 50 - is_frame,
\ })}

In the past, popup_create() was invoked immediately, and so these settings were applied in the popup:

setlocal winfixwidth winfixheight

setlocal bufhidden=hide

But now, the popup is created later, once the terminal buffer is created:

autocmd TerminalOpen * ++once call s:popup_create(str2nr(expand('<abuf>')))

So the settings are applied in the wrong window. I'm going to look for a fix. I wonder whether it makes sense to set 'winfixheight' and 'winfixwidth' when using a popup window. Since the latter doesn't alter the layout of the other windows, maybe they could be left alone when the user has configured fzf to use popups...

@lacygoill
Copy link
Contributor Author

lacygoill commented May 15, 2020

If I'm right, and 'winfixwidth', 'winfixheight' are useless in a popup, then this patch could fix the issue:

diff --git a/plugin/fzf.vim b/plugin/fzf.vim
index c288948..1bbb4f5 100644
--- a/plugin/fzf.vim
+++ b/plugin/fzf.vim
@@ -680,7 +680,9 @@ function! s:split(dict)
     endif
     return [ppos, { '&l:wfw': &l:wfw, '&l:wfh': &l:wfh }, is_popup]
   finally
-    setlocal winfixwidth winfixheight
+    if !is_popup
+      setlocal winfixwidth winfixheight
+    endif
   endtry
 endfunction
 
@@ -751,9 +753,6 @@ function! s:execute_term(dict, command, temps) abort
     if has('nvim')
       call termopen(command, fzf)
     else
-      if !len(&bufhidden)
-        setlocal bufhidden=hide
-      endif
       let term_opts = {'exit_cb': function(fzf.on_exit)}
       if is_popup
         let term_opts.hidden = 1

It also gets rid of 'bufhidden'; the latter was set to hide to handle these issues: 1, 2. But I can no longer reproduce them; even when 'hidden' is reset. I think that's because – in the past – the plugin created 2 terminal buffers, one being hidden. That's not the case anymore; now the plugin only creates 1 terminal buffer, so there should be no need to set 'bufhidden' anymore. I'll test the patch for a few days, and if I don't find anything wrong with it, I'll submit a PR.

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

No branches or pull requests

2 participants