search

junegunn / fzf.vim

fzf :heart: vim
MIT License
9.62k stars 584 forks source link

Ctrl+T/Ctrl+X/Ctrl+V actions don't work correctly in some cases when a swap file exists #1514

Open michalsieron opened 10 months ago

michalsieron commented 10 months ago

Problem exists since https://github.com/vim/vim/commit/c3f91c0648f4b04a6a9ceb4ccec45ea767a63796. Bisected and then checked again by reverting it on the current master branch of vim.

Steps to reproduce:

  1. Get yourself a vim v8.2.3833 or newer (I compiled it from source with the default Makefile, so steps assume you are in vim/src).
  2. Open one instance with ./vim -p beval.c beval.h (only needed for swap files to exist).
  3. Use Ctrl+Z to put it in the background, or open another terminal.
  4. Open a new vim instance using minimal configuration. vim -Nu <(curl https://gist.githubusercontent.com/junegunn/6936bf79fedd3a079aeb1dd2f3c81ef5/raw) beval.c
  5. In the newly opened instance, confirm opening beval.c as read only.
  6. Using :Files command open second file selected in step (2) - in my case beval.h.

Observed result: It doesn't matter if you choose O, E or R, the new tab or split won't be created. Although the buffer itself will be opened (visible in :ls).

Expected result: A new tab or split is being created.

I don't know enough about the inner workings of vim or fzf.vim to analyze it deeper. Also, it seems that for this bug to occur, one needs to have one file, which was opened when a swap file existed for it, and then tries to open another file, also with an existing swap file. As in, it doesn't matter if you try to open two files with swap files one after another, or if you open some other files in between.

junegunn commented 9 months ago

Thanks for the report. I can reproduce the problem. I was able to fix the issue by commenting out some try-catches in the code. But unfortunately, I don't fully understand why this happens and why removing them helps. Doing this will likely cause problems in other areas, so we need to find a better solution.

diff --git a/plugin/fzf.vim b/plugin/fzf.vim
index dd10990..917af29 100644
--- a/plugin/fzf.vim
+++ b/plugin/fzf.vim
@@ -314,7 +314,7 @@ function! s:common_sink(action, lines) abort
             \| call s:warn('fzf: E325: swap file exists: '.s:fzf_expand('<afile>'))
     augroup END
   endif
-  try
+  " try
     let empty = empty(s:fzf_expand('%')) && line('$') == 1 && empty(getline(1)) && !&modified
     " Preserve the current working directory in case it's changed during
     " the execution (e.g. `set autochdir` or `autocmd BufEnter * lcd ...`)
@@ -335,10 +335,10 @@ function! s:common_sink(action, lines) abort
         doautocmd BufEnter
       endif
     endfor
-  catch /^Vim:Interrupt$/
-  finally
+  " catch /^Vim:Interrupt$/
+  " finally
     silent! autocmd! fzf_swap
-  endtry
+  " endtry
 endfunction

 function! s:get_color(attr, ...)
@@ -982,7 +982,7 @@ function! s:callback(dict, lines) abort
     let w:fzf_pushd = a:dict.pushd
   endif

-  try
+  " try
     if has_key(a:dict, 'sink')
       for line in a:lines
         if type(a:dict.sink) == 2
@@ -997,11 +997,11 @@ function! s:callback(dict, lines) abort
     elseif has_key(a:dict, 'sinklist')
       call a:dict['sinklist'](a:lines)
     endif
-  catch
-    if stridx(v:exception, ':E325:') < 0
-      echoerr v:exception
-    endif
-  endtry
+  " catch
+  "   if stridx(v:exception, ':E325:') < 0
+  "     echoerr v:exception
+  "   endif
+  " endtry

   " We may have opened a new window or tab
   if popd
michalsieron commented 9 months ago

With the locations you provided, I tried debugging it further.

I ended up on this line https://github.com/junegunn/fzf/blob/91387a741b659ba709a67d55d68b74cb3daa812a/plugin/fzf.vim#L299.

Applying your diff, but surrounding this line with try-catch reproduced the issue. Then I thought, maybe it is not an fzf.vim issue after all?

I used the following as the new .vimrc:

func! TryTabSplit(what)
    try
        execute "tab split" a:what
    catch
    endtry
endfunc

And sure enough, it reproduced!

Then I looked at the file the regression happened in vim and noticed it's autocmd.c. So I removed all autocmd and the problem disappeared. Is the problem related to some autocmd then?

Turns out it is a pure vim problem :open_mouth: And a way to reproduce it is to:

  1. Save the following as a file. Let's use reprorc here. 
    
autocmd BufEnter * echo "Hello there :)"

func! TryTabSplit(what) try execute "tab split" a:what catch endtry endfunc


2. Have two files, let's say `echo a > a.txt; echo b > b.txt`
3. Run `vim -u reprorc -p a.txt b.txt`
4. Ctrl+Z out of vim or open a new terminal. It is important that the swap files exist
5. Open a new vim instance with `vim -u reprorc a.txt`
6. Confirm opening in read-only mode
7. In this new vim run `:call TryTabSplit("b.txt")`
8. Confirm opening in read-only mode
9. No new tab, but the buffer is opened.

For some reason, this also causes the `Hello there :)` message to be displayed twice :shrug:
As tested earlier with reverting commit, commenting out this line https://github.com/vim/vim/blob/82f19734bfcbddbaee8d5d837f7b7a7119366020/src/autocmd.c#L2317 fixes the issue (and the message is being displayed only once).

I tried following debugging this with GDB, but to be honest I don't understand what vim is doing with its `did_emsg`, `did_throw`, `need_rethrow`, `force_abort` etc. :slightly_frowning_face:
junegunn commented 9 months ago

Impressive research, thank you.

maybe it is not an fzf.vim issue after all?

Right. fzf.vim isn't doing something wrong. Vim as a scripting platform has many quirks, some by design, some by bugs. As a Vim plugin developer, you end up accepting the situation and get into the habit of finding ways to work around the problems you don't understand :)

Emilv2 commented 9 months ago

I believe this is the same issue as https://github.com/junegunn/fzf/issues/2895