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.
There was a problem hiding this comment.
Follow-up after testing: I tried enabling force_marked_text_for_direct_commit globally. With the earlier implementation, Microsoft Word showed font fallback after direct commits. The synthetic marked-text cleanup added in #1171 has fixed that issue in my current testing. I am still testing this more broadly, so I do not think it should become the global default yet; #1171 keeps it app-scoped and enables it only for the known affected clients, Alacritty and VS Code.
|
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