-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
On fzf.vim opening in new cmd Window in Git Bash #3809
Comments
This checks for the Win32 version of Vim, using Unix files (Cygwin), according to As So yes, one could likely check for something like Likely only its original author @janlazo can tell us why |
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. |
Is it? What does branch refer to? If you mean spawning cmd.exe, that would be great.
but seems somewhat outdated, since now the dev branch already supports Fzf in Git Bash default Mintty term. Mind that #3804 asks similarly if the spawning of cmd.exe could be removed. let command = 'start /WAIT sh -c '.shellscript suffice? |
If I understand correctly, in latest Git Bash simply instead of the involved cmd.exe shell script execution suffices. |
For me If you could confirm that all commands work as expected under the Git Bash Terminal as well? |
Maybe that could be part of the next release since it yet improves again on the Git Bash experience |
Since This would also make it clearer, if guessed correctly, why all the convolution Lines 711 to 716 in daa6024
|
@Konfekt Can you open a pull request against devel branch? |
Interestingly, I can use fzf in Git Bash without winpty. I don't know why that is. fzf also works for me with no |
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. |
This is the Git Bash terminal from Git for Windows. It is from |
I can confirm that fzf.exe 0.52.1 works in Git Bash Mintty under Windows 11; it does not under Window 10. |
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 {
... |
* Git Bash Mintty: only use cmd.exe if winpty missing Addresses #3809 * preferably use term in Git Bash for popup window See #3811 (comment)
No, this was not about the OS versions. It was rather about the difference between git 2.42.0 and 2.45.1. |
@ykhan21 You may check with an older Git Bash, confirming #3809 (comment) |
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 |
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
Indeed MSYS=enable_pcon makes fzf work even on older Git Bash terms |
So Fzf could suggest
Once settled how fzf handles this, the Wiki entry https://github.com/junegunn/fzf/wiki/Cygwin can be updated |
Good thing: Mintty provides the environment variable 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
} |
@Konfekt Thanks, but my Git bash reports
|
However, even with |
Hm... 3.7.0 was a guess; supposedly 3.6.4 should have worked, but neither did for me.
Was this ever addressed? In |
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 ...
}
}
} |
Would you mind attaching the exe? I did not find a devel mingw compile action https://github.com/junegunn/fzf/actions |
It works fine in Git Bash 2.42.0. 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') |
Nice, a moment's thought would have suggested to me that there's already a |
Maybe time has come to update the wiki |
Has anyone tested with Windows Terminal(different from the builtin cmd console) as an alternative terminal to mintty? |
Windows Terminal was never a trouble-maker to begin with, |
I had ( |
There is this old issue with fzf+Git Bash. I wonder if the new changes fixes it. |
Thanks for pointing that out, so it's indeed a list of options. There are some doubts on their separators, though. I found To complicate Portable Git now enables pcon, whereas the classic install not. |
Somehow I'd trouble with |
Which "Fzf plug-ins" do you mean? Are you referring to the shell integration scripts in this repo? I don't think they're supposed to work on cmd or powershell. |
Hmm, |
With a series of fixes, now everything seems to work nicely.
You can test this binary: |
I checked with https://github.com/chrisant996/clink-fzf and https://github.com/kelleyma49/PSFzf on cmd and powershell. It's working fine now. Good work! |
Thanks for the fix. |
Checklist
man fzf
)Output of
fzf --version
0.52.1 (6432f00)
OS
Shell
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.
fzf/plugin/fzf.vim
Lines 711 to 716 in daa6024
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.
The text was updated successfully, but these errors were encountered: