fzf On fzf.vim opening in new cmd Window in Git Bash

Checklist

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

Output of `fzf --version`

0.52.1 (6432f00)

OS

[ ] Linux
[ ] macOS
[X] Windows
[ ] Etc.

Shell

[X] bash
[ ] zsh
[ ] fish

Problem / Steps to reproduce

This PR made by @Konfekt allows fzf.vim to work in Git Bash. However, I found that by commenting the following part of the if-branch allows fzf.vim with the stock vim in Git Bash to run normally. https://github.com/junegunn/fzf/blob/daa602422dc272cbec6b99bc7520262672ad7230/plugin/fzf.vim#L711-L716

In Git Bash's vim, :echo has('unix') returns 1, :echo has('win32unix') returns 1, :echo has('win32') returns 0, and :echo has('win64') returns 0.

What is has('win32unix') used for? It seems like it can be removed in a few cases.

May 21 '24 19:05 ykhan21

This checks for the Win32 version of Vim, using Unix files (Cygwin), according to :help feature-list, but Cygwin nowadays rather refers to MSYS2. One striking difference is that it expects Unix paths.

As vim in Git Bash is the default Vim, we looked no further. By has('win32unix') being true, we know not only that this is most likely Git Bash vim, but also that the Git Bash executable exists, used by line 714.

So yes, one could likely check for something like has('win32unix') || (has('win32') && &shell =~# '\v<(ba)?sh>') instead of has('win32unix')

Likely only its original author @janlazo can tell us why $TERM !=# 'cygwin' must be ensured.

May 21 '24 20:05 Konfekt

Why does it need to be run as a cmd.exe process?

Removing this branch lets Git Bash's vim use the unix branches and most importantly does not cause a separate cmd.exe Window to appear.

May 21 '24 20:05 ykhan21

Removing this branch lets Git Bash's vim use the unix branches and most importantly does not cause a separate cmd.exe Window to appear.

Is it? What does branch refer to? If you mean spawning cmd.exe, that would be great. Again, this code was added in https://github.com/junegunn/fzf/pull/933 whose authors know better. The Wiki says

On Cygwin, install script will download the prebuilt fzf binary for Windows platform. It does not run on mintty, the default terminal emulator shipped with Cygwin, but it works fine on ConEmu or the default Command Prompt (cmd.exe).

The Vim plugin of fzf also works with the Windows binary. It will start an extra cmd.exe window on mintty to circumvent the aforementioned limitation.

but seems somewhat outdated, since now the dev branch already supports Fzf in Git Bash default Mintty term.

Mind that https://github.com/junegunn/fzf/issues/3804 asks similarly if the spawning of cmd.exe could be removed. Maybe with the latest changes this is indeed possible. For starters, does

let command = 'start /WAIT sh -c '.shellscript

suffice?

May 21 '24 20:05 Konfekt

If I understand correctly, in latest Git Bash simply

execute 'silent !'.command

instead of the involved cmd.exe shell script execution suffices. Did you check :FZF to work as expected for all kind of paths, namely with spaces?

May 21 '24 20:05 Konfekt

Yes, see the image below:

However, it executes it in full screen. Is this is normal behavior on a unix system or is this an artifact of some other has('win32unix') check?

May 21 '24 21:05 ykhan21

For me :FZF $path without the above shown if-branch worked out-of-the-box in Windows Terminal, but not in Git Bash's default Mintty terminal. However, it did with the exe from the dev branch using winpty. Therefore I suspect that this if-branch was needed to make fzf work in Git Bash without winpty (which maybe wasn't included in Git bash from the start?) and we can drop it together with the next release.

If you could confirm that all commands work as expected under the Git Bash Terminal as well?

May 22 '24 04:05 Konfekt

@junegunn

