Repository: microsoft/terminal (language: cpp)

Issue:
Keyboard Issues with zsh: Keypad and Other Keys Not Functional.
### Windows Terminal version

1.20.10303.0

### Windows build number

10.0.19045.3930

### Other Software

zsh 5.9 (x86_64-debian-linux-gnu)

### Steps to reproduce

I installed the latest preview version 1.20.10303.0, and now I'm experiencing issues with my keypad and some other keys. I use a French AZERTY keyboard, and the problem only occurs when I'm using zsh; everything works fine on bash for some reason.

### Expected Behavior

_No response_

### Actual Behavior

I have reinstalled Windows Terminal Preview Version 1.19.3172.0, and everything is working perfectly!


Discussion / hints:
Hi.
I've started having this issue today too, and I was able to fix ZSH by adding this in my zshrc, however i still have an Issue with 3 & 9 acting as PgDown & PgUp in `vim` so there's something strange that changed.
```
# Fix numpad not working
# 0 . Enter
bindkey -s "^[Op" "0"
bindkey -s "^[Ol" "."
bindkey -s "^[OM" "^M"
# 1 2 3
bindkey -s "^[Oq" "1"
bindkey -s "^[Or" "2"
bindkey -s "^[Os" "3"
# 4 5 6
bindkey -s "^[Ot" "4"
bindkey -s "^[Ou" "5"
bindkey -s "^[Ov" "6"
# 7 8 9
bindkey -s "^[Ow" "7"
bindkey -s "^[Ox" "8"
bindkey -s "^[Oy" "9"
# + -  * /
bindkey -s "^[Ok" "+"
bindkey -s "^[Om" "-"
bindkey -s "^[Oj" "*"
bindkey -s "^[Oo" "/"
```
@mveroone 
Thank you for your reply. I am aware that this solution can address the issue, but I am still encountering problems with other keys on my keyboard. I am uncertain if there are additional keys affected, as I haven't tested all functionalities, which is not what I expected from Windows Terminal.
Technically the keypad is working as intended. In version 1.20 we added support for [Keypad Application Mode](https://vt100.net/docs/vt510-rm/DECKPAM.html), which enables applications to differentiate between the keys on the numeric keypad and the corresponding numeric keys on the main keyboard. Both zsh and vim ask the terminal to enable that mode.

For zsh, the keypad keys aren't bound to anything by default, which is why they have no effect. For vim, most of the keys are assigned the same character values that you'd normally expect, except `9` and `3` appear to be bound to PageUp and PageDown. I'm not sure why that is - possibly because some of the original hardware terminals didn't have separate PageUp and PageDown keys.

> I am still encountering problems with other keys on my keyboard.

@ab2pentest If those are not keypad keys, please let us know exactly which keys are not working for you, because that's likely a different issue.
So how can we turn support for Keypad Application Mode off, @j4james? I don't see any setting for it in the wt UI and I don't see anything about it in the json settings schema.
@dziban303 Unfortunately it's not possible disable this kind of functionality from within the WT UI. But if the application you're using is built with the _curses_ library, and you don't mind disabling it for all _curses_ applications, then you should be able to fix the issue by modifying your terminfo as described here: https://stackoverflow.com/a/41336431

Otherwise for apps like vim and nano, I'm assuming you can at least rebind the keypad keys to whatever characters you want them to send.

That said, I am looking for a better way for WT to handle this.
I'm sure you'll agree that it's totally unreasonable to make users remap ~17 keys in one or more programs, in every linux instance they use, to accommodate a feature which Google tells me can be disabled in other terminal programs like Putty with a checkbox. 
As I said, I am looking for a better way to handle this, but worst case we can just remove the functionality altogether if nobody wants it.
Maybe I'm looking at the wrong places, but how can I bind the numpad keys to their corresponding characters in nano (i.e. restore the pre-1.20 functionality)? All I could find was how to bind shift-, control- and alt-key sequences, and even those only to specific functions in nano, not to text.

Otherwise yeah, +1 for the option to disable this functionality in an easy way, would be greatly appreciated. I'm sure there was a good reason for implementing it without the option of opting out, but it's been a bit jarring to have the cursor jump around or not do anything when I expected it to write a number after using the numpad keys.
This feature should be rolled back until a way to toggle it is arrived at. A line in the config file is fine, it doesn't need a bespoke UI element right away. And it should probably be disabled by default. Windows Terminal users have made it for years without it. I don't know how many users have been waiting for it to be implemented but I wager it's a smaller pool than that of people who need to use text editors.

@jtabox The easiest workaround is here: https://stackoverflow.com/a/41336431

Of course, this needs to be done on every distros you have installed, as well as all systems you may SSH in to, and it might cause unexpected and unwanted behavior in other programs which use `curses`. I've heard it causes problems with Midnight Commander, for example. It's nuts that we have to do any of this, though.
@dziban303 Yes, I checked that link but I'm a bit hesitant to modify the file it says, since it feels a bit too drastic and I'm really not in a mood of potentially creating more unexpected behaviors that I then have to try and fix. This has already taken 1+ hour until I could figure out why my numpad suddenly isn't working any more, and trying to fix it.

And as you say, I'd have to make this change to 3 local WSL distros and 2 remote ones. I had hoped there was an easy way to fix the binds in nano, same way I could with zsh but it seems there isn't any. For now I've stopped using the Preview version and gone back to version 1.19.

And true, it doesn't need to be an option in Windows Terminal's GUI, just an option in the config file or somewhere would be greatly appreciated.

But thanks for your reply regardless.
I understand, I was hesitant to do it as well. I couldn't figure it how to bind the keypad keys in nano either.
@j4james as other stated, I think the most sensible action at this point would be to revert this change for now until you find a better approach. ~There likely are thousands of affected users here. It would also allow you to have time to look for a good solution without the pressure.~

EDIT : apologies, I understood that this only affects the "preview" version of MS term which as any beta program actually guarantees this kind of issues might happen and must be bore with. I'm reverting to the stable version for now on my side.
@ MS Devs: For the record, I'd be happy for you to pull the `DECKPAM` mappings[^1] if you think that's best. I did look to see what other options we might have, and there is an XTerm mode that we could potentially use to toggle the functionality (mode ?1035), but it's not a simple on/off kind of thing, which makes it a bit of a pain to implement. So personally I don't think it's worth the effort, but we could possibly reconsider that later.

A couple of other points worth noting:

* Technically `DECKPAM` is a VT conformance requirement, but I suspect I'm the only person that cares, and it could eventually be re-enabled via something like `DECSCL`, so I don't think that's a big deal.

* It's needed to correctly map a number of keys in the `xterm-256color` terminfo definition (see `ka1`, `ka3`, `kb2`, `kc1`, `kc3`, and `kent`). That's not something that I care about, but other people might do.

[^1]: I'd suggest we just remove the [ANSI mappings](https://github.com/microsoft/terminal/blob/ef96e225da6b0df496390eed9fe31dc7e434a939/src/terminal/input/terminalInput.cpp#L440-L465), but leave the VT52 ones. We may also need to nuke the `defineNumericKey` helper.
For everyone participating in this thread: Thank you for bearing with us and reporting this issue!

And thank you, @j4james, for fielding all the comments over here and your eternal pursuit of VT conformance. 🙂

For right now, we're thinking that we should disable `DECKPAM` via velocity (keep the code, leave it on in the Dev build so that we can work on it.) I suppose that right now we're almost _too_ compatible.

* Disable `DECKPAM` (ANSI) with velocity; backport this to 1.20
* Make it configurable on the Terminal side
	* Disable it in conhost until we figure out where to store settings for it
	* Invest in the ConPTY configuration API (Leonard keeps terrifyingly calling this ConPTY IOCTL) so that Terminal can _tell the console to enable or disable it_

> EDIT : apologies ... beta program

@mveroone Thank you for adding that :smile:

I'd also like to emphasize the "beta program" part! Thank you all for testing out the Preview channel so we can get some eyes our new stuff before we inflict it on a really mind-boggling amount of people.
(FWIW I'm gonna split this into two threads. This thread, we'll use for tracking disabling it broadly, and #16672 for re-enabling with a toggle)
@j4james I want to apologize if my post above came off as bitter. I didn't mean to disrespect your work, and the fact you're allowing us to use it for free is greatly appreciated.

Fix patch:
diff --git a/src/features.xml b/src/features.xml
index be08ced6932..4a555d7d4e6 100644
--- a/src/features.xml
+++ b/src/features.xml
@@ -177,4 +177,14 @@
         </alwaysEnabledBrandingTokens>
     </feature>
 
+    <feature>
+        <name>Feature_KeypadModeEnabled</name>
+        <description>Enables the DECKPAM, DECKPNM sequences to work as intended </description>
+        <id>16654</id>
+        <stage>AlwaysDisabled</stage>
+        <alwaysEnabledBrandingTokens>
+            <brandingToken>Dev</brandingToken>
+        </alwaysEnabledBrandingTokens>
+    </feature>
+
 </featureStaging>
diff --git a/src/terminal/input/terminalInput.cpp b/src/terminal/input/terminalInput.cpp
index bdd10ca8d63..30ad0d638cd 100644
--- a/src/terminal/input/terminalInput.cpp
+++ b/src/terminal/input/terminalInput.cpp
@@ -441,7 +441,7 @@ try
         // the ASCII character assigned by the keyboard layout, but when set
         // they transmit SS3 escape sequences. When used with a modifier, the
         // modifier is embedded as a parameter value (not standard).
-        if (_inputMode.test(Mode::Keypad))
+        if (Feature_KeypadModeEnabled::IsEnabled() && _inputMode.test(Mode::Keypad))
         {
             defineNumericKey(VK_MULTIPLY, L'j');
             defineNumericKey(VK_ADD, L'k');
@@ -493,7 +493,7 @@ try
 
         // Keypad keys also depend on Keypad mode, the same as ANSI mappings,
         // but the sequences use an ESC ? prefix instead of SS3.
-        if (_inputMode.test(Mode::Keypad))
+        if (Feature_KeypadModeEnabled::IsEnabled() && _inputMode.test(Mode::Keypad))
         {
             defineKeyWithUnusedModifiers(VK_MULTIPLY, L"\033?j"s);
             defineKeyWithUnusedModifiers(VK_ADD, L"\033?k"s);


Test patch: