-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add trigger hotkeys for temporary word/emoji prediction #658
base: main
Are you sure you want to change the base?
Conversation
@@ -595,6 +597,7 @@ def __init__( | |||
'label': _('On'), | |||
} | |||
} | |||
# TODO: modify this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where these menus appear, so I'm not sure if there should be new menus for the new hotkeys. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These menus appear here:
But only if a keybinding to the command toogle_input_mode_on_off
is set (needs an ibus restart
after setting a keybinding to this command for the menu to appear.
I hide this menu if no key is bound to the command because there was a time (long ago) when Gnome did not want to have ibus input methods to have an On/Off switch to toggle between direct input mode (pass almost everything through but still do basic stuff like Compose) and "normal" input mode. I remember that they wanted even the CJK input methods ibus-anthy and ibus-libpinyin to remove that, the idea was that one should switch to a plain keyboard layout instead of switching the input method Off and having two ways of doing things was superfluous and would just clutter the UI.
The CJK input methods refused to do that and never removed direct input mode though.
But when I wanted to add a direct input mode to ibus-typing-booster long ago, I wanted to avoid getting into arguments with Gnome design people about this, so I added this in a way that it is hidden by default, so nobody testing the defaults would notice but the feature could be enabled. But if enabled, I wanted to have that menu in a similar way like the CJK input methods have it.
Now the design trend seems to go into the opposite direction and I even got a request to add a direct input mode to ibus-m17n:
Because now some people want to have only an input method setup by default in Gnome, i.e. for Japanese have only ibus-anthy by default and not the jp
keyboard as a second entry in the menu.
Or have only ibus-m17n with hi-inscript2.mim
by default for the hi_IN
locale and not as it is now have that plus the in(eng)
keyboard layout. If that keyboard layout option is gone, one really needs the option to switch the input method off to be able to type ASCII at all.
Long explanation, but I think we don't need to add more menus for new commands, not every command needs to have a menu.
def _command_trigger_emoji_predictions(self) -> bool: | ||
'''Handle hotkey for the command “trigger_emoji_predictions” | ||
|
||
:return: True if the key was completely handled, False if not. | ||
''' | ||
self._temporary_emoji_predictions = True | ||
return True | ||
|
||
def _command_trigger_word_predictions(self) -> bool: | ||
'''Handle hotkey for the command “trigger_word_predictions” | ||
|
||
:return: True if the key was completely handled, False if not. | ||
''' | ||
self._temporary_word_predictions = True | ||
return True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_handle_hotkeys()
looks like it can only call one of these at a time even if they both have the same hotkey. Personally, I'd want to give them different hotkeys, but I could picture somebody wanting to trigger both at the same time. Do you think I should add a way to do that? Maybe a third hotkey, trigger_all_predictions
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think _handle_hotkeys()
can call several things if they share the same hotkey. I am pretty sure about that because Tab
is usually bound both to enable_lookup
and select_next_candidate
and I remember seing both being called when pressing Tab recently when I worked on the ja-anthy
support. So I think it works but should verify this again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, no, I think it can call several commands as long as they keep failing (I.e. returning False) but it stops at the first command which succeeds.
Your idea surely looks interesting! I’ll have a closer look later, need to do something else for the next few hours now ... |
A little bit easier than your approach would be two commands to toggle the value of But if I understand you correctly that is not really what you want, you can already do that with your way of changing the existing gsettings keys with keybindings. And toggling word predictions on that way would leave then on until you toggle them off again manually. I think you want to press a key to enable word and/or emoji predictions for the next word only. I.e.
So your keybinding would need to be pressed before starting to type a new word, would enable the predictions for that word only and reset automatically to no predictions when that word is finished, right? Pressing your keybinding in the middle of the word would be too late in many cases because then the early commits have probably already committed and you don't get that part of the word back into preedit. That would indeed to almost the same as starting a word with an emoji trigger character. It sounds like a reasonable approach and I'll certainly merge your patch if you can make that work. |
These are meant to behave similarly to emoji trigger characters, but without adding any characters to the text itself. And with support for word predictions too.
At this point, the code is mostly working but has some rough edges and I haven't added tests yet. Do you think this is a reasonable approach? If so, I'll finish it up.