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

Ignore Unknown keys #192

Merged
merged 1 commit into from Jan 14, 2023

Conversation

deadalusai
Copy link
Contributor

@deadalusai deadalusai commented May 7, 2022

I found this issue when pressing the Ctrl key while using Input, which causes it to return Err(Not a terminal) under windows (but works fine on Linux). In this case, I was in the process of pressing Ctrl + C in order to terminate the program.

There are a few keystrokes which cause console-rs to produce Unknown keys when running under Windows. At the time of this commit this includes pressing the Control key, but also previously included Shift and Alt.

Pressing these keys under a *nix terminal does not appear to produce a "key" at all - rather the user must press a complete key chord before Term::read_key() will produce a result.

Looking at the console-rs source, from what I can see that it produces an Unknown key under Windows in only two cases:

  1. When the console is not "attended": https://github.com/console-rs/console/blob/master/src/term.rs#L274-L276
  2. When the key code does not map to a known key in the Key enum: https://github.com/console-rs/console/blob/master/src/windows_term.rs#L337

Since dialoguer is already checking for non-attended consoles and executing an early return we can rule out needing to handle that scenario:
https://github.com/mitsuhiko/dialoguer/blob/f7d02321f5eda89c15b76ffbefbaebdd2c9f0bcb/src/prompts/input.rs#L276-L278

For the second scenario, it seems reasonable to ignore any "key" not recognized by console-rs for the purposes of the Input component.

There are a few keystrokes which cause `console-rs` to produce `Unknown`
keys. At the time of this commit this includes pressing the `Control`
key, but also previously included `Shift` and `Alt`.

Pressing these keys under a *nix terminal does not appear to produce a
"key" at all - rather the user must press a complete key chord before
`Term::read_key()` will produce a result.

Looking at the `console-rs` source, we can see that it produces an
`Unknown` key under Windows in at least two cases:

1. When the console is not "attended"
2. When the key code does not map to a known key in the Key enum

Since `dialoguer` is already checking for non-attended consoles and
executing an early return we can rule out needing to handle that
scenario. For the second scenario, it seems reasonable to ignore any "key"
not recognized by `console-rs` for the purposes of the Input component.
@deadalusai
Copy link
Contributor Author

Related PR on console: console-rs/console#124

@ejpcmac
Copy link

ejpcmac commented Aug 23, 2022

Pressing AltGr also triggers this behaviour on Windows, which can be more preblematic to enter special characters.

@kotx
Copy link

kotx commented Dec 15, 2022

This also includes Caps Lock (as mentioned in the issues above). Any chance this could be merged?

@mitsuhiko mitsuhiko merged commit d35b5de into console-rs:master Jan 14, 2023
connor4312 added a commit to microsoft/vscode that referenced this pull request Mar 24, 2023
connor4312 added a commit to microsoft/vscode that referenced this pull request Mar 24, 2023
* cli: fix tunnel message command

Fixes #177394

* fix: windows terminal erroring on tunnel first run

Was fixed by console-rs/dialoguer#192

Fixes #175747
qwq0 pushed a commit to qwq0/vscode that referenced this pull request Mar 25, 2023
)

* cli: fix tunnel message command

Fixes microsoft#177394

* fix: windows terminal erroring on tunnel first run

Was fixed by console-rs/dialoguer#192

Fixes microsoft#175747
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants