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

Strange behavior with ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT #4949

Open
alexrp opened this issue Mar 16, 2020 · 23 comments
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Milestone

Comments

@alexrp
Copy link

alexrp commented Mar 16, 2020

Environment

Windows build number: Microsoft Windows [Version 10.0.19041.153]
PowerShell Core 7.0.0
.NET Core 3.1.200

Steps to reproduce

C# program:

using System;
using System.Diagnostics;
using System.Text;
using static Vanara.PInvoke.Kernel32;

namespace Test
{
    static class Program
    {
        static void Main()
        {
            var stdin = GetStdHandle(StdHandleType.STD_INPUT_HANDLE);

            Debug.Assert(GetConsoleMode(stdin, out CONSOLE_INPUT_MODE inMode));

            inMode |= CONSOLE_INPUT_MODE.ENABLE_VIRTUAL_TERMINAL_INPUT;

            Debug.Assert(SetConsoleMode(stdin, inMode));

            Console.WriteLine(Console.ReadLine());
        }
    }
}

(Uses the Vanara.PInvoke.Kernel32 NuGet package for convenience.)

You will need to test this program in both CMD and PowerShell Core.

Try doing these:

  1. Type aaa bbb ccc and press backspace a couple times.
  2. Type aaa bbb ccc and press control + backspace several times.

Also, try using special keys (such as arrow keys) after the program closes.

Actual behavior

For the first part:

  1. Words are deleted with each backspace press.
  2. Characters are deleted with each control + backspace press.

This behavior happens in both CMD and PowerShell Core.

For the second part:

After closing the program, you'll notice that e.g. using the arrow keys echoes VT sequences (such as ^[[D^[[A^[[C for left, up, right) rather than doing what they're supposed to. This behavior persists even if the ReadLine + WriteLine is removed from the program.

This behavior only happens in CMD. PowerShell Core seems fine.

Expected behavior

I have no idea to what extent, if any, this behavior is actually buggy/unintended. But here is what I would expect as a user:

  • Backspace and control + backspace behavior should not be swapped like this. This seems very strange and unlike other terminals.
  • Console modes set from within a program should not persist after that program exits. PowerShell Core appears to get this right, while CMD does not, for whatever reason.

Perhaps worth noting that running CMD and PowerShell Core under Windows Terminal has no impact on the behavior seen here.

Also, I wasn't sure whether to file this as two separate issues. I can do that if necessary.

@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 Mar 16, 2020
@DHowett-MSFT
Copy link
Contributor

Thanks for doing all the legwork here! This is, unfortunately, to a fairly great extent "by design." The input and output modes are global state, and while we're thinking about what it would take to change that we're definitely concerned about backwards compatibility.

