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

Git Bash Mintty: only use cmd.exe if winpty missing #3811

Merged
merged 2 commits into from
May 23, 2024

Conversation

Konfekt
Copy link
Contributor

@Konfekt Konfekt commented May 22, 2024

Addresses #3809

@junegunn
Copy link
Owner

junegunn commented May 22, 2024

Thanks. While testing the patch, I have noticed a few things.

  • When fzf is started from Git bash, CTRL-C fails to properly terminate fzf and I get error restoring terminal mode: Input/output error message. I'll look into it. (Fixed in 303c3ba)
  • The patch works; fzf opens in full-screen mode. But we also have that CTRL-C problem.
  • Looks like we can use built-in terminal of Vim. It works nicely, and fzf opens in the central popup window inside Vim. We can do this in addition to the patch. What do you think?
diff --git a/plugin/fzf.vim b/plugin/fzf.vim
index eee5e9c..290db45 100644
--- a/plugin/fzf.vim
+++ b/plugin/fzf.vim
@@ -530,7 +530,7 @@ try
   let has_vim8_term = has('terminal') && has('patch-8.0.995')
   let has_nvim_term = has('nvim-0.2.1') || has('nvim') && !s:is_win
   let use_term = has_nvim_term ||
-    \ has_vim8_term && !has('win32unix') && (has('gui_running') || s:is_win || s:present(dict, 'down', 'up', 'left', 'right', 'window'))
+    \ has_vim8_term && (has('gui_running') || s:is_win || s:present(dict, 'down', 'up', 'left', 'right', 'window'))
   let use_tmux = (has_key(dict, 'tmux') || (!use_height && !use_term || prefer_tmux) && !has('win32unix') && s:splittable(dict)) && s:tmux_enabled()
   if prefer_tmux && use_tmux
     let use_height = 0
  • However, the border around fzf is for some reason not visible
    image
  • When I select a file from fzf, I'm getting this error.
    image
    • Seems to be caused by mixed / and \.

@junegunn
Copy link
Owner

CTRL-C problem is fixed in 303c3ba.

@Konfekt
Copy link
Contributor Author

Konfekt commented May 22, 2024

When fzf is started from Git bash, CTRL-C fails to properly terminate fzf and I get error restoring terminal mode: Input/output error message. I'll look into it

I can't reproduce it with Windows 10, Git Bash 2.42.0 and latest fzf 0.52.1dev (without winpty), neither in Git Bash nor inside Vim in Git Bash. However, exiting, independent of the shortcut, sends an 130 signal. That might be on purpose, though.

We can do this in addition to the patch. What do you think?

I think that's a good idea. My slight adaption tries to keep everything working where winpty is not readily available, say older installs of Git on administered Window work machines, in harmony with the change at the other line.

Let me suggest an additional check for the existence of winpty at 573df52 (#3807) to this end.

@junegunn
Copy link
Owner

junegunn commented May 22, 2024

Let me suggest an additional check for the existence of winpty at 573df52 (#3807) to this end.

So what do we do when winpty is not available? I decided to not perform the check and let it fail because I thought fzf wouldn't run it anyway without winpty, but according to the latest discussion in #3809 (comment) that is not necessarily true?

@junegunn
Copy link
Owner

junegunn commented May 23, 2024

  • winpty detection: b5b0d6b
  • Fix mixed forward and backward slashes: 561f929

@junegunn junegunn merged commit bfe2bf4 into junegunn:devel May 23, 2024
2 checks passed
@Konfekt
Copy link
Contributor Author

Konfekt commented May 23, 2024

fzf wouldn't run it anyway without winpty, but according to the latest discussion in #3809 (comment) that is not necessarily true?

When I removed winpty on Windows 10 with fzf 0.52.1-dev then :FZF in Vim in Git Bash Mintty with

  elseif has('win32unix') && $TERM_PROGRAM ==# 'mintty' " && !executable('winpty')
    let shellscript = s:fzf_tempname()
    call s:writefile([command], shellscript)
    let command = 'start //WAIT sh -c '.shellscript
    let a:temps.shellscript = shellscript
  endif

the pop up shows

grafik

whereas fzf 0.52.1 works fine.

So it was meant that 0.52.1-dev keeps doing in the absence of winpty on Windows 11 what 0.52.1 did

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

Successfully merging this pull request may close these issues.

None yet

2 participants