Skip to content

Fix 'm' not displayed in uppercase (azerty) #10

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

Closed

Conversation

lcazef
Copy link

@lcazef lcazef commented Sep 22, 2020

On azerty layout, capslock + "M" was decoded as a lowercase "m".

@lcazef lcazef changed the title Fix 'm' not displayed in uppercase Fix 'm' not displayed in uppercase (azerty) Sep 22, 2020
Copy link
Contributor

@vinc vinc left a comment

Choose a reason for hiding this comment

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

I tested your code on MOROS and it doesn't break anything but I can't reproduce the issue on my laptop with an azerty keyboard. The uppercase M seems to work for me with the current version of the crate.

@@ -324,7 +324,9 @@ impl KeyboardLayout for Azerty {
}
}
KeyCode::SemiColon => {
if modifiers.is_shifted() {
if map_to_unicode && modifiers.is_ctrl() {
DecodedKey::Unicode('\u{000D}')
Copy link
Contributor

Choose a reason for hiding this comment

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

The layout would then have two keys producing \u000D with ctrl because of KeyCode::M. I think your change is correct and KeyCode::M should not produce \u000D on azerty. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I agree that this key should produce Ctrl-M (0x0D) when pressed with the Control modifier. The 0x0D on line 397 does need removing though.

Copy link
Member

@thejpster thejpster left a comment

Choose a reason for hiding this comment

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

The DecodedKey::Unicode('\u{000D}') on Keycode::M needs removing.

@thejpster
Copy link
Member

Thank you for this PR. I spotted one other bug in my code that we should clean up at the same time!

@vinc vinc mentioned this pull request Sep 17, 2022
@vinc
Copy link
Contributor

vinc commented Oct 20, 2022

This PR can be closed now that the other one has been merged.

@thejpster thejpster closed this Oct 20, 2022
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.

3 participants