Therefore I suspect that this if-branch was needed to make fzf work without winpty in Git Bash (which maybe wasn't included in Git bash from the start?) and we can drop it together with the next release.

Maybe that could be part of the next release since it yet improves again on the Git Bash experience

May 22 '24 04:05 Konfekt

Since 573df52 (#3807) seems to call winpty directly, the check for win32unix could be replaced by executable('winpty').

This would also make it clearer, if guessed correctly, why all the convolution https://github.com/junegunn/fzf/blob/daa602422dc272cbec6b99bc7520262672ad7230/plugin/fzf.vim#L711-L716 had to be added in the first place, when likely neither winpty was an option nor Windows Terminal was widely used.

May 22 '24 06:05 Konfekt

@Konfekt Can you open a pull request against devel branch?

May 22 '24 06:05 junegunn

Interestingly, I can use fzf in Git Bash without winpty. I don't know why that is. 0.52.1 (6432f00)

fzf also works for me with no ~/.bashrc.

May 22 '24 13:05 ykhan21

Phew, to confirm: that might be because this terminal is from MSYS2 itself, not the somewhat more restricted Git Bash MSYS2? For most users, Vim in MSYS2 will be Vim in Git Bash, so that's a case to be accounted for.

May 22 '24 14:05 Konfekt

This is the Git Bash terminal from Git for Windows. It is from scoop install git.

May 22 '24 14:05 ykhan21

I can confirm that fzf.exe 0.52.1 works in Git Bash Mintty under Windows 11; it does not under Window 10.

May 22 '24 17:05 Konfekt

Maybe it's solved in Windows 11, and winpty only needs to be used after a check for Windows 10 in fzf.exe, such as

    var osvi OSVERSIONINFOEXW
    osvi.dwOSVersionInfoSize = uint32(unsafe.Sizeof(osvi))

    mod := syscall.NewLazyDLL("kernel32.dll")
    proc := mod.NewProc("GetVersionExW")
    ret, _, _ := proc.Call(uintptr(unsafe.Pointer(&osvi)))

    if ret != 0 {
        if osvi.dwMajorVersion == 10 {
        ...

May 22 '24 17:05 Konfekt

No, this was not about the OS versions. It was rather about the difference between git 2.42.0 and 2.45.1. @junegunn Fzf in latest Git Bash Mintty no longer seems to need the winpty workaround. Here's where were at: https://github.com/junegunn/fzf/issues/3809#issuecomment-2126358071

May 23 '24 06:05 Konfekt

@ykhan21 You may check with an older Git Bash, confirming https://github.com/junegunn/fzf/issues/3809#issuecomment-2126358071

May 23 '24 06:05 Konfekt

I am not sure what might have changed regarding winpty in Git Bash, from crawling their repo there's nothing striking and the release notes https://github.com/git-for-windows/build-extra/blob/main/ReleaseNotes.md#known-issues still state that winpty is often needed. A comment two month ago by its maintainer reinstating it

May 23 '24 06:05 Konfekt

Here we go, this might have tickled down from mintty to Git Bash: https://github.com/mintty/mintty/wiki/Tips#inputoutput-interaction-with-alien-programs

When interacting with programs that use a native Windows API for command-line user interaction (“console mode”), a number of undesirable effects used to be observed; this is the pty incompatibility problem and the character encoding incompatibility problem. This would basically affect all programs not compiled in a cygwin or msys environment (and note that MinGW is not msys in this context), and would occur in all pty-based terminals (like xterm, rxvt etc).

Cygwin 3.1.0 compensates for this issue via the ConPTY API of Windows 10. On MSYS2, its usage can be enabled by setting the environment variable MSYS=enable_pcon (or selecting this setting when installing an older version). You can also later set MSYS=enable_pcon in file /etc/git-bash.config. MSYS2 releases since 2022-10-28 enable ConPTY by default. You can also set mintty option ConPTY=true to override the MSYS2 setting.

As a workaround on older versions of Cygwin or Windows, you can use winpty as a wrapper to invoke the Windows program.

Indeed MSYS=enable_pcon makes fzf work even on older Git Bash terms

May 23 '24 06:05 Konfekt

So Fzf could suggest

either installing a recent mintty (> 2022-11 which supposedly is https://github.com/mintty/mintty/releases/tag/3.6.2 but mintty 3.6.4 in Git Bash 2.42.1 does not enable it by default; likely 3.7.0 does) which is included also in most recent Git Bash >= 2.43 according to https://github.com/git-for-windows/build-extra/blob/34a82b52c89920c425b691e8ae61a8eed47152dd/versions/package-versions-2.43.0.txt,
if that's not an option, setting MSYS=enable_pcon in ~/.bash_profile or /etc/git-bash.config for mintty including https://github.com/mintty/mintty/commit/6999ae7f41ba856f1e78b39bbd480797b042924b from 2021-02, that is https://github.com/mintty/mintty/releases/tag/3.4.5 which states

New settings -P/--pcon/ConPTY to enable/disable ConPTY support (https://github.com/mintty/wsltty/issues/271).

This is included in Git Bash >= 2.31.0 according to https://github.com/git-for-windows/build-extra/blob/34a82b52c89920c425b691e8ae61a8eed47152dd/versions/package-versions-2.31.0.txt
for mintty < 3.4.5, that is Git Bash < 2.31.0 (2021-02) the winpty workaround is still needed
if there's no winpty, then in Vim this still can be worked around by spawning cmd.exe as has been fzf's default approach till today

Once settled how fzf handles this, the Wiki entry https://github.com/junegunn/fzf/wiki/Cygwin can be updated

May 23 '24 06:05 Konfekt

Good thing: Mintty provides the environment variable $TERM_PROGRAM_VERSION to check for these. So how about something similar to

func main() {
  termProgramVersion := os.Getenv("TERM_PROGRAM_VERSION")
  if compareVersions(termProgramVersion, "3.7.0") >= 0 {
    return
  } else if compareVersions(termProgramVersion, "3.4.5") >= 0 {
    originalMSYS := os.Getenv("MSYS")
    os.Setenv("MSYS", "enable_pcon")
    defer os.Setenv("MSYS", originalMSYS)
  } else {
    if isWinptyAvailable() {
      // Use fzf-0.52.1-dev code ...
    } else {
      // Use fzf-0.52.1     code ...
    }
  }
}

func compareVersions(v1, v2 string) int {
  parts1 := strings.Split(v1, ".")
  parts2 := strings.Split(v2, ".")
  for i := 0; i < len(parts1) && i < len(parts2); i++ {
    if parts1[i] > parts2[i] {
      return 1
    } else if parts1[i] < parts2[i] {
      return -1
    }
  }
  return 0
}

func isWinptyAvailable() bool {
  _, err := exec.LookPath("winpty")
  return err == nil
}

May 23 '24 07:05 Konfekt

@Konfekt Thanks, but my Git bash reports 3.7.1 but fzf doesn't run without winpty.

MSYS=enable_pcon ./fzf --no-winpty works great, and it gives accurate colors and border.

enable_pcon
winpty

May 23 '24 08:05 junegunn

However, even with MSYS=enable_pcon, --height doesn't work.

May 23 '24 09:05 junegunn

my Git bash reports 3.7.1 but fzf doesn't run without winpty.

Hm... 3.7.0 was a guess; supposedly 3.6.4 should have worked, but neither did for me. 3.7.1. is used in latest https://github.com/git-for-windows/build-extra/blob/34a82b52c89920c425b691e8ae61a8eed47152dd/versions/package-versions-2.45.1.txt and it worked for me in Scoop; according to their (install script](https://github.com/ScoopInstaller/Main/blob/master/bucket/git.json) they don't seem to make many adaptions.

even with MSYS=enable_pcon, --height doesn't work.

Was this ever addressed? In 573df52 (#3807) it seemed like rather not

May 23 '24 09:05 Konfekt

For what it's worth, I also content myself with

func main() {
  termProgramVersion := os.Getenv("TERM_PROGRAM_VERSION")
  if compareVersions(termProgramVersion, "3.4.5") >= 0 {
    originalMSYS := os.Getenv("MSYS")
    os.Setenv("MSYS", "enable_pcon")
    defer os.Setenv("MSYS", originalMSYS)
  } else {
    if isWinptyAvailable() {
      // Use fzf-0.52.1-dev code ...
    } else {
      // Use fzf-0.52.1     code ...
    }
  }
}

May 23 '24 09:05 Konfekt

@Konfekt Please review and test https://github.com/junegunn/fzf/commit/d4216b0dcc13567479d81cc5ad2adedb1443ea8b

May 23 '24 09:05 junegunn

Would you mind attaching the exe? I did not find a devel mingw compile action https://github.com/junegunn/fzf/actions

May 23 '24 09:05 Konfekt

@Konfekt fzf.exe.zip

May 23 '24 09:05 junegunn

It works fine in Git Bash 2.42.0. Mintty < 3.4.5 is three years old, so maybe it's not worth to rack one's brain too much about it, even less so for Mintty < 3.4.5 without winpty.

For the Vim plugin, the condition has become

elseif has('win32unix') && $TERM_PROGRAM ==# 'mintty' && $TERM_PROGRAM_VERSION =~# '\v^[0-2]\.|3\.[0-3]|3\.4\.[0-4]' && !executable('winpty')

May 23 '24 10:05 Konfekt

@Konfekt Thanks. See https://github.com/junegunn/fzf/commit/c36b846accc36c5a833a8e22d06107b778a4f219

May 23 '24 12:05 junegunn

Nice, a moment's thought would have suggested to me that there's already a s:compare_versions for such a long-lived versatile program.

May 23 '24 12:05 Konfekt

On fzf.vim opening in new cmd Window in Git Bash

Checklist

Output of fzf --version

OS

Shell

Problem / Steps to reproduce

Output of `fzf --version`