Repository navigation
Mapper: captured sequences render as macros; a lone media key is its system action - #1328
Conversation
…system action Found on the Air: Mute followed by ⌃⇧⌘4 was rendered by the generator as a bare list "(mute M-S-C-4)", which Kanata cannot parse, so the save was refused with the bridge's generic message. Multi-token outputs are now always a "(macro …)". A lone captured media key is now the system action it names (MapperViewModel.applyCapturedOutput), so the output tile shows the action's symbol from the first moment instead of an emoji that changed to the symbol after the save, and the save goes through the action path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Review (diff only) No blockers, but two things worth checking:
Minor: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90babb601f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let action = SystemActionInfo.find(byOutput: KeyMappingFormatter.toKanataFormat(sequence)), | ||
| action.isSupportedInDriverlessSession | ||
| { | ||
| selectSystemAction(action) |
There was a problem hiding this comment.
Defer media-action selection until capture finalizes
When a media key begins a multi-key output, sequence capture first emits the one-key prefix, so this call selects the system action and immediately starts saveSystemActionMapping because the input is already set. The later callback containing the full sequence sets outputSequence but never clears selectedSystemAction, and autosaveCapturedMapping prioritizes that action; consequently an actual Mute-then-chord capture saves only Mute rather than the new macro, while the added macro test bypasses this streaming path by assigning outputSequence directly. Classify the media key only after capture finalization, or otherwise avoid the premature save and clear the action when the sequence grows.
Useful? React with 👍 / 👎.
Two findings from the phase 6 hardware check on the Air.
KanataKeyConverter.convertToKanataSequenceas a bare list(mute M-S-C-4), which Kanata can't parse, so the save was refused with the bridge's generic "could not be parsed". Multi-token outputs are now always(macro …); a chord is still one token with modifier prefixes. Reproduced and pinned bytestMediaKeyThenChordSavesAsAMacroagainst the real bridge, plus unit expectations.MapperViewModel.applyCapturedOutput→selectSystemAction), so the output tile shows the action's SF Symbol from the first moment (previously an emoji fromKeyPress.displayStringthat changed to the symbol after the save) and the save goes through the action path.testLoneMediaKeyCaptureBecomesTheSystemAction.KEYPATH_TEST_ENFORCE_CLEAN_SUMMARY=1 Scripts/run-tests-safe.sh: 5,793 passed, 0 app warnings/errors, 0 build warnings.check-accessibility.pyclean. Doc note in rules-admission-review.md §4.5.🤖 Generated with Claude Code