Fix Emits of Additional ANSI Characters

[schrieveslaach]

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

Fixes #4975

[op3]

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]

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

[op3]

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]

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

[wez]

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]

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]

@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]

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

[stefanfrede]

What can be done to speed up the merging of this request? As far as I can tell, this will help solve the problem I and a handful of others are having. So if there is anything I can do, please let me know.

[willghatch]

I'd like to chime in that while this patch is an improvement (and I would love to see it landed), I think more generally it should handle all modifiers this way. Eg. in my keyboard layout as mentioned in #6518 (comment) the problem also happens with Hyper_L and Hyper_R.

[schrieveslaach]

@willghatch, thanks for the information. I updated the PR and also use now an existing method of xkbcommon.

@wez, any chance to review this PR because it is very minimal now?

[willghatch]

I've been using the old version of this patch, plus a couple lines for hyper keys, for over a month now. This new version looks even better. I'll switch to using it, and I hope it can land.

[Darkproduct]

I'm an idiot. I forgot to change the branch to fix-additional-emit.

It works perfectly. I'll use this branch until it is hopefully merged. Thank you so much.

[gagbo]

Small ping again, is there anything you want us to test/assert in order to merge this? The patch is small in size but it's impact is almost infinitely huge for me (and a few others, probably)

[willghatch]

As an update to my previous comment, I've been using wezterm with the current version of this patch since March. With my keyboard layout, Wezterm is basically unusable without this patch, but this patch fixes the problem. This patch makes a huge impact for people who use the popular NEO keyboard layout or a custom keyboard layout on Linux. This patch is solid gold.

Note that this patch fixes #6518 as well as #4975 since they are duplicates.

[swy7ch]

Hi all,

Any news on this? I'm using the fr(ergol) layout, and it is bugged in wezterm since it uses a dead key for accentuation :(

[schrieveslaach]

The issue still persists on main. I rebased this branch and tested again that this change fixes the issue. @wez, do you mind to have a quick look and maybe merging it?

[bnjbvr]

This also seems to fix #5866 for me, fwiw!

[schrieveslaach]

@bnjbvr, thanks for the note. Please note, I rebased and tested this PR again and still works for me.

@wez, @bew, what can I add to this PR to push it across the finish line?

[schrieveslaach]

@bew, I noticed that you labeled the PR with X11 and that made me realize that the code change is related to the X11 subsystem (totally forgot about it). However, I'm running wezterm under Wayland (GNOME has no x11 session anymore). Is the label wrong or should I investigate why the code is used on Wayland too?

[bnjbvr]

I think my earlier scan of the code showed that the same code was used on Wayland, resulting in fixing the issue for me on Wayland. I seem to recall I came to this conclusion by going up the chains of callers via LSP, fwiw

[honnorat]

This PR fixes #5866, which is a blocker for me.
There is no conflict with wezterm:main, @wez is it possible to merge it soon ?