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

Non-cygwin subprocesses are not handling the ANSI escape sequence #6634

Closed
trajano opened this issue Jun 22, 2020 · 8 comments
Closed

Non-cygwin subprocesses are not handling the ANSI escape sequence #6634

trajano opened this issue Jun 22, 2020 · 8 comments
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-External For issues that are outside this codebase

Comments

@trajano
Copy link

trajano commented Jun 22, 2020

This is a follow up to #2837 which is already resolved in that the a large set of ANSI is already handled by Windows Terminal. This seems to propagate properly through any other tools that use the cygwin/msys libraries such as ls

However, it does not appear to handle ANSI escape sequences for processes that do not use Cygwin/MSys in those scenarios the raw escape codes appear.

I am thinking this is likely an External bug, but I wanted to double check since I saw one of the Terminal devs using something called "Libra" to find the process flow to debug something like this. It could likely be something about setting an envvar or something to trigger a proper call.

Environment

Microsoft Windows [Version 10.0.19041.329]

Windows Terminal version (if applicable):
Version: 1.0.1401.0

Any other software?
git version 2.27.0.windows.1

Steps to reproduce

  1. Set up Git Bash as your shell https://stackoverflow.com/questions/56839307/adding-git-bash-to-the-new-windows-terminal/57369284#57369284
  2. Have a Gradle project or some other program that would spit out ANSI escape sequences that does not use MSys/Cygwin.
  3. Run ./gradlew build

Expected behavior

ANSI escape sequences are parsed and handled like this in the Git Bash shortcut provided by Git for Windows

image

Actual behavior

ANSI escape sequences are visible and not handled in Windows Terminal's shell
image

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 22, 2020
@DHowett
Copy link
Member

DHowett commented Jun 22, 2020

So, here's the deal.

This is, quite firmly, a client application problem. I know, that's not the "right" answer. It is, however, the self-consistent one.

We didn't always ship with support for VT sequences, so a bunch of applications took a dependency on the ability to print raw control characters directly to the screen. I realize that that sounds like insanity. However, because we have to support them (forever!), we leveraged a system we already had:

The Windows Console subsystem has the notion of "modes". Input modes and output modes. Back in Windows 10 TH1 (10.0.10240) we introduced the output mode ENABLE_VIRTUAL_TERMINAL_PROCESSING. It's off by default so that legacy applications continue to work properly.

When that mode is enabled, the console handles escape sequences. When that mode is disabled, it prints the escape character 0x1B directly to the screen as a tiny "left arrow" glyph.

When we added support for ENABLE_VT_PROCESSING, we also added a registry key called VirtualTerminalLevel (DWORD). When it's set to 0x01, ENABLE_VIRTUAL_TERMINAL_PROCESSING is turned on by default.

None of this has changed since Windows 10 10.0.10240.0. The Terminal doesn't impact the console modes*, because it is running on the other side of the console. It's a replacement for the user's eyes seeing the console window, and it does not (cannot!) impact the console mode.

That footnote, though. That footnote haunts me. Terminal doesn't impact the console modes in a way that would make this situation worse. Terminal sets up the console host it talks to in such a way as to always enable ENABLE_VT_PROCESSING by default. There's a one-way one-time change here, and that change is "by default, on startup, the console application WILL BE IN VT MODE".

Any application found to not be in VT mode has explicitly turned it off by calling SetConsoleMode() without ENABLE_VIRTUAL_TERMINAL_PROCESSING.

@DHowett
Copy link
Member

DHowett commented Jun 22, 2020

(The "Git Bash" shortcut does not, by default, use the windows console susbsytem. It is unfortunately invalid to use it for these comparisons. There's a special handshake that happens between mintty and mingw/cygwin that they use to light up special behaviors.)

FWIW: The application that's changing the console mode to turn off VT processing is likely to be java.exe.

@DHowett
Copy link
Member

DHowett commented Jun 22, 2020

And one last note: I think the Libra you're referring to is my personal laptop 😄 I do a lot of terminal engineering on it, though, so it shows up all the time.

If you're talking about what I think you are, though, that's the "debug tap". More info inside the detail region below.

Info about the debug tap

The debug tap lets us see what raw output is coming out of the console translation layer, and what raw input is going into it. If you set the setting debugFeaturesEnabled to true, and then press both Alt keys while opening a new tab (with the menu), you'll get this split-screen view:

image

EDIT: The setting is "debugFeatures": true, not debugFeaturesEnabled

@trajano
Copy link
Author

trajano commented Jun 22, 2020

FWIW: The application that's changing the console mode to turn off VT processing is likely to be java.exe.

I was thinking something along those lines so that means existing programs that want to render properly in Terminal and is executed behind cygwin needs to be updated then.

I am guessing the code that someone has to change is here... https://github.com/openjdk/jdk/blob/master/src/java.base/windows/native/libjava/Console_md.c where it just needs some way of putting in ENABLE_VIRTUAL_TERMINAL_PROCESSING.

When we added support for ENABLE_VT_PROCESSING, we also added a registry key called VirtualTerminalLevel (DWORD). When it's set to 0x01, ENABLE_VIRTUAL_TERMINAL_PROCESSING is turned on by default.

I tried to enable it using the instructions in https://stackoverflow.com/a/51681675/242042 but to no avail, the gradlew inside bash inside Windows terminal still does not process the ANSI sequences. So I am leaning towards the code fix being needed by Java.

They appear to have some logic for in the jline library https://github.com/openjdk/jdk/blame/a9952bb5d9fe2fbf12d4ba43608dfc830180ec86/src/jdk.internal.le/windows/classes/jdk/internal/org/jline/terminal/impl/jna/win/JnaWinSysTerminal.java#L46 though it is conditional

@DHowett
Copy link
Member

DHowett commented Jun 22, 2020

Since there's no Terminal-specific magic, it's less about "being inside terminal" and more about being a good windows console subsystem citizen... but, essentially, yes.

@trajano
Copy link
Author

trajano commented Jun 22, 2020

I wonder if there's a way for MSys to detect SetConsoleMode() from a subprocess and adjust the calls accordingly.

@DHowett
Copy link
Member

DHowett commented Jun 25, 2020

I'd rather just handle that with #4954, which would isolate all of these processes from eachother. One day, one day.

Closing this one as discussed/external 😄

@trajano
Copy link
Author

trajano commented Feb 2, 2021

Adding a link to a potential solution for apps that have weird issues and how to fix it when setting the VT_PROCESSING does not work starship/starship#2258 (comment) it's a bit of an odd solution though (setting stdin to null)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-External For issues that are outside this codebase
Projects
None yet
Development

No branches or pull requests

2 participants