feat: restructure KeyMappings into nested config with form-specific k… #119

noe.trevino · 2026-02-21T21:51:41-06:00

noe.trevino commented

2026-02-21 21:51:41 -06:00

Reorganize flat KeyMappings struct into 7 nested sub-structs (navigation, tasks, kanban, projects, forms, pickers, general) so all keybindings come from config. Add form-specific ctrl+ shortcuts to FormKeys to avoid conflicting with text input. Rename navigation keys from prev_column/ next_column/prev_task/next_task to move_left/right/up/down since they are used across pickers, filter bar, and other contexts. Add example_config.yaml with all defaults for easy user setup.

Checklist

Read CONTRIBUTING.md
Ran tests (go test ./... -race) locally
Formatted and linted with gofmt -w . and golangci-lint run ./... respectively
Added tests if necessary (significant changes/complex logic)
Updated documentation if applicable
Updated README.md if applicable

Summary

Closes: #110

Reorganize flat KeyMappings struct into 7 nested sub-structs (navigation, tasks, kanban, projects, forms, pickers, general) so all keybindings come from config. Add form-specific ctrl+ shortcuts to FormKeys to avoid conflicting with text input. Rename navigation keys from prev_column/ next_column/prev_task/next_task to move_left/right/up/down since they are used across pickers, filter bar, and other contexts. Add example_config.yaml with all defaults for easy user setup. ## Checklist - [x] Read `CONTRIBUTING.md` - [x] Ran tests (`go test ./... -race`) locally - [x] Formatted and linted with `gofmt -w .` and `golangci-lint run ./...` respectively - [x] Added tests if necessary (significant changes/complex logic) - [x] Updated documentation if applicable - [x] Updated `README.md` if applicable ## Summary  Closes: #110

claude[bot] (Migrated from github.com) reviewed

2026-02-21 21:58:38 -06:00

internal/config/keymaps.go

Each of the 7 sub-struct applyDefaults() methods calls DefaultKeyMappings() independently, so a single KeyMappings.applyDefaults() invocation constructs 7 full KeyMappings values and discards 6/7ths of each. The fix is to compute the defaults once and pass the relevant slice down:

func (k *KeyMappings) applyDefaults() {
    d := DefaultKeyMappings()
    k.Navigation.applyDefaultsFrom(d.Navigation)
    k.Tasks.applyDefaultsFrom(d.Tasks)
    // ...
}

func (n *NavigationKeys) applyDefaultsFrom(defaults NavigationKeys) {
    if n.MoveLeft == "" { n.MoveLeft = defaults.MoveLeft }
    // ...
}

This keeps the logic per-struct while allocating only once.

Each of the 7 sub-struct `applyDefaults()` methods calls `DefaultKeyMappings()` independently, so a single `KeyMappings.applyDefaults()` invocation constructs 7 full `KeyMappings` values and discards 6/7ths of each. The fix is to compute the defaults once and pass the relevant slice down: ```go func (k *KeyMappings) applyDefaults() { d := DefaultKeyMappings() k.Navigation.applyDefaultsFrom(d.Navigation) k.Tasks.applyDefaultsFrom(d.Tasks) // ... } func (n *NavigationKeys) applyDefaultsFrom(defaults NavigationKeys) { if n.MoveLeft == "" { n.MoveLeft = defaults.MoveLeft } // ... } ``` This keeps the logic per-struct while allocating only once.

claude[bot] (Migrated from github.com) reviewed

2026-02-21 21:58:41 -06:00

internal/tui/update_notes.go

					
				@ -49,7 +51,7 @@ func (m Model) updateCommentEdit(msg tea.KeyMsg) (tea.Model, tea.Cmd) {

						}

The "n" alternative binding for "new comment" has been silently removed. The previous case "ctrl+n", "n": is now case km.Forms.OpenCommentsView: (defaults to ctrl+n only). updateCommentEdit operates on a comment list, not a text input field, so "n" did not conflict with text entry — its removal is a behavioral regression with no rationale in the PR description.

The `"n"` alternative binding for "new comment" has been silently removed. The previous `case "ctrl+n", "n":` is now `case km.Forms.OpenCommentsView:` (defaults to `ctrl+n` only). `updateCommentEdit` operates on a comment list, not a text input field, so `"n"` did not conflict with text entry — its removal is a behavioral regression with no rationale in the PR description.

claude[bot] (Migrated from github.com) reviewed

2026-02-21 21:58:44 -06:00

example_config.yaml

					
				@ -0,0 +34,4 @@

				    edit_due_date: "ctrl+d"

				    edit_parent_task: "p"

				    edit_child_task: "c"

				    edit_type: "ctrl+t"

tasks.edit_type is documented here as a normal-mode task shortcut (the section comment says "single-key shortcuts"), but km.Tasks.EditType has no case handler in handleNormalMode (update_normal.go). Only km.Forms.EditType is consumed (inside updateTaskForm). A user who sets this key will observe no effect from the kanban/list view. Either wire it up in normal mode or move it exclusively to the forms section.

`tasks.edit_type` is documented here as a normal-mode task shortcut (the section comment says "single-key shortcuts"), but `km.Tasks.EditType` has no `case` handler in `handleNormalMode` (`update_normal.go`). Only `km.Forms.EditType` is consumed (inside `updateTaskForm`). A user who sets this key will observe no effect from the kanban/list view. Either wire it up in normal mode or move it exclusively to the `forms` section.

claude[bot] commented

2026-02-21 21:59:01 -06:00

(Migrated from github.com)

Review Summary

The restructuring is clean and the intent is correct — breaking a flat 30-field struct into 7 contextual sub-structs is the right call, and the rename from prev_column/next_column/prev_task/next_task to directional names is an improvement. Three issues found:

applyDefaults efficiency (keymaps.go): DefaultKeyMappings() is invoked once per sub-struct, so a single config load constructs 7 full KeyMappings values. Minor in practice (config loads once), but structurally wrong.
Silent removal of 'n' binding (update_notes.go): The plain-n shortcut for new comment in comment-edit mode was dropped without explanation. That mode presents a list — no text input — so there is no conflict reason to remove it.
Dead config key (example_config.yaml, keymaps.go): tasks.edit_type is listed as a normal-mode single-key shortcut but km.Tasks.EditType has no handler in handleNormalMode. Only km.Forms.EditType is consumed. Setting this key does nothing from the kanban/list view.

Score: 7/10

## Review Summary The restructuring is clean and the intent is correct — breaking a flat 30-field struct into 7 contextual sub-structs is the right call, and the rename from prev_column/next_column/prev_task/next_task to directional names is an improvement. Three issues found: 1. **applyDefaults efficiency** (keymaps.go): DefaultKeyMappings() is invoked once per sub-struct, so a single config load constructs 7 full KeyMappings values. Minor in practice (config loads once), but structurally wrong. 2. **Silent removal of 'n' binding** (update_notes.go): The plain-n shortcut for new comment in comment-edit mode was dropped without explanation. That mode presents a list — no text input — so there is no conflict reason to remove it. 3. **Dead config key** (example_config.yaml, keymaps.go): tasks.edit_type is listed as a normal-mode single-key shortcut but km.Tasks.EditType has no handler in handleNormalMode. Only km.Forms.EditType is consumed. Setting this key does nothing from the kanban/list view. **Score: 7/10**

Sign in to join this conversation.

No reviewers