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 Emits of Additional ANSI Characters #4991

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

schrieveslaach
Copy link

This commit ensures that ANSI chars won't be emitted when pressing non-standard modifier keys.

Fixes #4975

@op3
Copy link

op3 commented Feb 13, 2024

This does not catch all possible keys that should be silent. For example, using the neo layout, I can press Shift + ISO_Level3_Shift + Tab, which results in ISO_Level5_Lock. The current nightly version and also this patch still emit a Tab key, even though no key should be emitted.

(there are also a few typos in the comment in the code)

@schrieveslaach
Copy link
Author

@op3, thanks for testing. Do you see more missing keys? If not, I would update my PR with the aforementioned missing keys.

@op3
Copy link

op3 commented Feb 14, 2024

So, ISO_Level3_Lock and ISO_Level5_Lock would be missing, obviously.

Another key combination that should not result in any characters is ISO Level5 Shift + 4. This key combination is not assigned and thus is not supposed to do anything, but the current code tries to find a fallback (which would be 4).

Obviously, this is much less of a problem than the modifier keys, because one would usually not press this key combination. Still, I would consider this to be incorrect behavior. And adding “4” to the list of exceptions also does not feel right (too specific …).

There are a few more unassigned key combinations. The W, A, and Z keys on layer 5 come to mind.

@schrieveslaach schrieveslaach force-pushed the fix-additional-emit branch 2 times, most recently from 5b56c5e to 9992d59 Compare February 15, 2024 18:58
@schrieveslaach
Copy link
Author

@wez, does this fix provide a solution that you would consider to be merged?

@wez
Copy link
Owner

wez commented Jul 13, 2024

Thanks for this; only just getting around to looking at it.
I think the question on my mind is, are there any cases where this change has a negative impact? eg: for non-neo layout users?

@tsacha
Copy link

tsacha commented Jul 24, 2024

Can we include ISO_Level3_Latch and ISO_Level5_Latch for Ergo-L layout (https://ergol.org/)? 🙏

I think the question on my mind is, are there any cases where this change has a negative impact? eg: for non-neo layout users?

Russian layout mentioned in #4910 is still working with this PR.

@schrieveslaach
Copy link
Author

@wez, sorry for the late response.

I think the question on my mind is, are there any cases where this change has a negative impact? eg: for non-neo layout users?

I couldn't find any issue when using the regular German layout and the behavior with and without my fix is the same.

@tsacha, I also included ISO_Level3/5_Latch

@apraga
Copy link

apraga commented Nov 20, 2024

@wez This PR works with ergol so merging it would be great !

This commit ensures that ANSI chars won't be emitted when pressing
non-standard modifier keys.

Fixes wez#4975
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.

Non-standard modifier keys emit original ANSI characters for that key on keydown
5 participants