CPLAT-12413: make shortcuts reachable under a Korean input source - #167
Merged
Merged
Conversation
Ten default bindings were a bare uppercase letter, which a user typing under a 2-set Korean layout cannot produce: Shift only yields a distinct character on the keys carrying a doubled consonant or ㅒ/ㅖ (q w e r t o p), every other key sends the same jamo either way, and this bubbletea version's tea.Key has no Shift field, so the distinction is already gone by the time we see it. Where a lowercase action shared the letter, the lowercase case sits earlier in the switch and swallowed both presses — Files "f" beat Fork "F", Move "m" beat ImportMem "M", and fold-all "f" beat expand-all "F". That is the reported symptom: folding appeared to only ever fold. This is a follow-up to CPLAT-10983. The langmap maps a jamo back to its Latin key correctly, but it can only map to lowercase, so uppercase bindings stayed dead even with langmap working. Rebound to lowercase or ctrl+ keys (modifiers survive the IME): Session.Views V -> v Actions.Fork F -> b (branch off) Actions.ImportMem M -> a (add memory) Actions.RemoveMem X -> z Conversation.ExecutionContexts A -> a Conversation.RegionUp/Down K/J -> ctrl+p / ctrl+n Conversation.LiveToggle L -> ctrl+l Conversation.Input I -> w Preview.ExpandAll F -> u (matches Session.ExpandAll) config/plugins reverse search N -> b (3 call sites) Region nav takes ctrl+p/ctrl+n rather than ctrl+k/ctrl+j because ctrl+j is LF and terminals may deliver it as Enter. Four session-list keys were hardcoded in the handler, so no keymap override reached them and no struct-level check could see them. The daily-view toggle was one of them, spelled "D" — unreachable, and the help overlay advertised it anyway. They are now Keymap fields: fold_group, state_menu, daily_view (D -> d), page_menu. migrateKeymapDefaults moves an existing config.yaml off the old defaults, rewriting only values that still match the old default so a deliberately chosen key survives. Also fixed along the way, each found by checking the Keymap struct against every place that consumes it rather than by reading: - The KEYMAPS config page listed fields by hand and had drifted, omitting 15 bindings — most of the actions menu (fork, edit, tags, copy, changes, new, remote). It claimed keys did not exist that the menu answered to. Now walks the struct by reflection. - mergeKeymap ignored Actions.Tags and Actions.Edit, so those overrides never loaded. - KeymapsConfig had no Preview section at all, so every preview binding the config page offered was silently dropped on load. - fillKeymapDefaults skipped Actions.Changes/Copy/Tags, writing them to config.yaml as blanks. - Preview.FoldAll/ExpandAll were hardcoded inside FoldState.HandleKey, which has no access to the keymap. Split into FoldAll/ExpandAll methods dispatched by the caller. Five guard tests, each verified by reintroducing the defect and confirming it fails: - reachability and per-menu uniqueness over the Keymap by reflection, so a new field is covered without anyone extending a list - a source scan for hardcoded uppercase dispatch, which is the only thing that can catch a literal like key == "D" that never reaches the struct - config-page and defaults-fill round-trip coverage Verified by driving the real Update loop with the jamo a Korean IME sends, not just by asserting on the keymap. README's session and inspector tables were stale on g/G/L/S/v/J independently of this change and are corrected.
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
29af0b1 < |
2026-09-28 07:34 UTC | 0 | 0 | 0 |
Last scanned: 29af0b1 · 2026-09-28 07:34 UTC
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
29af0b1 < |
2026-09-28 07:34 UTC | 0 | 0 | 0 |
Last scanned: 29af0b1 · 2026-09-28 07:34 UTC
Kairo-Kim
approved these changes
Sep 28, 2026
Kairo-Kim
added
the
auto-review/approved
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA: https://sendbird.atlassian.net/browse/CPLAT-12413
Why
Ten default bindings were a bare uppercase letter, which a user typing under a 2-set Korean layout cannot produce. Shift only yields a distinct character on the keys carrying a doubled consonant or ㅒ/ㅖ (
q w e r t o p); every other key sends the same jamo either way, and this bubbletea version'stea.Keyhas no Shift field, so the distinction is gone before it reaches us.Where a lowercase action shared the letter, the lowercase case sits earlier in the switch and swallowed both presses —
Files "f"beatFork "F",Move "m"beatImportMem "M",foldAll "f"beatexpandAll "F". That is the reported symptom: folding appeared to only ever fold, never expand.Follow-up to CPLAT-10983. The langmap maps a jamo back to its Latin key correctly, but only ever to lowercase, so uppercase bindings stayed dead even with langmap working.
Rebinds
Lowercase, or
ctrl+where no letter was free (modifiers survive the IME):Session.ViewsVvActions.ForkFb(branch off)Actions.ImportMemMa(add memory)Actions.RemoveMemXzConversation.ExecutionContextsAaConversation.RegionUp/RegionDownK/Jctrl+p/ctrl+nConversation.LiveToggleLctrl+lConversation.InputIwPreview.ExpandAllFu(matchesSession.ExpandAll)Nb(3 call sites)Region nav takes
ctrl+p/ctrl+nrather thanctrl+k/ctrl+jbecausectrl+jis LF and terminals may deliver it as Enter.Four session-list keys were hardcoded in the handler, so no keymap override reached them and no struct-level check could see them. The daily-view toggle was one, spelled
"D"— unreachable, with the help overlay advertising it anyway. Now Keymap fields:fold_group,state_menu,daily_view(D→d),page_menu.migrateKeymapDefaultsmoves an existingconfig.yamloff the old defaults, rewriting only values that still match an old default so a deliberately chosen key survives.Also fixed
Found by checking the Keymap struct against every place that consumes it, rather than by reading:
mergeKeymapignoredActions.TagsandActions.Edit, so those overrides never loaded.KeymapsConfighad noPreviewsection at all, so every preview binding the config page offered was silently dropped on load.fillKeymapDefaultsskippedActions.Changes/Copy/Tags, writing them toconfig.yamlas blanks.Preview.FoldAll/ExpandAllwere hardcoded insideFoldState.HandleKey, which has no keymap access. Split intoFoldAll/ExpandAllmethods dispatched by the caller.Test plan
go build ./... && go vet ./... && go test ./...green on latest masterkey == "D"that never reaches the structUpdateloop with the jamo a Korean IME actually sends (ㄹ,ㅕ,ㅍ, …) and confirmed each rebound action fires — not just asserting on the keymapconfig.yamlcarries the newpreview:section and every new fieldUpdatelevelNotes for reviewers
V,x→F/M/X, and conversation-viewA/K/J/L/I. Overridable inkeymap.yaml.g/G/L/S/v/Jindependently of this change; corrected here.Actions.Jumplooked like a dead field during triage but is live atapp.go:3943— left alone.Security checklist
0.0.0.0/0inbound