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 use_ime=true swallows keys with LEADER modifier in macOS #1410

Closed
wants to merge 1 commit into from

Conversation

tizee
Copy link

@tizee tizee commented Dec 17, 2021

close #1409

wez added a commit that referenced this pull request Dec 31, 2021
When the IME is enabled, pressing `CTRL-A P` would generate
`Composed("P")` for the second key press, which we couldn't
then match in the keymapping layer.

This commit checks for that and normalizes it to `Char('P')`
instead.

refs: #1409
refs: #1410
Copy link
Owner

@wez wez left a comment

Choose a reason for hiding this comment

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

Thanks for diving in and submitting this PR!
I ended up fixing this in a slightly different place: e3afdd7

I don't think this PR is needed any more, except that there is another hunk that looks like it is tackling a different problem: I'd love to understand more about that!

@@ -1791,7 +1791,8 @@ impl WindowView {
"\x08"
} else if virtual_key == super::keycodes::kVK_Tab {
"\t"
} else if !use_ime && virtual_key == super::keycodes::kVK_Delete {
} else if virtual_key == super::keycodes::kVK_Delete {
// When using IME in macOS, delete applied in candidate window.
Copy link
Owner

Choose a reason for hiding this comment

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

This appears to be a different issue from the main thing you mentioned in #1409; could you give some more details on how to reproduce the issue that you're fixing with this?

Copy link
Owner

Choose a reason for hiding this comment

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

ah, I see now; I'll fix this in a commit I have in progress!

@wez wez closed this Dec 31, 2021
wez added a commit that referenced this pull request Dec 31, 2021
Certain keys are "handled" by the IME through it generating a "noop"
command.

That's not super useful for us, so this commit detects the noop case
and then treats it as though the IME didn't handle the input event.

While implementing the above fix, I realized that the same technique
could be used more generally to return processing to our main input
handling for the various selectors that we do recognize: we were
essentially inferring the original key combinations based on the
selector which is not scalable and potentially lossy.

We can't capture CTRL-ESC this same way, as that key combination
is magical and is routed to the callback without generating any
key events.

refs: #615
refs: #975
refs: #1410
@tizee tizee deleted the fix/macos_ime branch December 31, 2021 06:13
@tizee tizee restored the fix/macos_ime branch December 31, 2021 08:29
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.

use_ime=true conflicts with key assigments that defined in LEADER mode
2 participants