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

Cannot register key handlers for the same key with different modifiers #3555

Open
3 tasks done
akbyrd opened this issue Jan 29, 2023 · 9 comments
Open
3 tasks done

Cannot register key handlers for the same key with different modifiers #3555

akbyrd opened this issue Jan 29, 2023 · 9 comments
Labels
Area-KeyHandlers Label for issues related to key handlers Issue-Bug It either shouldn't be doing this or needs an investigation.

Comments

@akbyrd
Copy link

akbyrd commented Jan 29, 2023

Prerequisites

  • Write a descriptive title.
  • Make sure you are able to repro it on the latest released version
  • Search the existing issues, especially the pinned issues.

Exception report

N/A

Screenshot

image

Environment data

PS Version: 7.3.2
PS HostName: ConsoleHost (Windows Terminal)
PSReadLine Version: 2.2.6
PSReadLine EditMode: Windows
OS: 10.0.19041.1 (WinBuild.160101.0800)
BufferWidth: 178
BufferHeight: 56

Steps to reproduce

Paste the following text:

Function Inject-Command([String] $command) {
	[Microsoft.PowerShell.PSConsoleReadLine]::RevertLine()
	[Microsoft.PowerShell.PSConsoleReadLine]::Insert($command)
	[Microsoft.PowerShell.PSConsoleReadLine]::AcceptLine()
}

Set-PSReadLineKeyHandler -Chord "Ctrl+," -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Alt+," -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Shift+," -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Ctrl+Alt+." -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Ctrl+Shift+," -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Alt+Shift+," -ScriptBlock { Inject-Command("echo success") }
Set-PSReadLineKeyHandler -Chord "Ctrl+Alt+Shift+," -ScriptBlock { Inject-Command("echo success") }
  • Notice the first 4 handlers are able to be registered
  • Notice the remaining 3 handlers are not able to be registered
  • Type ,
  • Notice , is replaced with echo success (this handler was never registered)

Expected behavior

  • Each key handler should be able to be registered
  • Unmodified , should not have a handler registered

Actual behavior

  • Each key handler is not able to be able to be registered
  • Unmodified , has a handler registered
@ghost ghost added the Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. label Jan 29, 2023
@andyleejordan
Copy link
Member

