On Fri, 10 Dec 2021 00:05:27 +0100 (CET) Johannes Schindelin wrote: > sorry for responding to a patch you sent almost 10 months ago... but... I > am struggling with it. > > First of all, let me describe the problem I am seeing (see also > https://github.com/git-for-windows/git/issues/3579): after upgrading the > MSYS2 runtime to v3.3.3 in Git for Windows, whenever I ask `git.exe` to > spawn `vim.exe` to edit any file, after quitting `vim` I see spurious ANSI > sequences being "ghost-typed" into the terminal (which is a MinTTY running > under `TERM=xterm`). > > Apparently the ANSI sequences report the cursor position and the > foreground/background color in response to a CSI [ 6n sent from `vim`. > > Clearly, those sequences should go to `vim.exe`, but they mostly don't > arrive there (but in MinTTY instead, as if I had typed them). Sometimes, > the foreground/background color seems to arrive in the `vim` process, but > the cursor position almost always does not. I suspect that it is important > that `git.exe` is a non-MSYS2 process whereas `vim.exe` is an MSYS2 > process, and something inside the MSYS2 runtime is at fault. > > I've bisected this incorrect behavior to the patch I am replying to. > > I tried to trigger the same bug in pure Cygwin (as opposed to MSYS2), > specifically using `disable_pcon` (because MSYS2 defaults to not using the > pseudo console support because I ran into too many issues to be confident > enough in it yet), but I think that Cygwin's `vim` is too old and > therefore might not even send that CSI [ 6n (although `:h t_RV` _does_ > show the expected help). > > Now, the patch which I am responding to is completely obscure to me. It is > very, very unclear to me whether it really tries to only do one thing > (namely to transfer the input no longer in `read()` but in `setpgid()`), > or rather does many things at once. Even worse, I have not the faintest > clue how this patch is trying to accomplish what the commit message > describes (_because_ it does so many things at once), nor how that could > be related to the observed incorrect behavior, and as a consequence I have > no idea how I can hope to fix said observed incorrect behavior. > > Could you help shed some light into the problem?
Thanks for the report. Could you please test if the following patch solves the issue? diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index f523dafed..ba282b897 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -1239,10 +1239,13 @@ fhandler_pty_slave::mask_switch_to_pcon_in (bool mask, bool xfer) else if (InterlockedDecrement (&num_reader) == 0) CloseHandle (slave_reading); + bool need_xfer = + get_ttyp ()->switch_to_pcon_in && !get_ttyp ()->pcon_activated; + /* In GDB, transfer input based on setpgid() does not work because GDB may not set terminal process group properly. Therefore, transfer input here if isHybrid is set. */ - if (isHybrid && !!masked != mask && xfer + if ((isHybrid || need_xfer) && !!masked != mask && xfer && GetStdHandle (STD_INPUT_HANDLE) == get_handle ()) { if (mask && get_ttyp ()->pcon_input_state_eq (tty::to_nat)) @@ -1536,7 +1539,7 @@ out: if (ptr0) { /* Not tcflush() */ bool saw_eol = totalread > 0 && strchr ("\r\n", ptr0[totalread -1]); - mask_switch_to_pcon_in (false, saw_eol); + mask_switch_to_pcon_in (false, saw_eol || len == 0); } } @@ -2214,6 +2217,15 @@ fhandler_pty_master::write (const void *ptr, size_t len) return len; } + if (to_be_read_from_pcon () && !get_ttyp ()->pcon_activated + && get_ttyp ()->pcon_input_state == tty::to_cyg) + { + WaitForSingleObject (input_mutex, INFINITE); + fhandler_pty_slave::transfer_input (tty::to_nat, from_master, + get_ttyp (), input_available_event); + ReleaseMutex (input_mutex); + } + line_edit_status status = line_edit (p, len, ti, &ret); if (status > line_edit_signalled && status != line_edit_pipe_full) ret = -1; -- Takashi Yano <takashi.y...@nifty.ne.jp>