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

TerminalControl does not produce ENHANCED_KEY in win32 input mode #7074

Closed
krontzo opened this issue Jul 25, 2020 · 12 comments · Fixed by #7106
Closed

TerminalControl does not produce ENHANCED_KEY in win32 input mode #7074

krontzo opened this issue Jul 25, 2020 · 12 comments · Fixed by #7106
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@krontzo
Copy link

krontzo commented Jul 25, 2020

Environment

Platform ServicePack Version      VersionString
-------- ----------- -------      -------------
 Win32NT             10.0.19041.0 Microsoft Windows NT 10.0.19041.0

Windows Terminal
Version: 1.1.2021.0

mpv 0.32.0-572-g8fbc4b1737

Steps to reproduce

  1. Play an audio only file as (i.e. only TUI, not GUI):
mpv *.mp3
  1. Press arrow keys to control playback
LEFT and RIGHT
    Seek backward/forward 5 seconds. Shift+arrow does a 1 second exact seek (see --hr-seek).
UP and DOWN
    Seek forward/backward 1 minute. Shift+arrow does a 5 second exact seek (see --hr-seek).

Expected behavior

With previous Windows Terminal versions, the current position was modified as expected.

Actual behavior

LEFT, RIGHT, UP, DOWN keys not sending correct key codes:

[input] No key binding found for key 'KP6'.
A: 00:00:09 / 00:49:16 (0%)
[input] No key binding found for key 'KP4'.
A: 00:00:10 / 00:49:16 (0%)
[input] No key binding found for key 'KP8'.
A: 00:00:10 / 00:49:16 (0%)
[input] No key binding found for key 'KP2'.
A: 00:00:16 / 00:49:16 (1%)

If using powershell 7 without Windows Terminal, the arrow keys work ok. And as a reminder, it was working as expected in previous versions of Windows Terminal. Also works as expected in other shells (cmd.exe, Git Bash) without Windows Terminal.

PS: nvim and vim do not have this issue. Looking for other TUI apps to verify their behavior.

@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 Jul 25, 2020
@krontzo
Copy link
Author

krontzo commented Jul 25, 2020

I just tried alacritty, version 0.4.3, and now I has the same problem; but I didn't update it. With alacritty, it it possible to choose between conpty and winpty. With winpty, mpv works as expected.

I guess that conpty got updated with WT, and it produces the new key codes for mpv .

A workaround for mpv is to create an input.conf and map the new key codes that conpty assigns to arrow keys.

@DHowett
Copy link
Member

DHowett commented Jul 25, 2020

KP6/4/8/2 is interesting. Those are not the keys in the traditional T-cluster of arrow keys. Which arrow keys are you pressing? Do you have a number pad? Is numlock enabled?

@DHowett
Copy link
Member

DHowett commented Jul 25, 2020

Just for the record, WT cannot update the version of conpty that comes with Windows. Alacritty is using a version of conpty that was finished almost a year ago. It's missing a bunch of our input changes.

As a workaround, can you see what happens if you put the following setting at the root of your settings file?

"experimental.input.forceVT": true,

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 25, 2020
@krontzo
Copy link
Author

krontzo commented Jul 25, 2020

KP6/4/8/2 is interesting. Those are not the keys in the traditional T-cluster of arrow keys. Which arrow keys are you pressing? Do you have a number pad? Is numlock enabled?

It's a laptop keyboard. It doesn't have a dedicated number pad. Numlock is not enabled.
The keys are the T-cluster of arrow keys.

Just for the record, WT cannot update the version of conpty that comes with Windows. Alacritty is using a version of conpty that was finished almost a year ago. It's missing a bunch of our input changes.

As a workaround, can you see what happens if you put the following setting at the root of your settings file?

"experimental.input.forceVT": true,

I just installed Windows Terminal v1.0.1811.0 and it works (as it used to).

Reinstalling alacritty to test.

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 25, 2020
@krontzo
Copy link
Author

krontzo commented Jul 25, 2020

I didn't know that conpty was not updated by WT. Sorry about making that assumption.
I just tested alacritty 0.4.3 and it has the problem, but now I'm not so sure that it didn't have that issue before. I used WT fulltime.

Is it possible to query my system to know conpty's current version and the last time it was updated?
But since WT v1.0.1811.0 does not have the issue, I am clueless about the root of the problem:

Terminal/shell Has the issue? PTY
WT 1.0.18.11.0/any No ConPTY
WT 1.1.2021.0/any Yes ConPTY
alacritty 0.4.3/PS7 Yes ConPTY
alacritty 0.4.3/PS7 No WinPTY
NA / PS7 No NA
NA / WPS No NA
NA / cmd No NA
NA / Git Bash No NA

I'll reinstall WT 1.1.2021.0 to try the suggested setting:

"experimental.input.forceVT": true,

@krontzo
Copy link
Author

krontzo commented Jul 25, 2020

WT 1.1.2021.0 works as expected with the setting.

@DHowett
Copy link
Member

DHowett commented Jul 27, 2020

I'm also quite surprised that alacritty has this problem with ConPTY. We made some input changes (#4999) between Terminal 1.0 and 1.1 that are not available to alacritty yet.

You're using ConPTY 10.0.19041.0 (same as the version of Windows you're using.)

Is this a Windows build of mpv? Can you point me to where you got it?

