Skip to content

Conversation

@ichizok
Copy link
Member

@ichizok ichizok commented Nov 22, 2018

fixes #785

  • Restrict the condition not calling interpretKeyEvents: ; only Ctrl-O and Ctrl-U without no marked text

Call interpretKeyEvents: with except Ctrl-O and Ctrl-U since should
translate key-input by doCommandBySelector:.
@ychin
Copy link
Member

ychin commented Nov 22, 2018

Wow… these are some really unfortunately hairy special cases. A couple comments for @ichizok:

  • Has this bug been reported to Apple before? I played around with the Japanese IME with "Windows-like shortcuts" turned on, and seemed to me Ctrl-U/I/O would toggle hiragana/katakana/romanji when marked texts are visible, but when no marked texts are up I couldn't figure out what Ctrl-U/O are even supposed to do. Is this simply a bug where the IME is eating keys for no good reason?

  • The condition for checking whether we call interpretKeyEvents is getting quite long. Can you move the "is this Ctrl-U or Ctrl-O" logic to a separate variable to make it clearer?

  • I'm wondering if we should make sure to only enable this special case for the Japanese IME, since I don't know if there are other IMEs there could for some reason use Ctrl-U and broken by MacVim's special handling. It would also make it more explicit that this is a special hack for one case instead of a generic thing. I think something like this would work:

    BOOL isJapaneseIME = [[[NSTextInputContext currentInputContext] selectedKeyboardInputSource] hasPrefix:@"com.apple.inputmethod.Kotoeri"];
    

Only limiting this condition to be under Japanese IME helps prevents
unintended effects on other IMEs as well, as this bug is only known to
happen under Japanese IME with Windows shortcuts.

Also update comments to be clear that most of the time you do want to go
through the interpretKeyEvents: path.
@ychin ychin merged commit b1e26a6 into macvim-dev:master Nov 25, 2018
@ychin
Copy link
Member

ychin commented Nov 25, 2018

I made those changes and it's merged now.

@ichizok
Copy link
Member Author

ichizok commented Nov 25, 2018

@ychin Sorry for the late reply.. and Thank you for improving 😄

@ichizok ichizok deleted the fix/ctrl-6 branch November 25, 2018 16:06
@ychin
Copy link
Member

ychin commented Nov 26, 2018

Sure np. I filed a bug with Apple, so hopefully they can address this (not going to share the radar number since it's internal to Apple anyway).

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.

CTRL-6 stopped working after #742

2 participants