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

Calling fzf#run with a list as source fail (n)vim is used from git bash #3777

Closed
6 of 10 tasks
DanSM-5 opened this issue May 7, 2024 · 25 comments
Closed
6 of 10 tasks

Comments

@DanSM-5
Copy link

DanSM-5 commented May 7, 2024

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues
  • For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.51.0-devel (c5fb0c4)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

I noticed some issues running :Lines or :BLines when using nvim under bash from git. The source of the issueis that when the source in the options passed to fzf#run is a list, the source_command will use the type command to get the content of the temporary file. This works fine if called from powershell or cmd as type will do the expected action but for bash (and zsh if you install it) it will fail as type is a shell built-in.

image

  if has_key(dict, 'source')
    let source = remove(dict, 'source')
    let type = type(source)
    if type == 1
      let source_command = source
    elseif type == 3
      let temps.input = s:fzf_tempname()
      call s:writefile(source, temps.input)
+     let source_command = (s:is_win ? 'type ' : 'cat ').fzf#shellescape(temps.input)
    else
      throw 'Invalid source type'
    endif
  else

I've manually changed the command from type to cat and that fixes the issue but it is hard to identify when bash/zsh is the current interactive shell. I'd suggest to provide some global variable like g:fzf_cat that if present, it will be used instead of choosing between type and cat.

If the detection route is preferred, checking for the environment variable $SHELL should work as it won't be set neither from cmd nor powershell.

@junegunn
Copy link
Owner

junegunn commented May 7, 2024

I wonder if this patch fixes your issue:

diff --git a/plugin/fzf.vim b/plugin/fzf.vim
index b73f2aa..02dd60c 100644
--- a/plugin/fzf.vim
+++ b/plugin/fzf.vim
@@ -508,19 +508,19 @@ try
   endif
 
   if has_key(dict, 'source')
-    let source = remove(dict, 'source')
+    let source = dict.source
     let type = type(source)
     if type == 1
-      let source_command = source
+      let prefix = '('.source.')|'
     elseif type == 3
       let temps.input = s:fzf_tempname()
       call s:writefile(source, temps.input)
-      let source_command = (s:is_win ? 'type ' : 'cat ').fzf#shellescape(temps.input)
+      let prefix = (s:is_win ? 'type ' : 'cat ').fzf#shellescape(temps.input).'|'
     else
       throw 'Invalid source type'
     endif
   else
-    let source_command = ''
+    let prefix = ''
   endif
 
   let prefer_tmux = get(g:, 'fzf_prefer_tmux', 0) || has_key(dict, 'tmux')
@@ -544,11 +544,7 @@ try
   endif
   " Respect --border option given in $FZF_DEFAULT_OPTS and 'options'
   let optstr = join([s:border_opt(get(dict, 'window', 0)), s:extract_option($FZF_DEFAULT_OPTS, 'border'), optstr])
-  let prev_default_command = $FZF_DEFAULT_COMMAND
-  if len(source_command)
-    let $FZF_DEFAULT_COMMAND = source_command
-  endif
-  let command = (use_tmux ? s:fzf_tmux(dict) : fzf_exec).' '.optstr.' > '.temps.result
+  let command = prefix.(use_tmux ? s:fzf_tmux(dict) : fzf_exec).' '.optstr.' > '.temps.result
 
   if use_term
     return s:execute_term(dict, command, temps)
@@ -559,14 +555,6 @@ try
   call s:callback(dict, lines)
   return lines
 finally
-  if exists('source_command') && len(source_command)
-    if len(prev_default_command)
-      let $FZF_DEFAULT_COMMAND = prev_default_command
-    else
-      let $FZF_DEFAULT_COMMAND = ''
-      silent! execute 'unlet $FZF_DEFAULT_COMMAND'
-    endif
-  endif
   let [&shell, &shellslash, &shellcmdflag, &shellxquote] = [shell, shellslash, shellcmdflag, shellxquote]
 endtry
 endfunction
@@ -596,8 +584,8 @@ function! s:fzf_tmux(dict)
       endif
     endfor
   endif
