microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
94.4k stars 8.17k forks source link

App output different in Terminal than conhost with null characters #8680

Open asklar opened 3 years ago

asklar commented 3 years ago

Environment

Windows build number: 21280
Windows Terminal version (if applicable): 1.5.3242.0 (Preview)

Any other software? 

Steps to reproduce

Run the project in https://github.com/asklar/textlogo The program initializes a text buffer, draws something in it, then prints it to stdout. It looks like Terminal treats Console.WriteLine('\0') as a no-op when it should be interpreting it as "write a space". If I initialize my buffer to spaces, then the bug goes away, but this should not be necessary and it's a breaking behavior between conhost and terminal.

Expected behavior

In conhost, this prints: image

Actual behavior

In Terminal, this prints: image

KalleOlaviNiemitalo commented 3 years ago

AFAIK, output from a computer to a terminal has historically used null characters as padding, to give the terminal time to process a command that the computer sent earlier. The terminal is not expected to display the null characters as spaces.

asklar commented 3 years ago

The ASCII standard allows NUL to affect layout: https://web.archive.org/web/20140512221203/http://kikaku.itscj.ipsj.or.jp/ISO-IR/001.pdf image

Wikipedia says "some terminals, however, incorrectly display it as space". Regardless of whether it is correct for conhost to do so, that's where we are and have been for a long time. If Windows Terminal ever wants to be able to replace conhost, it must behave in a backwards compatible way and not break a behavior that's been around for 30-40+ years.

KalleOlaviNiemitalo commented 3 years ago

If you clear the ENABLE_VIRTUAL_TERMINAL_PROCESSING output mode first, then the display is what you wanted.

        static void DisableOutputVirtualTerminalProcessing()
        {
            IntPtr handle = GetStdHandle(STD_OUTPUT_HANDLE);
            if (GetConsoleMode(handle, out uint mode))
            {
                Console.WriteLine($"Original output modes: {mode:X}");
                mode &= ~ENABLE_VIRTUAL_TERMINAL_PROCESSING;
                Console.WriteLine($"New output modes: {mode:X}");
                if (SetConsoleMode(handle, mode))
                {
                    Console.WriteLine("Changed the output modes OK.");
                }
            }
        }

        [System.Runtime.InteropServices.DllImport("kernel32")]
        static extern IntPtr GetStdHandle(int nStdHandle);

        [System.Runtime.InteropServices.DllImport("kernel32")]
        [return: System.Runtime.InteropServices.MarshalAs(System.Runtime.InteropServices.UnmanagedType.Bool)]
        static extern bool GetConsoleMode(IntPtr hConsoleHandle, out uint lpMode);

        [System.Runtime.InteropServices.DllImport("kernel32")]
        [return: System.Runtime.InteropServices.MarshalAs(System.Runtime.InteropServices.UnmanagedType.Bool)]
        static extern bool SetConsoleMode(IntPtr hConsoleHandle, uint dwMode);

        const int STD_OUTPUT_HANDLE = -11;
        const uint ENABLE_VIRTUAL_TERMINAL_PROCESSING = 4;
KalleOlaviNiemitalo commented 3 years ago

If you set the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode first, then the output loses the spaces even in conhost.

j4james commented 3 years ago

I believe this is a duplicate of issue #4363. See also #6265.

KalleOlaviNiemitalo commented 3 years ago

I believe this is a duplicate of issue #4363.

I don't think so. The program here uses Console.Write, which goes to WriteFile or maybe WriteConsole, not to WriteConsoleOutputCharacter or other functions that would take coordinates as parameters. I believe that, when ENABLE_PROCESSED_OUTPUT is set but ENABLE_VIRTUAL_TERMINAL_PROCESSING is not, conhost converts the null character to a space in the screen buffer so that https://github.com/microsoft/terminal/issues/4363 is avoided: https://github.com/microsoft/terminal/blob/0b0161d537006ca397e680ac07a071a0cd70e02d/src/host/_stream.cpp#L487-L490

KalleOlaviNiemitalo commented 3 years ago

Anyway, the behavior of null characters without ENABLE_VIRTUAL_TERMINAL_PROCESSING is compatible with the old Windows console, and the behavior with ENABLE_VIRTUAL_TERMINAL_PROCESSING is compatible with serial terminals, so neither behavior should be changed. Rather, one could think about how to give each application the behavior it expects.

j4james commented 3 years ago

@KalleOlaviNiemitalo Yeah, sorry, you're absolutely right. Disabling the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode does fix the issue.

KalleOlaviNiemitalo commented 3 years ago

one could think about how to give each application the behavior it expects

https://github.com/microsoft/terminal/issues/4954 might help with that.

zadjii-msft commented 3 years ago

Alright so I'm a little confused on what the resolution was here. Is this a dupe of #4363, because conpty is re-rendering the NULs as NULs, and the Terminal is ignoring them? Or is this "You need to disable ENABLE_VIRTUAL_TERMINAL_PROCESSING manually?"

If it's the second, then maybe the difference is because in conhost, you're running the script from cmd, which is going to disable VT output before launching your process. I'm out on a limb here, but I'm guessing that pwsh doesn't, which is why the output seems different in the Terminal.

asklar commented 3 years ago

@zadjii-msft the same problem happens when running the app in cmd.exe inside Terminal. I'm not sure whether this is a dupe of 4363 or not

j4james commented 3 years ago

Sorry for the confusion. It's not a dup of #4368 - that what just me being an idiot. As @KalleOlaviNiemitalo said, when you're not in VT mode (which is typically the default case in the old conhost shell), then most of the low ascii characters are "printable", and NUL shows as a blank. When you are in VT mode (as it the default case in Windows Terminal), then NUL is interpreted as a padding control character which won't be seen.

So if you want your app to work exactly the way it does in the old conhost, then you have to explicitly disable the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode.

I should also clarify that it's not going to make a difference whether you're using cmd or pwsh. Both should start apps with VT mode enabled in Window Terminal, and both should start apps with VT mode disabled when in conhost (at least by default).

asklar commented 3 years ago

So if you want your app to work exactly the way it does in the old conhost, then you have to explicitly disable the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode.

Which is not going to be possible for many legacy apps that need to continue to work properly without being recompiled. There needs to be a quirk or Windows shim for this.

j4james commented 3 years ago

Which is not going to be possible for many legacy apps that need to continue to work properly without being recompiled. There needs to be a quirk or Windows shim for this.

As @KalleOlaviNiemitalo pointed out, #4954 may be able to help with. See #6973 (which was duped to #4954) for a similar requirement.

DHowett commented 3 years ago

Marking this one up for the console backlog -- we need an answer to it, but at the same time we have to be able to move the ecosystem forward. \0 is important to VT applications. We may need a way to tell ConPTY that it should not start in VT enabled mode.

(Sorry for the delay!)