Centralise TUI key bindings: one table, no duplicates, generated help #93

New issue

Closed

opened 2026-08-13 14:42:32 +00:00 by a73x · 0 comments

a73x commented

2026-08-13 14:42:32 +00:00

(Migrated from git.a73x.sh)

d7158619 is one symptom. The cause is that key handling is spread across 57 KeyCode:: arms — 44 in src/tui/state.rs, 13 in src/tui/events.rs — nested by mode and pane, with no single place that says what a key means where. Nothing stops the same key being bound twice in one context, and nothing checks that the help text matches reality.

Both failure modes are already live:

Same key, three meanings. c comments on a patch, opens Event History on an issue, and does nothing at all in the issue list pane (the binding is gated on pane == Pane::Detail and falls through silently otherwise).
Help text is hand-written and can lie. widgets.rs:1369-1385 composes footers as string literals — j/k:navigate Enter:detail Esc:back q:quit — maintained separately from the bindings they describe. This is the same defect class as a18e9b76, where a message named a command that did not exist: prose describing behaviour, kept by hand, drifting silently.

Wanted

One table mapping context → key → action, where context is the (ViewMode, Pane) pair that already governs dispatch. The match arms become a lookup; the table becomes the single readable answer to "what does this key do here".

A test that no context binds a key twice. That is the invariant the current structure cannot express, and it is cheap once the table exists. It should also flag a key bound in a context where it is unreachable.

Generate the footer and any help overlay from the table. Once the bindings are data, the help cannot drift — the same move cli_surface_test made by using clap's own command tree as the oracle for command citations rather than a hand-kept list. A ? overlay listing the current context's bindings becomes nearly free, and is the discoverability fix for a surface that has grown c/R/x/a/w/o/n/u/P/Ctrl-E/Ctrl-Y without ever telling anyone.

Rename the misleading internals while there. KeyAction::OpenCommitBrowser and ViewMode::CommitList draw a pane titled Event History. Reading those names is how I concluded the wrong thing about c before a terminal corrected me.

Note on scope

Do this before or with d7158619, not after. That issue adds issue-commenting and has to resolve the c collision — doing it against the spread-out structure means making the same class of decision by hand again, in the place that has already proved it cannot be done reliably by hand.

Migrated from git-collab issue 02d3eb34a9a166fb91c25ec1f77c313071a2b933.

Original author: a73x dev@a73x.sh.

Close reason: merged in patch 3ba51232

`d7158619` is one symptom. The cause is that key handling is spread across **57 `KeyCode::` arms** — 44 in `src/tui/state.rs`, 13 in `src/tui/events.rs` — nested by mode and pane, with no single place that says what a key means where. Nothing stops the same key being bound twice in one context, and nothing checks that the help text matches reality. Both failure modes are already live: - **Same key, three meanings.** `c` comments on a patch, opens Event History on an issue, and does nothing at all in the issue list pane (the binding is gated on `pane == Pane::Detail` and falls through silently otherwise). - **Help text is hand-written and can lie.** `widgets.rs:1369-1385` composes footers as string literals — ` j/k:navigate Enter:detail Esc:back q:quit` — maintained separately from the bindings they describe. This is the same defect class as `a18e9b76`, where a message named a command that did not exist: prose describing behaviour, kept by hand, drifting silently. ## Wanted **One table** mapping context → key → action, where context is the (`ViewMode`, `Pane`) pair that already governs dispatch. The match arms become a lookup; the table becomes the single readable answer to "what does this key do here". **A test that no context binds a key twice.** That is the invariant the current structure cannot express, and it is cheap once the table exists. It should also flag a key bound in a context where it is unreachable. **Generate the footer and any help overlay from the table.** Once the bindings are data, the help cannot drift — the same move `cli_surface_test` made by using clap's own command tree as the oracle for command citations rather than a hand-kept list. A `?` overlay listing the current context's bindings becomes nearly free, and is the discoverability fix for a surface that has grown `c`/`R`/`x`/`a`/`w`/`o`/`n`/`u`/`P`/`Ctrl-E`/`Ctrl-Y` without ever telling anyone. **Rename the misleading internals while there.** `KeyAction::OpenCommitBrowser` and `ViewMode::CommitList` draw a pane titled `Event History`. Reading those names is how I concluded the wrong thing about `c` before a terminal corrected me. ## Note on scope Do this **before or with** `d7158619`, not after. That issue adds issue-commenting and has to resolve the `c` collision — doing it against the spread-out structure means making the same class of decision by hand again, in the place that has already proved it cannot be done reliably by hand. --- Migrated from git-collab issue `02d3eb34a9a166fb91c25ec1f77c313071a2b933`. Original author: a73x <dev@a73x.sh>. Close reason: merged in patch 3ba51232