fix: support direct commits in marked text clients - #1165
Conversation
|
|
||
| let forceMarkedText = | ||
| session != 0 && | ||
| rimeAPI.get_option(session, "_force_marked_text_for_direct_commit") |
There was a problem hiding this comment.
I set that in squirrel.custom.yaml, near to ascii_mode, like this:
There was a problem hiding this comment.
Options that start with _ are cleared if switched to another input schema.
https://github.com/rime/librime/blob/1d0df6e40cdcac17a986adc65e4668ae84ae0ada/src/rime/context.h#L81
I guess the option value should persist thoughout the input session - with its lifetime bound to the conntected app. Do not start with _.
There was a problem hiding this comment.
Thanks for the quick fix.
For the PR to be self-contained, please also update data/squirrel.yaml (in a new commit) to configure known apps that need this workaround.
|
|
||
| data_files=( | ||
| $(ls data/plum/* | xargs basename) | ||
| $(for file in data/plum/*; do basename "$file"; done) |
There was a problem hiding this comment.
This change is nice to have, but better be in a separate git commit.
|
|
||
| let forceMarkedText = | ||
| session != 0 && | ||
| rimeAPI.get_option(session, "_force_marked_text_for_direct_commit") |
There was a problem hiding this comment.
Options that start with _ are cleared if switched to another input schema.
https://github.com/rime/librime/blob/1d0df6e40cdcac17a986adc65e4668ae84ae0ada/src/rime/context.h#L81
I guess the option value should persist thoughout the input session - with its lifetime bound to the conntected app. Do not start with _.
| // one before accepting insertText. | ||
| if forceMarkedText && preedit.isEmpty && !string.isEmpty { | ||
| let markedText = NSMutableAttributedString(string: string) | ||
| client.setMarkedText( |
There was a problem hiding this comment.
Question:
Is it possible that we always do it when committing text without marked text?
We can experiment with the current solution to see if there are side effects in normal cases, and make it the default behaviour at some point.
|
Thanks for the review. I’ve split the PR into two commits and corrected the option value name. I’ll also test it by enabling it in the global default features to see how it behaves. For now, though, it may be better to keep it optional. |
Sounds good. |
|
Great. Feel free to merge the commits when you think it's ready. |
|
Thanks! I think everything looks good on my side. |
2 participants
Summary
force_marked_text_for_direct_commitorg.alacritty)basenameCloses #741