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

CMD in "Current Console Mode" not cleared when vim exits #3177

Closed
1 task done
kevinoid opened this issue Apr 15, 2021 · 28 comments
Closed
1 task done

CMD in "Current Console Mode" not cleared when vim exits #3177

kevinoid opened this issue Apr 15, 2021 · 28 comments

Comments

@kevinoid
Copy link

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options

git version 2.31.1.windows.1
cpu: x86_64
built from commit: c5f0be26a7e3846e3b6268d1c6c4800d838c6bbb
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver

Microsoft Windows [Version 10.0.19042.928]
  • What options did you set as part of the installation? Or did you choose the defaults?
> type "C:\Program Files\Git\etc\install-options.txt"

Editor Option: VIM
Custom Editor Path:
Default Branch Option:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Core
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
  • Any other interesting things about your environment that might be related to the issue you're seeing?

None that I am aware of.

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

CMD (with Legacy Console Mode disabled)

The problem occurs for after commands which invoke vim (e.g. git commit, git diff, git rebase -i, etc.). An easy way to reproduce the issue from inside or outside a repo:

git config --system -e
  • What did you expect to occur after running these commands?

Console would be cleared when vim is exited as it is in Legacy Console Mode:
Screenshot of CMD in Legacy Console Mode after exiting vim from git config --system -e

  • What actually happened instead?

Console is not cleared and file content being edited in vim remains in the console around the prompt:
Screenshot of CMD in Current Console Mode after exiting vim from git config --system -e

Note: The issue does not appear if git is configured to use vim v8.2.2767 from vim/vim-win32-installer. The issue appears to be specific to the msys2 version of vim shipped with git.

Thanks for reading,
Kevin

@dscho
Copy link
Member

dscho commented Apr 15, 2021

Would you mind testing this with the MSYS2 runtime v3.2.0 as built here?

You'll need to download the install.zip artifact from the latest build (currently https://github.com/git-for-windows/msys2-runtime/actions/runs/730930260, the artifact is here) and unzip it into your installation (I would recommend using the Portable Git variant of Git for Windows so that you can be sure not to hose any existing installation).

I ask because there has been a ton of work on the pseudo terminal front in v3.2.0, and the issue you reported might Just Be Fixed there already.

Thank you!

@kevinoid
Copy link
Author

Thanks @dscho! Good idea.