The vast majority of these key combinations aren't even able to be received by PSReadLine, just on Windows (and experimenting on macOS/Linux is going to reveal this is even more complicated), try [Console]::ReadKey() and then execute those chords, and see what happens:

  • Ctrl+, is intercepted by the OS and opens up the Terminal app's settings
  • Alt-, ought to work, it comes through as Alt-,
  • Shift-, might work, assuming in PSReadLine we understand to translate what comes through (Shift-<) as equivalent
  • Ctrl+Alt+. is weird, it reports control and alt modifiers with OemPeriod key, but no KeyChar, so PSReadLine is presumably confused by that
  • Ctrl+Shift+, is again intercepted by the OS, no idea why but Windows opened up a prompt to "select an app to open this .json file" (I have no JSON file, I'm just in the Windows Terminal, pressing that key combo, maybe it's opening the Terminal app's settings file?)
  • Alt+Shift+, might work, it comes through as Alt-Shift-< (like Alt-, but with the shift pre-translating the key to < like Shift-, did
  • Ctrl+Alt+Shift+, is weird, like Ctrl+Alt+. it reports control, alt, and shift modifiers with OemComma key, but again no KeyChar and I don't know if PSReadLine will handle that

In short, control characters have a long history, and I would not expect random combinations of Ctrl and keys other than alpha-numeric to necessarily behave as you might expect, they're liable to get intercepted at different levels of the stack (the keyboard drive itself, the OS, the terminal emulator, .NET's own ReadKey implementation, etc.).

@akbyrd
Copy link
Author

akbyrd commented Jan 30, 2023

Yea, there's a lot fiddly stuff going on here.

Here's a more minimal case to examine. I'm launching PowerShell directly now to eliminate Windows Terminal from the equation. I've also removed my PowerShell profile

Register these 2 handlers, then press Ctrl+,

Set-PSReadLineKeyHandler -Chord "Ctrl+," -ScriptBlock { Inject-Command("'c,'") }
Set-PSReadLineKeyHandler -Chord "Ctrl+Shift+," -ScriptBlock { Inject-Command("'cs,'") }

The expectation is that Ctrl+, is bound to the first handler. However, the second handler is executed.

image

As far as I can tell, Ctrl+, is not being intercepted by anyone else.

@andyleejordan
Copy link
Member

@akbyrd try this experiment in PowerShell:

@andyschwartc421 ~
> [Console]::ReadKey()
,
KeyChar      Key Modifiers
-------      --- ---------
      , OemComma         0

Like, directly execute [Console]::ReadKey() and then test your key chords. Neither Ctrl+, nor Ctrl+Shift+, were getting sent to ReadKey() for me, I had to press just ,. If ReadKey can't get it, PSReadLine definitely can't get it.

@akbyrd
Copy link
Author

akbyrd commented Jan 30, 2023

ReadKey sees all three chords on my machine.

PS C:\Windows\System32> [Console]::ReadKey()
,
KeyChar      Key Modifiers
-------      --- ---------
      , OemComma         0

PS C:\Windows\System32> [Console]::ReadKey()

KeyChar      Key Modifiers
-------      --- ---------
       OemComma   Control

PS C:\Windows\System32> [Console]::ReadKey()

KeyChar      Key      Modifiers
-------      ---      ---------
       OemComma Shift, Control

I didn't explicitly state it in my previous post, but the Ctrl+, handler works fine until the Ctrl+Shift+, handler is registered. It's only once both are registered that it misbehaves.

@StevenBucher98
Copy link
Collaborator

Maybe I am missing something but is it odd that for the last two you posted it does not show the , in the KeyChar? Does that mean ReadKey() is having issues getting it? @andschwa

@StevenBucher98 StevenBucher98 added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-KeyHandlers Label for issues related to key handlers and removed Needs-Triage 🔍 It's a new issue that core contributor team needs to triage. labels Mar 27, 2023
@andyleejordan
Copy link
Member

It is odd, and it means we (and the users) can't depend on KeyChar nor Modifiers in these cases. It looks like Key is at least consistently OemComma, but yeah what .NET is returning to is via ReadKey is probably the root of the problem.

@SeeminglyScience
Copy link
Contributor

SeeminglyScience commented Mar 29, 2023

It is odd, and it means we (and the users) can't depend on KeyChar nor Modifiers in these cases. It looks like Key is at least consistently OemComma, but yeah what .NET is returning to is via ReadKey is probably the root of the problem.

KeyChar shows a representation of the character, but it's not always particularly relevant. iirc for anything with a direct representation (like when Ctrl is involved) it just shows default(char).

Modifiers should be accurate on Windows though. I believe Jason made Shift essentially ignored and instead it looks for the character that would be created. So Ctrl + Shift + , is no different from Ctrl + ,, but Ctrl + < gives the desired result (feel free to correct me if I'm wrong there @lzybkr).

@andyleejordan
Copy link
Member

In summary: key chords are a mess.

@lzybkr
Copy link
Member

lzybkr commented Mar 30, 2023

@SeeminglyScience - mostly accurate IIRC. On non-Windows, you couldn't reliably get the shift state required to generate specific keystroke, so I moved towards a world where you wouldn't specify the shift state. I think there were also problems with consistency on Windows even when you could count on the shift state - you might have a profile that works with one keyboard layout but not another.

In hindsight I might have gone too far. The world seems to expect that Ctrl+C means Ctrl+c - so maybe the shift state should have been required for alphabetic characters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-KeyHandlers Label for issues related to key handlers Issue-Bug It either shouldn't be doing this or needs an investigation.
Projects
None yet
Development

No branches or pull requests

5 participants