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

tweak key reporting #4861

Merged
merged 12 commits into from
Aug 12, 2024
Merged

tweak key reporting #4861

merged 12 commits into from
Aug 12, 2024

Conversation

willmcgugan
Copy link
Collaborator

@willmcgugan willmcgugan commented Aug 9, 2024

Some tweaks to how keys are reported (with the Kitty protocol).

  • If "shift" is held with another modifier it is reported as "shift", rather than a capital letter. Previously if you press shift + ctrl + e, it is reported as "ctrl+E". Now it is reported as "ctrl+shft+e".

I think this makes sense, as such a combination is unlikely intended to be a printable key.

  • Removed caps_lock and num_lock modifiers. This resulted in shift + key, and caps_lock + key being considered as different keys -- which I don't think makes sense. If this is ever a requirement, I think we should add booleans to the Key event.

@darrenburns
Copy link
Member

darrenburns commented Aug 9, 2024

Will this break existing apps with bindings for ctrl+E for example? Will those apps need to update their Bindings to ctrl+shift+e?

@willmcgugan
Copy link
Collaborator Author

It would. I've added a note in the changelog.

@darrenburns
Copy link
Member

Just had a thought about this - isn't it a bit strange to have this reported like this but not have "E" be reported as "shift+e"? I'm not sure which is better, but I'd definitely expect it to be consistent.

@willmcgugan
Copy link
Collaborator Author

Just had a thought about this - isn't it a bit strange to have this reported like this but not have "E" be reported as "shift+e"? I'm not sure which is better, but I'd definitely expect it to be consistent.

There isn't a simple one to one mapping of shift + letter. It depends on your keyboard and locale. e.g. shift+3 will give a £ in GB layout or a # in US layout. I think that's also true of letter keys with some international layouts. There may also be multiple ways of producing a key. For instance, if capslock is on, you can produce capital letters without holding shift.

I think its safer to trust that when the keyboard produces a key, that's what the user intended, and we don't want to make assumptions about how it was produced.

@willmcgugan
Copy link
Collaborator Author

Actually, I think I just talked myself out of part of this update. When the keyboard reports an alphanumeric letter, Kitty does not report that the shift modifier was pressed.

It does report the shift modifier for non alphanumeric keys. i.e. shift+space reports that the shift modifier was held.

So I don't think Textual should be trying to deduce that shift was pressed at all. So ctrl+shift+E should be reported as ctrl+E.

CHANGELOG.md Outdated
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Added `tooltip` to Binding https://github.com/Textualize/textual/pull/4859

### Changed

- Change to how keys with modifiers are reported. When shift + another modifier is pressed, then the key name will contain "shift", and the key will be *lower case*. i.e. `ctrl+shift+E` and not `ctrl+E` https://github.com/Textualize/textual/pull/4861
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need updating to match the latest state of the PR?

@willmcgugan willmcgugan merged commit 23172c1 into main Aug 12, 2024
20 checks passed
@willmcgugan willmcgugan deleted the key-modifiers branch August 12, 2024 10:13
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.

2 participants