-  return printf('LINES=%d COLUMNS=%d %s %s - --',
-    \ &lines, &columns, fzf#shellescape(s:fzf_tmux), size)
+  return printf('LINES=%d COLUMNS=%d %s %s %s --',
+    \ &lines, &columns, fzf#shellescape(s:fzf_tmux), size, (has_key(a:dict, 'source') ? '' : '-'))
 endfunction
 
 function! s:splittable(dict)
@@ -727,7 +715,8 @@ function! s:execute(dict, command, use_height, temps) abort
     let a:temps.shellscript = shellscript
   endif
   if a:use_height
-    call system(printf('tput cup %d > /dev/tty; tput cnorm > /dev/tty; %s < /dev/tty 2> /dev/tty', &lines, command))
+    let stdin = has_key(a:dict, 'source') ? '' : '< /dev/tty'
+    call system(printf('tput cup %d > /dev/tty; tput cnorm > /dev/tty; %s %s 2> /dev/tty', &lines, command, stdin))
   else
     execute 'silent !'.command
   endif

It's basically reverting what I did in 7411da8.

fzf#run forces using cmd.exe via s:use_sh

fzf/plugin/fzf.vim

Lines 447 to 458 in 4bedd33

function! s:use_sh()
let [shell, shellslash, shellcmdflag, shellxquote] = [&shell, &shellslash, &shellcmdflag, &shellxquote]
if s:is_win
set shell=cmd.exe
set noshellslash
let &shellcmdflag = has('nvim') ? '/s /c' : '/c'
let &shellxquote = has('nvim') ? '"' : '('
else
set shell=sh
endif
return [shell, shellslash, shellcmdflag, shellxquote]
endfunction

so type should be available, but instead of directly executing the command, we're setting $FZF_DEFAULT_COMMAND so that fzf launches the process instead. And the behavior depends on $SHELL.

Another option would be to temporarily switch $SHELL as well in s:use_sh. But this is probably not a good idea.

diff --git a/plugin/fzf.vim b/plugin/fzf.vim
index b73f2aa..aa0ac02 100644
--- a/plugin/fzf.vim
+++ b/plugin/fzf.vim
@@ -447,6 +447,7 @@ endfunction
 function! s:use_sh()
   let [shell, shellslash, shellcmdflag, shellxquote] = [&shell, &shellslash, &shellcmdflag, &shellxquote]
   if s:is_win
+    let $SHELL = 'cmd.exe'
     set shell=cmd.exe
     set noshellslash
     let &shellcmdflag = has('nvim') ? '/s /c' : '/c'
@@ -567,7 +568,7 @@ finally
       silent! execute 'unlet $FZF_DEFAULT_COMMAND'
     endif
   endif
-  let [&shell, &shellslash, &shellcmdflag, &shellxquote] = [shell, shellslash, shellcmdflag, shellxquote]
+  let [$SHELL, &shell, &shellslash, &shellcmdflag, &shellxquote] = [shell, shell, shellslash, shellcmdflag, shellxquote]
 endtry
 endfunction
 

@junegunn
Copy link
Owner

junegunn commented May 7, 2024

I decided to apply the first patch, as I don't really think temporarily swapping $FZF_DEFAULT_COMMAND is a good idea, and a typical source command will terminate itself when its output stream is broken. Please update and see if it helps.

@DanSM-5
Copy link
Author

DanSM-5 commented May 7, 2024

Yes, it does fixes the issue. Thanks!

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

Dear @DanSM-5 , would you mind specifying your Windows version? Using latest fzf 0.52.1 and Git Bash, I can run :FZF from Vim in Git Bash in Windows 11, but not under Windows 10. Under Windows 10, only an empty cmd prompt shows up that can be exited to return to Vim. Fzf works fine in Git Bash itself. I am out of ideas why this does not work on Windows 10.

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

Okay, the solution is given in the open issue #1983
Not sure why the fix is not applied?
I also wonder why Windows 11 does not suffer from this

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

I have windows 11. I think I've encounter the issue described in #1983 and I called cmd as cmd.exe //c from git bash.

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

@Konfekt what do you get if you echo $MSYSTEM? I kind of remember that it may be an issue if get MSYS and not MINGW as the value.

Could you try starting git bash like this from powershell and test if :FZF works for you?

& "$env:PROGRAMFILES/Git/usr/bin/env.exe" MSYS=enable_pcon MSYSTEM=MINGW64 enable_pcon=1 /usr/bin/bash -il

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

Thank you for your assistance. The same issue persisted.

What works wonders is either adding the suggested environment variable or rather doubling the initial slashes

let command = 'cmd.exe ///C '.fzf#shellescape('set "TERM=" & start /WAIT sh -c '.shellscript)

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

Curiously, this breaks :FZF under Windows 11. So a case distinction is in order?

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

Do you have same git version? Recent versions of git change the MSYS runtime which could cause those differences you experience.
Is git bash in your path in both PCs?

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

What works under both versions, 10 and 11, is MSYS_NO_PATHCONV=1 as proposed in #1983

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

I tried with the same latest git, vim and fzf version by scoop.
How did you get it working under Windows 10?

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

I find it rather likely that issue #1983, probably using Windows 10 as it is from April 2020, is still open

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

Recent versions of git change the MSYS runtime

Would you mind explaining how this affects this issue?

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

How did you get it working under Windows 10?

I don't have windows 10. I mentioned above I use windows 11.

Would you mind explaining how this affects this issue?

Sure, at some point I started experiencing dandavison/delta#1318. The root cause came from some changes in MSYS which git bash uses. dscho/msys2-runtime@10e99fe

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

I don't have windows 10. I mentioned above I use windows 11

As it works fine under Windows 11 with the exact same setup, but not under Windows 10,
I find it unlikely that MSYS is the cause, but rather (Conhost ?!) differences between the Windows versions.

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

I find it unlikely that MSYS is the cause

I asked about the git version to rule out this. Conhost is the terminal but I don't think it influences the environment. You could try the git bash terminal (I think it is MinTTY) or Windows Terminal to see if anything changes.

@Konfekt
Copy link
Contributor

Konfekt commented May 14, 2024

You could try the git bash terminal (I think it is MinTTY) or Windows Terminal to see if anything changes.

Yes, I tried other terminals, notably the latest Git Bash terminal. Nothing changes. That's why I'd have appreciated your testing on Windows 10 to confirm.

@DanSM-5
Copy link
Author

DanSM-5 commented May 14, 2024

@Konfekt I figured out your issue. I got the clue from #1983 as the addition for MSYS_NO_PATHCONV checks for the following has('win32unix'). It is only true in the version of vim that comes with git bash (and probably with cygwing as per the help description). I found that I have neovim and vim from scoop higher in my path which return false to that feature check but they return true to has('win32').

If I load fzf and fzf.vim in the vim from git bash it does what you describe, it goes directly into cmd.exe.
Could you try downloading neovim (to avoid confusion with vim) and try there? If has('win32unix') is false, it should work fine.

@Konfekt
Copy link
Contributor

Konfekt commented May 15, 2024

It is only true in the version of vim that comes with git bash (and probably with cygwing as per the help description)

Yes, the fix only applies for Vim in Git Bash which sets has('win32unix').
Since this issue's title contained

vim is used from git bash

I thought it to be about Vim from git bash.

There's no issue for any other Vim executable, say that installed by Scoop;
it worked fine in any other terminal, but not for Git Bash, which I however use by preference.

For some reason, the issue for Vim in Git Bash is gone in Windows 11 on my computer, but yet there in Windows 10, as one expects if bash converts any arguments containing / such as in cmd /c to a path.
Interesting that you can reproduce it for Vim in Git Bash in Windows 11. (I can't.) Are you using the latest versions?

In view of this, replacing /C by ///C or adding MSYS_NO_PATHCONV=1 might fix be considered a universal fix. Could you try it in

let command = 'cmd.exe /C '.fzf#shellescape('set "TERM=" & start /WAIT sh -c '.shellscript)
?

Konfekt added a commit to Konfekt/fzf that referenced this issue May 15, 2024
Despite its title 'Calling fzf#run with a list as source fail (n)vim is used from git bash' the issue in 

junegunn#3777

of running `:FZF` in Vim in Git Bash was apparently only fixed for Neovim in Git Bash on Windows 11, but not for Vim from Git Bash.

In view of this, replacing /C by ///C might be considered a universal fix.

This PR just proposes the patch in junegunn#1983 that still seems open.

In view of the fourth item in the most recent 2.45.0 https://github.com/git-for-windows/build-extra/blob/main/ReleaseNotes.md#known-issues little seems to have changed regarding path conversion of arguments containing forward slashes
@Konfekt
Copy link
Contributor

Konfekt commented May 15, 2024

Since MSYS_NO_PATHCONV=1 also refrains from converting paths passed in s:command, but only /C should not be converted, doubling / in /C is the finer solution. Could you check that it works for you? Seemingly, depending on the environment, slashes had to be quadrupled.

junegunn pushed a commit that referenced this issue May 15, 2024
* make :FZF work in Vim from Git Bash

Despite its title 'Calling fzf#run with a list as source fail (n)vim is used from git bash' the issue in 

#3777

of running `:FZF` in Vim in Git Bash was apparently only fixed for Neovim in Git Bash on Windows 11, but not for Vim from Git Bash.

In view of this, replacing /C by ///C might be considered a universal fix.

This PR just proposes the patch in #1983 that still seems open.

In view of the fourth item in the most recent 2.45.0 https://github.com/git-for-windows/build-extra/blob/main/ReleaseNotes.md#known-issues little seems to have changed regarding path conversion of arguments containing forward slashes

* prefer doubling slashed instead of generic env. var

If MSYS_NO_PATHCONV=1 is used, then all arguments are preserved, in particular possibly paths passed in s:command.
Therefore, only avoid converting `/C` from `cmd` to a path.
@DanSM-5
Copy link
Author

DanSM-5 commented May 15, 2024

In my case I can see the same issue happening in windows 11 (make sure in your window 11 pc you are calling the same vim). I imagine it happens on the cygwin version of vim specifically (as the description of the feature had('win32unix') implies).

I have other vim excutables due to other issues happening in the provided vim with git bash, thus I ensured to have it lower in my path.

Could you try it in

I'll try that later.

@Konfekt
Copy link
Contributor

Konfekt commented May 15, 2024

Thank you for your testing. I am testing in a Windows 10 VM, where, as for junegunn, doubling the forward slash works wonders. However, on the Windows 11 machine no slash was needed, whereas the Windows 10 machine demanded three of them. Since this has been merged, it would be good to have some proven record.

@DanSM-5
Copy link
Author

DanSM-5 commented May 16, 2024

When I open git bash's vim executable and I run something like :FZF or :LINES (from fzf.vim), it opens a new window terminal with the fzf. You can choose something and the selection will open back to the initial terminal window.

I believe that's the expected behavior. If so, it should be working fine.

@Konfekt
Copy link
Contributor

Konfekt commented May 16, 2024

Good news, yes, that's also how it's been behaving for me. It's also working fine on Windows 10 and 11 here. Let it remain a mystery why it worked on Windows 11 in the first place.

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

3 participants