fix: use a bindable chord in keymap.conf's starter examples - #411
Merged
Conversation
cmd+shift+d is dashboard's default, so uncommenting the starter file's own `map cmd+shift+d toggle_split` line lost the built-in collision and bound nothing. Switch the examples to cmd+shift+l, the chord README already uses, and pin every starter `map` example with a test that uncomments it and requires it to parse clean and bind, so a future built-in claiming one fails there. Also point the header at where a skipped line is reported: Settings > Key Mapping and `agtermctl keymap list`. Fix #405
A custom command's shortcut rots the same way a built-in map does: validateCommands clears it when a built-in has claimed the chord. Collect command examples too, match the verb by token so a tab-separated line still counts, and pin the example count so one silently dropped from the guard fails.
parseCommandLine absorbs an unparseable first token as shell text with no diagnostic, so a command example could satisfy the guard while binding nothing. Count the shortcuts that survive instead, pinned at the two map examples plus the two chorded command ones.
There was a problem hiding this comment.
Pull request overview
This PR fixes the shipped keymap.conf starter examples so they use a bindable chord (avoiding collisions with built-in defaults like dashboard), and adds a regression test to prevent starter examples from silently rotting as built-ins evolve.
Changes:
- Update
ConfigPaths.starterKeymapConf()startermapexamples to usecmd+shift+land add a header hint pointing users to where skipped/rejected lines are reported. - Add a test that uncomments and validates all starter
map/commandexamples, and asserts the expected number of surviving bound chords. - Document the “starter example rot” class and the new guard test in
.claude/rules/keymap.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
agtermCore/Sources/agtermCore/ConfigPaths.swift |
Updates starter keymap examples to a bindable chord and points users to diagnostics visibility (Settings ▸ Key Mapping, agtermctl keymap list). |
agtermCore/Tests/agtermCoreTests/ConfigPathsTests.swift |
Adds a regression test to ensure starter examples parse cleanly and remain bindable when uncommented. |
.claude/rules/keymap.md |
Notes the starter-example rot class and references the regression test that pins it. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+89
to
+92
| let verb = text.prefix { !$0.isWhitespace } | ||
| // `<` skips the two verb-syntax lines, which state a grammar rather than an example. | ||
| guard verb == "map" || verb == "command", !text.contains("<") else { return nil } | ||
| return String(text) |
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
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.
the starter
keymap.conftold the user to uncommentmap cmd+shift+d toggle_split, and that line can never apply.cmd+shift+disBuiltinAction.dashboard's default, soresolveBuiltinOverridesdrops the override withchord conflicts with built-in 'dashboard'; map skipped. The example was fine when it was written and rotted whendashboardclaimed the chord.it is not a reserved chord, despite what the issue infers from
strings:isReservedMonitorChordcovers only Ctrl-Tab and Ctrl-1/2. The symptom is as reported, the cause is a plain built-in collision.changes:
mapexamples inConfigPaths.starterKeymapConf()now usecmd+shift+l, the chord README already usesagtermctl keymap listConfigPathsTests.starterKeymapExamplesApplyWhenUncommenteduncomments every startermapandcommandexample, requires each to parse without diagnostics, and pins the number of surviving chords at 4.commandexamples rot the same way:validateCommandsclears a custom shortcut a built-in has claimed.claude/rules/keymap.mdnotes the rot class and the test that pins itthe guard was checked against all three failure shapes: the shipped
cmd+shift+d, a built-in stealing acommandexample's chord, and an unparseable command chord (parseCommandLineabsorbs that one as shell text with no diagnostic, so the test counts surviving shortcuts rather than parsed commands).on the second half of the issue, the diagnostic text is already reachable.
agtermctl keymap listprints every diagnostic with its line number, Settings ▸ Key Mapping lists them, and a reload posts a banner. Putting them inkeymap.reload's own JSON is a control-API response change with skill and site mirrors, so it is not in this PR.Fix #405