After extracting install.zip into C:\Program Files\Git and overwriting matching files, the behavior appears to be unchanged. (Thanks for the warning! I'm testing in a VM with snapshots to avoid corruption/confusion when making changes.)

However, I discovered that the issue does not occur when vim is invoked directly:

cd "C:\Program Files\Git" && usr\bin\vim etc\gitconfig

However, it does occur when invoked directly if TERM=cygwin is set in the environment (as it is when invoked by git):

set TERM=cygwin
cd "C:\Program Files\Git" && usr\bin\vim etc\gitconfig

The above behavior is consistent between the msys runtime shipped with the current release and the build you suggested. TERM=cygwin does not appear to affect vim v8.2.2767 from vim/vim-win32-installer.

@dscho
Copy link
Member

dscho commented Apr 22, 2021

I had another look at this and was actually able to reproduce the behavior, but not when running with MSYS2 runtime v3.2.0 and setting MSYS=enable_pcon (i.e. the setting that Enable Pseudo Console Support: Disabled refers to).

Can you confirm my finding?

@kevinoid
Copy link
Author

Thanks for investigating @dscho. Good find!

Unfortunately, I'm unable to confirm. After extracting install.zip into C:\Program Files\Git and overwriting matching files, when I open cmd.exe and run

set MSYS=enable_pcon
git config --system -e

The console is not cleared after exiting vim. In case it is relevant, $TERM is cygwin in vim and I can also still reproduce the issue when invoking vim directly:

set MSYS=enable_pcon
set TERM=cygwin
cd "C:\Program Files\Git" && usr\bin\vim etc\gitconfig

Let me know if you are unable to reproduce the issue with MSYS=enable_pcon and I can find a way to share a reliable reproduction environment.

@dscho
Copy link
Member

dscho commented Apr 22, 2021

Funnily enough, when I set TERM=xterm256-color, then call git config -e and then set it to cygwin and try again, it works...

@kevinoid
Copy link
Author

Funnily enough, when I set TERM=xterm256-color, then call git config -e and then set it to cygwin and try again, it works...

Interesting! I am able to reproduce the behavior you described with MSYS2 runtime v3.2.0:

set MSYS=enable_pcon
set TERM=xterm256-color
git config --system -e
set TERM=cygwin
git config --system -e
git config --system -e

The console is not cleared after exiting vim the first time. It is cleared the second time. It is not cleared the third time (or any subsequent times).

I think the cause might be related to the cursor position when vim is started. For me, the console is cleared if the cursor is at the bottom of the window when vim is started: If I press Enter until the cursor is at the bottom of the window, then run git config --system -e, the console is cleared when vim exits, regardless of environment or MSYS2 runtime version.

@dscho
Copy link
Member

dscho commented Apr 23, 2021

Since this is an issue even with enable_pcon, maybe it even shows up with Cygwin? (If so, we could hope for a fix from that project.)

@kevinoid
Copy link
Author

kevinoid commented Apr 23, 2021

maybe it even shows up with Cygwin?

I tried running running vim in the Cygwin64 Terminal from Cygwin 3.2.0 and got:

Kevin@DESKTOP-3KATR88 /cygdrive/c/Program Files/Git
$ ./usr/bin/vim.exe etc/gitconfig
      2 [main] vim (6468) C:\Program Files\Git\usr\bin\vim.exe: *** fatal error
- cygheap base mismatch detected - 0x180357408/0x180347408.
This problem is probably due to using incompatible versions of the cygwin DLL.
Search for cygwin1.dll using the Windows Start->Find/Search facility
and delete all but the most recent version.  The most recent version *should*
reside in x:\cygwin\bin, where 'x' is the drive on which you have
installed the cygwin distribution.  Rebooting is also suggested if you
are unable to find another cygwin DLL.

I can investigate further, but I am not sure it would be worth the effort.

Since the issue only occurs with TERM=cygwin, and it seems reasonable that setting TERM=cygwin might cause vim to use terminal escape sequences not supported by cmd.exe, perhaps it would make sense to consider not setting TERM=cygwin in setup_windows_environment? I don't see any reference to TERM in color.c as the comment suggests and it does not appear to be necessary for colors in vim. Are there still cases where it is necessary?

Note: Vim supports terminal information with several builtin_terms enabled. When TERM is unset, vim sets 'term' to cygwin in "Legacy Console Mode" and xterm-256color in "Current Console Mode". If TERM must be set, perhaps it could be done using similar logic to however vim detects these?

@dscho
Copy link
Member

dscho commented Apr 26, 2021

/cygdrive/c/Program Files/Git
$ ./usr/bin/vim.exe etc/gitconfig

I meant Cygwin's own vim, in a CMD window.

it seems reasonable that setting TERM=cygwin might cause vim to use terminal escape sequences not supported by cmd.exe, perhaps it would make sense to consider not setting TERM=cygwin in setup_windows_environment?

We specifically set TERM so that Cygwin (or more correctly, the MSYS2 runtime) uses ANSI sequences...

@kevinoid
Copy link
Author

I meant Cygwin's own vim, in a CMD window.

On my system, C:\cygwin64\bin\vi.exe from the Cygwin 3.2.0 vim-minimal package installed by default exhibits the same behavior as vim shipped with git: With TERM unset, it clears the cmd window on exit, with TERM=cygwin it does not, with TERM=xterm-256color it does.

We specifically set TERM so that Cygwin (or more correctly, the MSYS2 runtime) uses ANSI sequences...

Good to know. Presumably that is programs using the msys2 runtime other than vim (which uses colors in cmd when TERM is unset on my system)?

Keep in mind, different terminal emulators support different sets of "ANSI sequences". I suspect TERM=cygwin is not the correct choice for cmd in "Current Console Mode". If it must be set, perhaps it could be chosen using similar logic to how vim chooses between cygwin and xterm-256color when TERM is unset?

@dscho
Copy link
Member

dscho commented Apr 26, 2021

I meant Cygwin's own vim, in a CMD window.

On my system, C:\cygwin64\bin\vi.exe from the Cygwin 3.2.0 vim-minimal package installed by default exhibits the same behavior as vim shipped with git: With TERM unset, it clears the cmd window on exit, with TERM=cygwin it does not, with TERM=xterm-256color it does.

Great, so we're getting closer to pinpointing this on Cygwin :-)

I vaguely remember a little flurry of ConPTY-related patches entering Cygwin. You wouldn't have time to cherry-pick them on top of git-for-windows/msys2-runtime#30 and let GitHub Actions rebuild the runtime, would you?

Keep in mind, different terminal emulators support different sets of "ANSI sequences". I suspect TERM=cygwin is not the correct choice for cmd in "Current Console Mode". If it must be set, perhaps it could be chosen using similar logic to how vim chooses between cygwin and xterm-256color when TERM is unset?

Possible. Do you know how vim chooses?

@kevinoid
Copy link
Author

I vaguely remember a little flurry of ConPTY-related patches entering Cygwin. You wouldn't have time to cherry-pick them on top of git-for-windows/msys2-runtime#30 and let GitHub Actions rebuild the runtime, would you?

Sure. I cherry-picked

onto dscho:rebase-to-v3.2.0 and pushed it as kevinoid:rebase-to-v3.2.0-with-conpty. build #1 is currently running.

Warning: The second patch required conflict resolution. I think I resolved it correctly, but I would recommend against using my branch for anything other than testing.

Possible. Do you know how vim chooses?

It looks like the choice is made by cygwin/msys2 in choosing which terminal to emulate. In "Legacy Console Mode":

C:\>"C:\Program Files\Git\usr\bin\tset.exe" -q
cygwin

In "Current Console Mode":

C:\>"C:\Program Files\Git\usr\bin\tset.exe" -q
xterm-256color
C:\>set TERM=cygwin

C:\>"C:\Program Files\Git\usr\bin\tset.exe" -q
cygwin

@dscho
Copy link
Member

dscho commented Apr 27, 2021

Possible. Do you know how vim chooses?

It looks like the choice is made by cygwin/msys2 in choosing which terminal to emulate

I bet it is this line: https://github.com/git-for-windows/msys2-runtime/blob/12e88192b84673b81c6550e48fb7df9b607dada3/winsup/cygwin/environ.cc#L962

It calls wincap.has_con_24bit_colors(), which, if I read the code correctly, essentially tests for Windows 10 1703 or above: https://github.com/git-for-windows/msys2-runtime/blob/12e88192b84673b81c6550e48fb7df9b607dada3/winsup/cygwin/wincap.cc#L248.

More interesting, however, is how con_is_legacy is set: apparently, we see whether we can toggle ENABLE_VIRTUAL_TERMINAL_PROCESSING successfully and if not, we know this is a legacy console.

Now, there are problems with doing that in git.exe, too ;-) The main problem is that we may not want to enable virtual processing (lest we're running within a console application that does not expect this flag to be flipped!). So when we run git.exe, TERM=cygwin might actually be correct, and only when we run vim from within git.exe (which, by virtue of vim.exe being an MSYS2 program which in turn starts up the MSYS2 runtime, which then toggles that switch under our feet), that setting is no longer that correct...

@kevinoid
Copy link
Author

Good find and analysis! That makes a lot of sense.

For what it's worth, I fixed a conflict resolution error in the second commit and build #2 succeeded. Unfortunately, it exhibits the same behavior as the other versions.

@dscho
Copy link
Member

dscho commented Apr 28, 2021

Right. With all that information as armor, do you think you can take this to the Cygwin mailing list? Like, I am pretty certain that their enabling the virtual terminal processing when TERM=cygwin is undesirable. But they might have different ideas.

@kevinoid
Copy link
Author

Right. With all that information as armor, do you think you can take this to the Cygwin mailing list?

Almost. I'm still unclear on is why git must set TERM at all. Could you clarify a bit more? Perhaps an example of something that would misbehave if mingw.c:3330-3332 were removed.

@kevinoid
Copy link
Author

Right. With all that information as armor, do you think you can take this to the Cygwin mailing list?

Almost. I'm still unclear on is why git must set TERM at all.

Alternatively, if that's not clear, and there are concerns about unknown backwards compatibility risks, I'm willing to raise the issue on the Cygwin mailing list as is. But I wouldn't be surprised if "it doesn't work when I do this" is met with "then don't do that". 😆

@kevinoid
Copy link
Author

I went ahead and posed the question to the Cygwin mailing list: https://cygwin.com/pipermail/cygwin/2021-April/248406.html

@LuanVSO
Copy link

LuanVSO commented May 5, 2021

i am also seeing this but i have

Bash Terminal Option: ConHost
Enable Pseudo Console Support: Enabled

on my install-options.txt

and if i run git config --global -e on git bash it works fine but not on git CMD.
env of git bash:

SHELL=/usr/bin/bash
TERM=xterm-256color
MSYS=enable_pcon

env of git CMD:

SHELL=
TERM=
MSYS=enable_pcon

output of tset on git cmd is xterm-256color
also, just running vim on git cmd or git bash works fine

windows version: 10.0.19043.964
Legacy Console Mode: disabled

@LuanVSO
Copy link

LuanVSO commented May 5, 2021

if i manually do set TERM=xterm-256color it works correctly

@dscho
Copy link
Member

dscho commented Mar 10, 2022

There has been again a flurry of ConPTY fixes, as well as some work-around in Git for Windows specifically for calling vim and re-setting the console afterwards.

Could I solicit y'all's assistance in testing with the latest snapshot? I'd like to close this ticket.

@kevinoid
Copy link
Author

Thanks @dscho. I'm still able to reproduce the issue with git version 2.35.1.windows.2.7.g158a30d60b.20220224121117.

@dscho
Copy link
Member

dscho commented Mar 14, 2022

@kevinoid Thank you for testing. I don't want to give you any illusions: I won't be able to investigate this issue let alone working on resolving it, that's why I hoped that we would "inherit" fixes from the Cygwin project by virtue of rebasing the MSYS2 runtime to the newest Cygwin runtime version. I still hope that that will get us a fix for your issue in the long run.

@kevinoid
Copy link
Author

Thanks @dscho, I appreciate the clarity. Is that due to lack of time, or other factors? Either is understandable, I'm just curious if there may be anything I can do to unblock it.

@dscho
Copy link
Member

dscho commented Mar 14, 2022

Time. I'm really scarce on unplanned-for time.

LuanVSO added a commit to LuanVSO/pwsh-profile that referenced this issue Jan 13, 2023
remove workaround for git-for-windows/git#3177
vim now correctly clears the screen when exiting on conPTY
@dscho
Copy link
Member

dscho commented Nov 22, 2023

LuanVSO/pwsh-profile@3806c56 claims that this may no longer be an issue?

@LuanVSO
Copy link

LuanVSO commented Nov 23, 2023

Correct, I've been using it without the workaround for quite some time, sorry for forgetting to update this thread

@dscho dscho closed this as completed Nov 23, 2023
@dscho
Copy link
Member

dscho commented Nov 23, 2023

Good. The ticket is addressed, then.

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