If you want to help debug what is happening, can you do the following?

  • Set the following settings:
"debugFeatures": true,
"experimental.input.forceVT": false,
  • Open a new tab in Terminal while holding down Left Alt and Right Alt
    • When you do this, you should get two panes, one of which contains weird garbage. Like this:

image

  • Press the arrow keys. This should generate red text on the right pane.
  • Send a screenshot of the garbage pane.

@DHowett DHowett added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Attention The core contributors need to come back around and look at this ASAP. labels Jul 27, 2020
@krontzo
Copy link
Author

krontzo commented Jul 28, 2020

Is this a Windows build of mpv? Can you point me to where you got it?

Yes, it is. From sourceforge: https://sourceforge.net/projects/mpv-player-windows/files

FYI and to help other mpv users of WT: mpv-player/mpv#7944

Here you are:

debug_wt_20200727T2102

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jul 28, 2020
@avih
Copy link

avih commented Jul 28, 2020

To demonstrate some difference on how applications recieve arrows input, I wrote this program:

#include <stdio.h>
#include <conio.h>

int main(int argc, char **argv)
{
    int c;
    while ((c = getch()) != '\033' /* ESC */)
        printf("'%c'  0x%02x\n", c < 32 ? '?' : c, c);

    return 0;
}

In windows 10 2004, in normal windows console (cmd.exe), pressing the left arrow (at the inverted T arrows cluster) produces:

'α'  0xe0
'K'  0x4b

While in Windows Terminal 1.1.200629002-release1.1 (command prompt - not powershell) it produces:

'?'  0x00
'K'  0x4b

I'm not very familiar with scancodes (which getch() returns), but some googling suggests that the pair 0xe0 0x4b represents left at the main arrows, while 0x00 0x4b represents left at the keypad.

Caveats:

  • The program was compiled using x86_64-w64-mingw32-gcc (GCC) 9.2.0 (from Cygwin).
  • Windows Terminal was not installed properly - the installer was extracted and WindowsTerminal.exe was executed at the extracted folder (inside CascadiaPackage_1.1.1812.0_x64.msix).
  • mpv uses ReadConsoleInputW and not getch(). mpv does set ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT as well as ENABLE_VIRTUAL_TERMINAL_PROCESSING where supported (e.g. on my system), but it does not set ENABLE_VIRTUAL_TERMINAL_INPUT.

Nevertheless, it does seem to suggest that there's some difference between windows console and windows terminal in how non-VT-input applications recieve arrow keypresses, and additionally with my setup I was able to reproduce the OP's issue when pressing the left arrow - mpv perceiving KP4 at the windows terminal, and LEFT at the windows console.

@DHowett DHowett changed the title Version 1.1.2021.0 new regression: arrow keys replaced codes in mpv TerminalControl does not produce ENHANCED_KEY in win32 input mode Jul 28, 2020
@DHowett
Copy link
Member

DHowett commented Jul 28, 2020

Thanks so much. This has been really helpful.

This is the Terminal equivalent of #2397. There's a specific encoding bit that we were missing from the standard arrow keys.

@DHowett DHowett added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. and removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 28, 2020
@DHowett DHowett added this to the Terminal v1.3 milestone Jul 28, 2020
DHowett added a commit that referenced this issue Jul 28, 2020
While I was working on the WPF control, I took a moment to refactor the
message cracking out into a helper. It's a lot easier on the eyes than
four lines of bit shifting repeated three times.

Fixes #7074.
@ghost ghost added the In-PR This issue has a related PR label Jul 28, 2020
@ghost ghost closed this as completed in #7106 Jul 31, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 31, 2020
ghost pushed a commit that referenced this issue Jul 31, 2020
When we added support for win32 input mode, we neglected to pass
`ENHANCED_KEY` through the two surfaces that would generate events. This
broke arrow keys in much the same way was #2397, but in a different
layer.

While I was working on the WPF control, I took a moment to refactor the
message cracking out into a helper. It's a lot easier on the eyes than
four lines of bit shifting repeated three times.

Fixes #7074
DHowett added a commit that referenced this issue Jul 31, 2020
When we added support for win32 input mode, we neglected to pass
`ENHANCED_KEY` through the two surfaces that would generate events. This
broke arrow keys in much the same way was #2397, but in a different
layer.

While I was working on the WPF control, I took a moment to refactor the
message cracking out into a helper. It's a lot easier on the eyes than
four lines of bit shifting repeated three times.

Fixes #7074

(cherry picked from commit dd0f7b7)
DHowett added a commit that referenced this issue Jul 31, 2020
When we added support for win32 input mode, we neglected to pass
`ENHANCED_KEY` through the two surfaces that would generate events. This
broke arrow keys in much the same way was #2397, but in a different
layer.

While I was working on the WPF control, I took a moment to refactor the
message cracking out into a helper. It's a lot easier on the eyes than
four lines of bit shifting repeated three times.

Fixes #7074

(cherry picked from commit dd0f7b7)
@ghost
Copy link

ghost commented Aug 13, 2020

🎉This issue was addressed in #7106, which has now been successfully released as Windows Terminal v1.1.2233.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Aug 13, 2020

🎉This issue was addressed in #7106, which has now been successfully released as Windows Terminal Preview v1.2.2234.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants