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

Fix altgr keyboard handling #259

Merged
merged 1 commit into from
Jun 24, 2023
Merged

Fix altgr keyboard handling #259

merged 1 commit into from
Jun 24, 2023

Conversation

waf
Copy link
Owner

@waf waf commented Jun 24, 2023

Previously, PR #252 was attempting to fix the scenario where users can have a Control-Alt-Space keybinding; the keybinding would be fired but the space key would be erroneously typed.

However, that PR broke AltGr handling. It assumed that filtering Ctrl to detect keybindings would be OK, and Alt keys would be unaffected. This is not correct, though, because AltGr is represented via Ctrl-Alt in C#, so it actually broke every AltGr based character, like curly-brace ({) on AZERTY keyboards.

This commit undoes the original fix for PR #252 and fixes it in a better way. Instead of assuming Ctrl is a keybinding, we use the actual configured keybindings in the filter, rather than assuming Ctrl represent a keybinding.

Previously, PR #252 was attempting to fix the scenario where users can
have a Control-Alt-Space keybinding; the keybinding would be fired but
the space key would be erroneously typed.

However, that PR broke AltGr handling. It assumed that filtering Ctrl
to detect keybindings would be OK, and Alt keys would be unaffected.
This is not correct, though, because AltGr is represented via Ctrl-Alt,
so it actually broke every AltGr based character, like curly-brace on
AZERTY keyboards.

This commit undoes the original fix for PR #252 and fixes it in a better
way. Instead of assuming Ctrl is a keybinding, we use the actual
configured keybindings in the filter, rather than assuming Ctrl
represent a keybinding.
@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

Merging #259 (7e504ae) into main (e767bab) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #259     +/-   ##
=======================================
- Coverage   85.2%   85.2%   -0.1%     
=======================================
  Files         45      45             
  Lines       3874    3875      +1     
  Branches     598     598             
=======================================
  Hits        3303    3303             
  Misses       433     433             
- Partials     138     139      +1     
Impacted Files Coverage Δ
src/PrettyPrompt/Panes/CodePane.cs 98.6% <100.0%> (+<0.1%) ⬆️
src/PrettyPrompt/Prompt.cs 87.3% <100.0%> (ø)

... and 1 file with indirect coverage changes

@waf
Copy link
Owner Author

waf commented Jun 24, 2023

See user bug reports in CSharpRepl: waf/CSharpRepl#260, waf/CSharpRepl#265, waf/CSharpRepl#266

@waf waf merged commit 5c82746 into main Jun 24, 2023
2 checks passed
@waf waf deleted the fix-bug-for-altgr-code-keyboards branch June 24, 2023 08:37
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

1 participant