Fix '@' appearing in console #1092
Conversation
rjmholt
requested review from
TylerLeonhardt,
SeeminglyScience and
daxian-dbw
| @@ -188,7 +188,7 @@ internal static ConsoleKeyInfo SafeReadKey(bool intercept, CancellationToken can | |||
| } | |||
| catch (OperationCanceledException) | |||
| { | |||
| return default(ConsoleKeyInfo); | |||
| return new ConsoleKeyInfo(' ', ConsoleKey.DownArrow, shift: false, alt: false, control: false); | |||
There was a problem hiding this comment.
Why ConsoleKey.DownArrow instead of ConsoleKey.Spacebar?
There was a problem hiding this comment.
It doesn't add any output to the buffer, and seemed to work without consequences when I tried it, which seems logical since inputting DownArrow in PSRL would do nothing if you're at the last prompt already.
There was a problem hiding this comment.
Nit: name the ' ' parameter.
There was a problem hiding this comment.
Does it trigger custom key handlers? Assuming it does, is there some character we could send that isn't typically possible to receive as input that we could specifically handle on the PSRL side?
There was a problem hiding this comment.
Valid values are here: https://docs.microsoft.com/en-us/dotnet/api/system.consolekey?view=netstandard-2.0
Could possibly use a numeric value that isn't in the enum, but that could be fragile.
There was a problem hiding this comment.
For some more options, we could do something similar to how I'm currently getting around lack of SHIFT + ENTER support (e.g. sending an emoji as raw input)
For instance, if you send 0x2665 to xterm directly as a raw text sequence it comes across as this:
KeyChar Key Modifiers
------- --- ---------
♥ 18 0
Note that 18 is VK_MENU aka ALT. Not 100% sure why it comes across that way, the raw input is just (Apparently it comes through as a ALT + NumPad unicode sequence).\u2665.
It is still possible to conflict with an existing key bind that way, but significantly less likely.
There was a problem hiding this comment.
This does mean we'll need to create another contract with PSRL to throw this character out if it is seen. That makes me uneasy... Seems very hacky.
@daxian-dbw would you mind giving your point of view?
There was a problem hiding this comment.
This does mean we'll need to create another contract with PSRL to throw this character out if it is seen. That makes me uneasy... Seems very hacky.
Is it really all that more hacky than sending a valid key that we hope no one uses? To be clear, I'm not saying it isn't hacky. Everything around our ReadKey is incredibly hacky and will continue to be until the day ReadKeyAsync is properly implemented in corefx (a man can dream right?). This variant of hacky is a lot less likely to spawn other issues, definitely open to better ideas though.
There was a problem hiding this comment.
The other possibility I had was an unmodified Space. That one seems safe, but it does add a space to the console.
The only actual safe thing to do is to redefine our contract with PSRL so that we can say "we didn't get anything at all". Currently that @ character represents PSRL not understanding the key press.
Anyway, I'm not proposing that this change be permanent. Just something that works better for now.
There was a problem hiding this comment.
Anyway, I'm not proposing that this change be permanent. Just something that works better for now.
Yeah fair enough. Probably fine for preview.
Changes the key we return to PSRL when the prompt is cancelled to a down arrow.
This resolves PowerShell/vscode-powershell#2274.