When an application exits and leaves the console in a state different from the one it has when it launched (which can happen for a number of reasons: it wasn't authored correctly, it crashes, it returns control of the console so the shell before it's ready (#4921)), we have little choice but to get into a weird state.

You see this with most terminals on the market, unfortunately. If vim is running in mouse mode, and is abruptly terminated, suddenly moving your mouse over your terminal generates an absolute heap of garbage input. It's impossible to solve in that case, but given that we own the console subsystem in its entirety we can put together a better story.

I'm going to close this one as a /dupe of #4954, which we'll use to track the overall scenario.

@ghost
Copy link

ghost commented Mar 17, 2020

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost closed this as completed Mar 17, 2020
@ghost ghost added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed 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 Mar 17, 2020
@alexrp
Copy link
Author

alexrp commented Mar 17, 2020

Thanks for the response @DHowett-MSFT!

I just wanted to clarify a few things:

For the mode issue, my understanding (after reading the linked issues) is that the reason my particular sample here works properly in PowerShell is that PowerShell actually makes an effort to set a sensible console mode before/after running a program, while CMD does not. Is that correct?

If so, then I suppose the best I can do right now is try really hard to set the console mode back to its original state, even in the case of a crash, to make things work under CMD?

Regarding the backspace issue I also brought up here, as far as I can tell, this isn't explained by the linked issues. To be clear, this issue happens even if you run the sample program from a 'clean' console.

@DHowett-MSFT
Copy link
Contributor

while CMD does not

Exactly this.

The backspace issue is an intricacy of how backspace is encoded in ENABLE_VIRTUAL_TERMINAL_INPUT mode, actually. When an application requests VT input, backspace is translated as 0x7f (DEL) instead of 0x8 (BS). Ctrl+Backspace is, then, translated as 0x8 (BS) instead of 0x7f (DEL). It's simply a matter of coincidence that they're flipped, but COOKED_READ then can't tell if the user actually pressed ctrl+backspace or it's only receiving 0x7f because it's in VT input mode.

@DHowett-MSFT
Copy link
Contributor

(because COOKED_READ isn't supposed to be used in VT input mode)

@alexrp
Copy link
Author

alexrp commented Mar 17, 2020

What does COOKED_READ refer to here?

@DHowett-MSFT
Copy link
Contributor

Ah, sorry. The input mode that CMD and legacy versions of PowerShell (without PSReadline) use. It's the mode there the console host handles producing a single line of input for the client application and only returns it when the user mashes Enter.

@DHowett-MSFT
Copy link
Contributor

It's the opposite of "raw" input 😉

@alexrp
Copy link
Author

alexrp commented Mar 17, 2020

OK, so to be clear, is the implication here that ENABLE_VIRTUAL_TERMINAL_INPUT in and of itself effectively means "raw mode"? In other words, that I'm pretty much expected/required to unset ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT at the same time as I set the former (because otherwise things just don't work properly)?

@DHowett-MSFT
Copy link
Contributor

It would be ideal if you do so, yeah -- if all you want is actual VT input, unmanipulated by the console host, you're going to want to disable line and processed input.

Most VT input consumers are doing their own line editing and want to get hold of the arrow keys and other keys germane to line editing, or they want to get mouse input in a cross-platform way, and having the processed or line input modes enabled will make it difficult to do that.

@alexrp
Copy link
Author

alexrp commented Mar 17, 2020

It would be ideal if you do so, yeah -- if all you want is actual VT input, unmanipulated by the console host, you're going to want to disable line and processed input.

But the thing is, that's not what I want. 🙂 For this project I'm working on, I'm trying to expose a consistent terminal interaction API that works the same way on Windows and Unix systems, and this issue touches on a pretty fundamental aspect of that.

I'm attempting to get the same behavior on Windows that you get in the Unix world: VT keyboard input at all times, with cooked mode by default, and raw mode optionally available via tcsetattr/termios. That's why I'm enabling ENABLE_VIRTUAL_TERMINAL_INPUT but leaving the other flags enabled, only disabling them when the user of the library explicitly requests raw mode.

Is there nothing that can be done to make the Windows console host handle this situation properly?

@DHowett-MSFT
Copy link
Contributor

Sorry, I misunderstood your use case. I'll try to break down a couple of the scenarios and the flags that'll light them up. Would you let me know if I've captured the ones you're looking for?

  • If you want completely raw input, with VT sequences, as they are generated by a terminal:
    • use ENABLE_VIRTUAL_TERMINAL_INPUT, disable ENABLE_LINE_INPUT, ENABLE_ECHO_INPUT, ENABLE_PROCESSED_INPUT
    • Try to always use ReadConsoleInput, as that's the only way to be notified of window size changes (we have a workitem tracking a real SIGWINCH-like signal)
  • If you want input that is broken up by line...
    • ... containing VT and control characters (^C, ^H, ^M):
      • use ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_LINE_INPUT, disable ENABLE_PROCESSED_INPUT, your choice for echo
      • lines will be delimited by ^M
    • ... containing VT, but no control characters (I think this is the Unix TTY vt-cooked mode?):
      • use ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT, your choice for echo
      • lines will be delimited by ^M ^J (the Windows way)
      • you will never receive ^H in your buffer
    • ... with Console-mediated line editing, but no VT:
      • use ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT
      • lines will be delimited by ^M ^J (the Windows way)
      • you will never receive ^H or ^[[A in your buffer

@DHowett-MSFT
Copy link
Contributor

Okay, now I get it. Item 2 sub 3, the one I think is vt-cooked mode, has the wrong behavior for backspace; this seems to be true back a couple major releases of windows. That's not excellent.

I'll check this out versus the version that introduced ENABLE_VIRTUAL_TERMINAL_INPUT.

Sorry, I had to talk myself through your problem. 😄

@alexrp
Copy link
Author

alexrp commented Mar 17, 2020

Maybe it's easier to explain with this C program:

#include <unistd.h>

int main()
{
    char buf[4096];

    write(STDOUT_FILENO, "reading: ", 9);
    ssize_t len = read(STDIN_FILENO, buf, sizeof(buf));
    write(STDOUT_FILENO, buf, len);

    return 0;
}

Run on a Linux system (or WSL) with gcc test.c && ./a.out.

You will notice that you are able to do fairly basic line editing while the program reads from stdin, i.e. the TTY is in cooked mode. The exact level of line editing available will probably depend on your terminal and termios settings (the latter of which is usually the shell's responsibility). In my case, I'm able to use Backspace, but not arrow keys.

But, you will also notice that if you type some special keys, they are picked up as VT sequences by the program. For example, aaa followed by Ctrl+X and Enter prints aaa� for me.

This is the behavior I'm trying to achieve.

To be clear, though, I don't particularly care what amount of editing features is available in cooked mode. It's okay that those are different between terminals and platforms. The important thing here is achieving the cooked + VT behavior, but not breaking Backspace / Ctrl+Backspace behavior on Windows in the process.

@DHowett-MSFT DHowett-MSFT changed the title Strange behavior with ENABLE_VIRTUAL_TERMINAL_INPUT in CMD/PS Strange behavior with ENABLE_VIRTUAL_TERMINAL_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT Mar 17, 2020
@DHowett-MSFT DHowett-MSFT removed the Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. label Mar 17, 2020
@DHowett-MSFT DHowett-MSFT reopened this Mar 17, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Mar 17, 2020
@DHowett-MSFT DHowett-MSFT added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase labels Mar 17, 2020
@xoofx
Copy link

xoofx commented Dec 30, 2021

@DHowett-MSFT This issue is quite annoying as it is making ENABLE_VIRTUAL_TERMINAL_INPUT quite useless when in cooked mode.

I have tried to investigate this and the end story is that the key is mapped by this table here for some reasons that I don't know:

TermKeyMap{ VK_BACK, L"\x7f" },

I could make a PR and remove this line, but I'm not sure about the consequences. Thoughts?

@xoofx
Copy link

xoofx commented Dec 30, 2021

I tested by modifying these two places in src\terminal\input\terminalInput.cpp:

static constexpr std::array<TermKeyMap, 20> s_keypadNumericMapping{
TermKeyMap{ VK_TAB, L"\x09" },
TermKeyMap{ VK_BACK, L"\x7f" },
TermKeyMap{ VK_PAUSE, L"\x1a" },

static constexpr std::array<TermKeyMap, 19> s_keypadNumericMapping{
    TermKeyMap{ VK_TAB, L"\x09" },
-    TermKeyMap{ VK_BACK, L"\x7f" }, 
    TermKeyMap{ VK_PAUSE, L"\x1a" },

static constexpr std::array<TermKeyMap, 14> s_simpleModifiedKeyMapping{
TermKeyMap{ VK_BACK, CTRL_PRESSED, L"\x8" },
TermKeyMap{ VK_BACK, ALT_PRESSED, L"\x1b\x7f" },
TermKeyMap{ VK_BACK, CTRL_PRESSED | ALT_PRESSED, L"\x1b\x8" },

static constexpr std::array<TermKeyMap, 14> s_simpleModifiedKeyMapping{
-    TermKeyMap{ VK_BACK, CTRL_PRESSED, L"\x8" },
-    TermKeyMap{ VK_BACK, ALT_PRESSED, L"\x1b\x7f" },
-    TermKeyMap{ VK_BACK, CTRL_PRESSED | ALT_PRESSED, L"\x1b\x8" },
+    TermKeyMap{ VK_BACK, CTRL_PRESSED, L"\x7f"},
+    TermKeyMap{ VK_BACK, ALT_PRESSED, L"\x1b\x8" },
+    TermKeyMap{ VK_BACK, CTRL_PRESSED | ALT_PRESSED, L"\x1b\x7f" },

And the terminal seems to behave correctly with or without ENABLE_VIRTUAL_TERMINAL_INPUT

There are a few other places in the file where VK_BACK is mapped to x7f, so not sure If I should change these as well.

@alexrp
Copy link
Author

alexrp commented Dec 31, 2021

@xoofx I think this might be relevant context: #5957 (comment)

@xoofx
Copy link

xoofx commented Dec 31, 2021

wow, ok, thanks for the link, what a mess 😅

So basically, the official terminal mapping is:

  • CTRL-H is usually mapped to \x8, e.g deletes a char on the left in a prompt shell
  • Backspace is usually mapped to \x7f, e.g deletes a char on the left in a prompt shell
  • CTRL+Backspace maps to \x8
  • CTRL+W maps to \x17 so Backword

While in powershell/cmd on Windows to, it is interpreted as:

  • CTRL-H = \x8
  • Backspace = \x8
  • CTRL+Backspace = \x7f (so Backword)
  • CTRL+W = \x17 (but not Backword)

And the issue here is that, when ENABLE_VIRTUAL_TERMINAL_INPUT we use the official terminal mapping, but with the Windows Terminal interpretation:

  • CTRL-H = \x8
  • Backspace = \x7f as Backword
  • CTRL+Backspace = \x8

But it's incompatible with vt/unix terminals and CTRL+Backspace is anyway mapped to \x8... 🤦

Could Windows Terminal interpret \x17as the actual Backword instead? and so \x8 and \x7f would both be interpreted as backspaces, and the terminal would choose to map CTRL+Backspace to \x17 (instead of \x8)

[Edit]I have edited multiple times this message to get my head around the problem, and I will end it as I started, what a mess 😞 )

@xoofx
Copy link

xoofx commented Dec 31, 2021

So thinking more about it, in the presence of ENABLE_VIRTUAL_TERMINAL_INPUT, I would propose that wt:

  • Keeps CTRL-H = \x8
  • Keeps BackSpace = \x7f but interpret it as real backspace instead of transforming it to a backword
  • Could translate CTRL+Backspace to \x17 (so CTRL+W, Backword). If not keep CTRL+Backspace = \x8. I don't know how much Unix folks cares about CTRL+Backspace mapping to \x8.

@xoofx
Copy link

xoofx commented Dec 31, 2021

Tested this fix proposal for readDataCooked.cpp by having both \x8 and \x7f deleting previous space when ENABLE_VIRTUAL_TERMINAL_INPUT is setup:

diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp
index 169b0973..c33e6f2f 100644
--- a/src/host/readDataCooked.cpp
+++ b/src/host/readDataCooked.cpp
@@ -503,6 +503,8 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
         return true;
     }
 
+    bool isInVirtualTerminalInputMode = _pInputBuffer->IsInVirtualTerminalInputMode();
+
     if (wch == EXTKEY_ERASE_PREV_WORD)
     {
         wch = UNICODE_BACKSPACE;
@@ -558,7 +560,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
                     _currentPosition -= 1;
 
                     // Repeat until it hits the word boundary
-                    if (wchOrig == EXTKEY_ERASE_PREV_WORD &&
+                    if (!isInVirtualTerminalInputMode && wchOrig == EXTKEY_ERASE_PREV_WORD &&
                         _bufPtr != _backupLimit &&
                         fStartFromDelim ^ !IsWordDelim(_bufPtr[-1]))
                     {
@@ -636,7 +638,7 @@ bool COOKED_READ_DATA::ProcessInput(const wchar_t wchOrig,
                     NumSpaces = 0;
 
                     // Repeat until it hits the word boundary
-                    if (wchOrig == EXTKEY_ERASE_PREV_WORD &&
+                    if (!isInVirtualTerminalInputMode && wchOrig == EXTKEY_ERASE_PREV_WORD &&
                         _bufPtr != _backupLimit &&
                         fStartFromDelim ^ !IsWordDelim(_bufPtr[-1]))
                     {

@zadjii-msft zadjii-msft modified the milestones: Console Backlog, Backlog Jan 4, 2022
@zadjii-msft zadjii-msft added the Area-CookedRead The cmd.exe COOKED_READ handling label Feb 23, 2022
@alexrp
Copy link
Author

alexrp commented Feb 15, 2023

Hi folks, any thoughts on @xoofx's comments above?

@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Feb 15, 2023
@zadjii-msft
Copy link
Member

short notes I'll fix up tomorrow morning: the fullest solution is to do translation at read time not insert time, but that's Hard to impossible. These modes together will probably always be a little fucky, but they could be less fucky. Proposed fix seems close, but is just all backspace. We need to flip the meaning of DEL and BS in this mode, not just always do the backspace thing

@carlos-zamora carlos-zamora removed the Needs-Discussion Something that requires a team discussion before we can proceed label Feb 22, 2023
@zadjii-msft
Copy link
Member

"tomorrow morning" lmao sorry for the delay

  • The problem is that we translate INPUT_RECORDs into their VT equivalent at the time they are written into the Input Buffer. The most correct solution would be to do taht translation at the time the events are read, but that's basically impossible. What if the caller tried to only read one event, but the VT translation would convert it to multiple events. What happens then? The buffer isn't quite set up to handle that kind of scenario.
  • These modes will always be weird - consider, you're in VT Input + cooked read, and you press the arrow keys. The events that get read are all VT encoded, like ^[[A! Yikes!
  • Even if this mode is fucky, it could sensibly be less fucky. The fix above is really close, but that seem slike it always results in both DEL and BS being treated as backspace. We should make sure that DEL gets treated as "backspace" and BS gets treated as "delete", as usual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CookedRead The cmd.exe COOKED_READ handling Area-Input Related to input processing (key presses, mouse, etc.) Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase
Projects
None yet
Development

No branches or pull requests

6 participants