diff --git a/docs/example-config.toml b/docs/example-config.toml index 8ea7c1c..7c27c3c 100644 --- a/docs/example-config.toml +++ b/docs/example-config.toml @@ -321,7 +321,9 @@ literal = false # the help overlay's scroll keys and its own `/` filter, the shared `/` # list-filter editing sub-state (annotation list / staging / accepted panel # / switcher, spec 12), Compose, the commit-message modal, the diff-view `/` -# search input, the fuzzy file finder, and the two Project Search focuses) — +# search input, the fuzzy file finder, the two Project Search focuses, the +# imported-thread overlay, the submit-review modal, the post-submit result +# view, and the finished-review cleanup modal) — # has its own `[keys.]` table, with the # identical grammar and merge semantics as `[keys.diff]`/`[keys.panel]` # above: `action-name = ""` or `= ["", ...]`, an override @@ -329,13 +331,18 @@ literal = false # its defaults, `= []` unbinds it, and a same-table collision is won by # the override (with a warning). # -# Free-text modes (Compose, the commit-message modal, Search, Finder, and -# both Project Search focuses) type printable characters into a buffer or -# query — that character insertion is never an action and can't be bound; -# only the *control* keys listed below (Enter/Esc/arrows/Backspace/...) -# are actions. -# -# The fourteen mode names and their complete action lists, each showing its +# Free-text modes (Compose, the commit-message modal, Search, Finder, both +# Project Search focuses, and the submit-review modal's summary field) type +# printable characters into a buffer or query — that character insertion is +# never an action and can't be bound; only the *control* keys listed below +# (Enter/Esc/arrows/Backspace/...) are actions. The table is consulted +# before the character-insert fallback, so binding a control action of one +# of those modes to a bare printable key is accepted but takes that +# character away from typing — e.g. `[keys.submit-forge] cancel = "q"` means +# you can no longer type `q` into the review summary. Prefer a modified key +# (`ctrl-`/`alt-`) or a named key in a free-text mode. +# +# The eighteen mode names and their complete action lists, each showing its # default key(s): # # [keys.list] # Annotation list panel (`a`) @@ -404,6 +411,10 @@ literal = false # jump-to-bottom = ["G", "end"] # Jump to bottom # confirm = "enter" # Confirm the highlighted row # enter-filter = "/" # Filter (fuzzy, narrows the active tab) +# toggle-all-commits = "a" # Commits tab: ahead-of-base <-> full recent-HEAD log +# cleanup-finished-reviews = "X" # Pull Requests tab: clean up finished reviews +# refresh = "r" # Pull Requests tab: re-fetch the PR list +# pr-details = "i" # Pull Requests tab: read the PR description # close = "esc" # Close # # [keys.help] # Help overlay scroll/close/filter-open keys @@ -501,6 +512,39 @@ literal = false # toggle-whole-word = "alt-w" # Toggle whole-word matching # toggle-literal = "alt-r" # Toggle regex / literal matching # +# [keys.thread-view] # Imported PR comment thread overlay (spec 13) +# scroll-down = ["j", "down"] # Scroll conversation down +# scroll-up = ["k", "up"] # Scroll conversation up +# reply = "r" # Reply to this thread +# close = ["q", "esc"] # Close the thread overlay +# +# [keys.submit-forge] # Submit-review modal (`U` in a PR review, spec 13) +# confirm = "enter" # Submit the review (publishes to the forge) +# cancel = "esc" # Cancel — close this modal, send nothing +# verdict-next = "tab" # Next verdict (comment / approve / request changes) +# verdict-prev = "shift-tab" # Previous verdict +# scroll-down = "down" # Scroll the batch preview down +# scroll-up = "up" # Scroll the batch preview up +# page-down = "pagedown" # Scroll the batch preview down a page +# page-up = "pageup" # Scroll the batch preview up a page +# delete-char = "backspace" # Delete summary character +# compose-summary = "ctrl-e" # Edit the summary in the composer (multi-line) +# +# [keys.submit-result] # Post-submit result view (shown when a submit stops early) +# retry = "U" # Submit again — retry everything that didn't land +# dismiss = ["enter", "esc", "q"] # Dismiss the result view +# scroll-down = ["j", "down"] # Scroll the outcome list down +# scroll-up = ["k", "up"] # Scroll the outcome list up +# page-down = "pagedown" # Scroll the outcome list down a page +# page-up = "pageup" # Scroll the outcome list up a page +# +# [keys.cleanup-reviews] # Finished-review cleanup confirm modal (launcher `X`) +# move-down = ["j", "down"] # Move the highlight down +# move-up = ["k", "up"] # Move the highlight up +# toggle = "space" # Toggle the highlighted entry's selection +# confirm = ["enter", "y"] # Delete the selected finished reviews (worktree, branch, saved state) +# cancel = ["esc", "n"] # Cancel — close this modal, delete nothing +# # Example: remap the staging panel's unstage key to `x` and the switcher's # confirm key to `l` (also still one of ToggleTab's defaults, so this would # collide in a real config — shown separately here for two independent, diff --git a/src/config/keys.rs b/src/config/keys.rs index 768c894..195ba97 100644 --- a/src/config/keys.rs +++ b/src/config/keys.rs @@ -180,6 +180,10 @@ const MODAL_MODE_NAMES: &[&str] = &[ "finder", "project-search-input", "project-search-results", + "thread-view", + "submit-forge", + "submit-result", + "cleanup-reviews", "filter-edit", ]; @@ -195,7 +199,7 @@ const MODAL_MODE_NAMES: &[&str] = &[ /// `diff`/`panel`/`global`, keyed by mode name (one of [`MODAL_MODE_NAMES`]) /// — a single map rather than one field per mode, since /// `crate::ui::modal_keys_config` (the edge module resolving these) already -/// needs one generic merge function reusable across all thirteen modes. +/// needs one generic merge function reusable across every mode. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct KeysConfig { pub diff: BTreeMap>, diff --git a/src/ui/footer_tests.rs b/src/ui/footer_tests.rs index a4d3ce9..bdb883f 100644 --- a/src/ui/footer_tests.rs +++ b/src/ui/footer_tests.rs @@ -362,6 +362,52 @@ fn switcher_mode_hints() { ); } +/// End-to-end for the `[keys.]` -> footer path: the strip is built +/// from the *effective* tables, so a remapped modal key reaches the footer +/// with no per-mode wiring. Uses `Mode::ThreadView` as the representative +/// PR-flow modal — a strip built from the compiled-in default table instead +/// would still print `r`. +#[test] +fn a_remapped_modal_key_shows_up_in_that_modes_hint_strip() { + let mut keys = crate::config::KeysConfig::default(); + let mut table = std::collections::BTreeMap::new(); + table.insert( + "reply".to_string(), + vec![crate::config::keys::KeySeqSpec::One( + crate::config::keys::ChordSpec { + code: KeyCode::Char('a'), + mods: KeyModifiers::NONE, + }, + )], + ); + keys.modal.insert("thread-view".to_string(), table); + let (modal_keys, warnings) = crate::ui::modal_keys_config::effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let km = Keymap::default_map(); + let entries = build_hints( + Mode::ThreadView, + FooterFlags { + staging_allowed: true, + code_intel_allowed: true, + push_publishes: false, + viewing_commit: false, + help_open: false, + project_search_focus: SearchFocus::Input, + review_session: true, + web_target: None, + }, + None, + &km, + &modal_keys, + ); + let reply = entries + .iter() + .find(|e| e.label == "reply") + .expect("the thread overlay's reply hint"); + assert_eq!(reply.key, "a"); +} + #[test] fn search_mode_has_no_hint_strip() { let km = Keymap::default_map(); diff --git a/src/ui/forge_submit_tests.rs b/src/ui/forge_submit_tests.rs index a47737f..8733958 100644 --- a/src/ui/forge_submit_tests.rs +++ b/src/ui/forge_submit_tests.rs @@ -617,6 +617,39 @@ fn typing_a_summary_clears_the_hint_and_lets_request_changes_confirm() { assert!(app.submit_forge.is_none()); } +/// The accepted trade for making `[keys.submit-forge]` remappable: the table +/// is consulted before the char-insert fallback, so a control action bound to +/// a bare printable key takes that character away from summary typing. Pins +/// the ordering — flipping it would silently un-remap every letter-keyed +/// submit-forge override. +#[test] +fn a_submit_forge_action_remapped_onto_a_letter_shadows_summary_typing() { + let mut keys = crate::config::KeysConfig::default(); + let mut table = std::collections::BTreeMap::new(); + table.insert( + "cancel".to_string(), + vec![crate::config::keys::KeySeqSpec::One( + crate::config::keys::ChordSpec { + code: KeyCode::Char('q'), + mods: KeyModifiers::NONE, + }, + )], + ); + keys.modal.insert("submit-forge".to_string(), table); + let (modal_keys, warnings) = crate::ui::modal_keys_config::effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + + let mut app = github_review_app(&["src/a.rs"]); + app.modal_keys = modal_keys; + app.open_submit_forge(); + handle_submit_forge_key( + &mut app, + KeyEvent::new(KeyCode::Char('q'), KeyModifiers::NONE), + ); + assert_eq!(app.mode, Mode::Normal, "`q` must cancel, not type"); + assert!(app.submit_forge.is_none()); +} + // -- confirm on the fake path sends nothing (no live backend) ---------------- // -- scrollable preview + overflow markers ----------------------------------- diff --git a/src/ui/modal_keys.rs b/src/ui/modal_keys.rs index 35427bd..827ed0f 100644 --- a/src/ui/modal_keys.rs +++ b/src/ui/modal_keys.rs @@ -1173,8 +1173,7 @@ pub(super) static END_REVIEW_KEYS: LazyLock>> /// What a key does in the imported-thread overlay /// ([`super::app::Mode::ThreadView`]): scroll the read-only conversation, -/// draft a reply to it, or close. Not config-remappable yet — see -/// [`THREAD_VIEW_KEYS`] and the module doc. +/// draft a reply to it, or close. Remappable through `[keys.thread-view]`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum ThreadViewAction { /// Scroll the conversation down one line (see @@ -1189,9 +1188,30 @@ pub(super) enum ThreadViewAction { Close, } +pub(super) fn thread_view_action_name(action: ThreadViewAction) -> &'static str { + match action { + ThreadViewAction::ScrollDown => "scroll-down", + ThreadViewAction::ScrollUp => "scroll-up", + ThreadViewAction::Reply => "reply", + ThreadViewAction::Close => "close", + } +} + +pub(super) fn thread_view_action_from_name(name: &str) -> Option { + Some(match name { + "scroll-down" => ThreadViewAction::ScrollDown, + "scroll-up" => ThreadViewAction::ScrollUp, + "reply" => ThreadViewAction::Reply, + "close" => ThreadViewAction::Close, + _ => return None, + }) +} + /// The thread-overlay control keys (`j`/`k`/arrow scroll plus `Esc`/`q` /// close), for the help overlay, footer strip, and -/// [`super::modes::handle_thread_view_key`]'s dispatch. +/// [`super::modes::handle_thread_view_key`]'s dispatch. Defaults only — the +/// effective table is this plus any `[keys.thread-view]` config override (see +/// `super::modal_keys_config`). pub(super) static THREAD_VIEW_KEYS: LazyLock>> = LazyLock::new(|| { vec![ @@ -1324,8 +1344,11 @@ pub(super) static PR_DESCRIPTION_KEYS: LazyLock &'static str { + match action { + SubmitForgeAction::Confirm => "confirm", + SubmitForgeAction::Cancel => "cancel", + SubmitForgeAction::VerdictNext => "verdict-next", + SubmitForgeAction::VerdictPrev => "verdict-prev", + SubmitForgeAction::ScrollDown => "scroll-down", + SubmitForgeAction::ScrollUp => "scroll-up", + SubmitForgeAction::PageDown => "page-down", + SubmitForgeAction::PageUp => "page-up", + SubmitForgeAction::DeleteChar => "delete-char", + SubmitForgeAction::ComposeSummary => "compose-summary", + } +} + +pub(super) fn submit_forge_action_from_name(name: &str) -> Option { + Some(match name { + "confirm" => SubmitForgeAction::Confirm, + "cancel" => SubmitForgeAction::Cancel, + "verdict-next" => SubmitForgeAction::VerdictNext, + "verdict-prev" => SubmitForgeAction::VerdictPrev, + "scroll-down" => SubmitForgeAction::ScrollDown, + "scroll-up" => SubmitForgeAction::ScrollUp, + "page-down" => SubmitForgeAction::PageDown, + "page-up" => SubmitForgeAction::PageUp, + "delete-char" => SubmitForgeAction::DeleteChar, + "compose-summary" => SubmitForgeAction::ComposeSummary, + _ => return None, + }) +} + /// The submit-review modal's control-key table, for the help overlay, footer -/// strip, and [`super::modes::handle_submit_forge_key`]'s dispatch. +/// strip, and [`super::modes::handle_submit_forge_key`]'s dispatch. Defaults +/// only — the effective table is this plus any `[keys.submit-forge]` config +/// override (see `super::modal_keys_config`). pub(super) static SUBMIT_FORGE_KEYS: LazyLock>> = LazyLock::new(|| { vec![ @@ -1443,8 +1499,8 @@ pub(super) static SUBMIT_FORGE_KEYS: LazyLock &'static str { + match action { + SubmitResultAction::ScrollDown => "scroll-down", + SubmitResultAction::ScrollUp => "scroll-up", + SubmitResultAction::PageDown => "page-down", + SubmitResultAction::PageUp => "page-up", + SubmitResultAction::Dismiss => "dismiss", + SubmitResultAction::Retry => "retry", + } +} + +pub(super) fn submit_result_action_from_name(name: &str) -> Option { + Some(match name { + "scroll-down" => SubmitResultAction::ScrollDown, + "scroll-up" => SubmitResultAction::ScrollUp, + "page-down" => SubmitResultAction::PageDown, + "page-up" => SubmitResultAction::PageUp, + "dismiss" => SubmitResultAction::Dismiss, + "retry" => SubmitResultAction::Retry, + _ => return None, + }) +} + /// The result-modal control keys, for the help overlay, footer strip, and -/// [`super::modes::handle_submit_result_key`]'s dispatch. +/// [`super::modes::handle_submit_result_key`]'s dispatch. Defaults only — the +/// effective table is this plus any `[keys.submit-result]` config override +/// (see `super::modal_keys_config`). pub(super) static SUBMIT_RESULT_KEYS: LazyLock>> = LazyLock::new(|| { vec![ @@ -1631,8 +1712,8 @@ pub(super) static RESTORE_KEYS: LazyLock>> = Laz /// toggles the highlighted entry's selection (all checked by default), and /// confirm/cancel gate the batch. Confirm deletes only the *selected* /// reviews' worktree, branch, and state entry (a no-op with nothing -/// selected); cancel mutates nothing. Not config-remappable yet — see module -/// doc. +/// selected); cancel mutates nothing. Remappable through +/// `[keys.cleanup-reviews]`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum CleanupReviewsAction { /// Moves the highlight down one entry (see @@ -1653,12 +1734,35 @@ pub(super) enum CleanupReviewsAction { Cancel, } +pub(super) fn cleanup_reviews_action_name(action: CleanupReviewsAction) -> &'static str { + match action { + CleanupReviewsAction::MoveDown => "move-down", + CleanupReviewsAction::MoveUp => "move-up", + CleanupReviewsAction::Toggle => "toggle", + CleanupReviewsAction::Confirm => "confirm", + CleanupReviewsAction::Cancel => "cancel", + } +} + +pub(super) fn cleanup_reviews_action_from_name(name: &str) -> Option { + Some(match name { + "move-down" => CleanupReviewsAction::MoveDown, + "move-up" => CleanupReviewsAction::MoveUp, + "toggle" => CleanupReviewsAction::Toggle, + "confirm" => CleanupReviewsAction::Confirm, + "cancel" => CleanupReviewsAction::Cancel, + _ => return None, + }) +} + /// The cleanup confirm modal's key table, for the help overlay, footer strip, /// and [`super::modes::handle_cleanup_reviews_key`]'s dispatch. The /// confirm/cancel rows mirror [`CONFIRM_REMOTE_OP_KEYS`]' binary-gate shape; /// the move pair mirrors [`SWITCHER_KEYS`]' — only `MoveDown` carries the /// footer hint, since its label ("j / Down") already reads as a compound key /// display and merging `MoveUp`'s in would double the " / " separators. +/// Defaults only — the effective table is this plus any +/// `[keys.cleanup-reviews]` config override (see `super::modal_keys_config`). pub(super) static CLEANUP_REVIEWS_KEYS: LazyLock>> = LazyLock::new(|| { vec![ @@ -3117,9 +3221,9 @@ pub(super) static SEARCH_HINTS: LazyLock>> = Lazy // -- Effective (post-config-override) modal tables -------------------------- /// The canonical `[keys.]` table names, in -/// the same order [`ModalKeymaps`]'s fields are declared. One table per modal -/// mode currently defined in this module; adding a fourteenth mode means -/// adding both a field here and a name here, which +/// the same order [`ModalKeymaps`]'s fields are declared. One name per +/// config-remappable modal mode; adding another means adding both a field +/// here and a name here, which /// `crate::config::keys::KeysConfig::from_value`'s parallel hardcoded list /// must also gain (that module can't import this one — see its layering /// note — so `crate::ui::modal_keys_config`'s tests cross-check the two @@ -3141,6 +3245,10 @@ pub(super) const MODAL_MODE_NAMES: &[&str] = &[ "finder", "project-search-input", "project-search-results", + "thread-view", + "submit-forge", + "submit-result", + "cleanup-reviews", "filter-edit", ]; @@ -3177,20 +3285,16 @@ pub struct ModalKeymaps { /// The pull/push confirm modal. Not config-remappable yet — see /// [`CONFIRM_REMOTE_OP_KEYS`]. pub(super) confirm_remote_op: Vec>, - /// The imported-thread overlay. Not config-remappable yet — see - /// [`THREAD_VIEW_KEYS`]. + /// The imported-thread overlay (`[keys.thread-view]`). pub(super) thread_view: Vec>, /// The read-only PR description overlay. Not config-remappable yet — see /// [`PR_DESCRIPTION_KEYS`]. pub(super) pr_description: Vec>, - /// The submit-review modal. Not config-remappable yet — see - /// [`SUBMIT_FORGE_KEYS`]. + /// The submit-review modal (`[keys.submit-forge]`). pub(super) submit_forge: Vec>, - /// The post-submit result modal. Not config-remappable yet — see - /// [`SUBMIT_RESULT_KEYS`]. + /// The post-submit result modal (`[keys.submit-result]`). pub(super) submit_result: Vec>, - /// The finished-review cleanup confirm modal. Not config-remappable yet — - /// see [`CLEANUP_REVIEWS_KEYS`]. + /// The finished-review cleanup confirm modal (`[keys.cleanup-reviews]`). pub(super) cleanup_reviews: Vec>, /// The restore confirm modal. Not config-remappable yet — see /// [`RESTORE_KEYS`]. @@ -3286,8 +3390,8 @@ mod tests { } } - /// One test over all fourteen mode tables: each mode's action names must - /// be unique and round-trip back to the same action. + /// One test over every config-remappable mode table: each mode's action + /// names must be unique and round-trip back to the same action. #[test] fn every_modal_action_name_mapping_is_total_and_bijective() { assert_action_names_are_total_and_bijective( @@ -3360,6 +3464,26 @@ mod tests { project_search_results_action_name, project_search_results_action_from_name, ); + assert_action_names_are_total_and_bijective( + &THREAD_VIEW_KEYS, + thread_view_action_name, + thread_view_action_from_name, + ); + assert_action_names_are_total_and_bijective( + &SUBMIT_FORGE_KEYS, + submit_forge_action_name, + submit_forge_action_from_name, + ); + assert_action_names_are_total_and_bijective( + &SUBMIT_RESULT_KEYS, + submit_result_action_name, + submit_result_action_from_name, + ); + assert_action_names_are_total_and_bijective( + &CLEANUP_REVIEWS_KEYS, + cleanup_reviews_action_name, + cleanup_reviews_action_from_name, + ); } fn sample_file() -> FileDiff { diff --git a/src/ui/modal_keys_config.rs b/src/ui/modal_keys_config.rs index fc4cb1f..78b59fb 100644 --- a/src/ui/modal_keys_config.rs +++ b/src/ui/modal_keys_config.rs @@ -21,6 +21,15 @@ //! is resolved user-wins, with one [`ConfigWarning`] recorded. An unknown //! action name, or a two-chord key sequence (modal tables never supported //! `gd`-style sequences), is itself an invalid value. +//! +//! **Free-text modes** (Compose, the commit-message modal, Search, Finder, +//! Project Search, and the submit-review modal's summary field) consult their +//! table before the printable-char fallback, so binding a bare printable key +//! to one of their control actions takes that character away from typing. +//! That's allowed rather than rejected — it's the same trade every free-text +//! mode here has always accepted, and the consequence is documented in +//! `docs/example-config.toml` rather than encoded as a special-case +//! validation rule. use std::collections::BTreeMap; @@ -146,6 +155,38 @@ pub(super) fn effective_modal_keys( modal_keys::project_search_results_action_from_name, &mut warnings, ), + thread_view: apply_modal_overrides( + modal_keys::THREAD_VIEW_KEYS.clone(), + overrides_for("thread-view"), + "keys.thread-view", + modal_keys::thread_view_action_name, + modal_keys::thread_view_action_from_name, + &mut warnings, + ), + submit_forge: apply_modal_overrides( + modal_keys::SUBMIT_FORGE_KEYS.clone(), + overrides_for("submit-forge"), + "keys.submit-forge", + modal_keys::submit_forge_action_name, + modal_keys::submit_forge_action_from_name, + &mut warnings, + ), + submit_result: apply_modal_overrides( + modal_keys::SUBMIT_RESULT_KEYS.clone(), + overrides_for("submit-result"), + "keys.submit-result", + modal_keys::submit_result_action_name, + modal_keys::submit_result_action_from_name, + &mut warnings, + ), + cleanup_reviews: apply_modal_overrides( + modal_keys::CLEANUP_REVIEWS_KEYS.clone(), + overrides_for("cleanup-reviews"), + "keys.cleanup-reviews", + modal_keys::cleanup_reviews_action_name, + modal_keys::cleanup_reviews_action_from_name, + &mut warnings, + ), filter_edit: apply_modal_overrides( modal_keys::FILTER_EDIT_KEYS.clone(), overrides_for("filter-edit"), @@ -160,20 +201,15 @@ pub(super) fn effective_modal_keys( end_review: modal_keys::END_REVIEW_KEYS.clone(), accepted_panel: modal_keys::ACCEPTED_PANEL_KEYS.clone(), confirm_remote_op: modal_keys::CONFIRM_REMOTE_OP_KEYS.clone(), - thread_view: modal_keys::THREAD_VIEW_KEYS.clone(), pr_description: modal_keys::PR_DESCRIPTION_KEYS.clone(), - submit_forge: modal_keys::SUBMIT_FORGE_KEYS.clone(), - submit_result: modal_keys::SUBMIT_RESULT_KEYS.clone(), - cleanup_reviews: modal_keys::CLEANUP_REVIEWS_KEYS.clone(), restore: modal_keys::RESTORE_KEYS.clone(), }; // Every mode name the config actually provided a table for that isn't - // one of the thirteen known modes was already flagged (unknown key) at - // parse time in `crate::config::keys::KeysConfig::from_value`, which - // hardcodes the same thirteen names — see that module's `MODAL_MODE_NAMES` - // doc and this module's tests for the cross-check that the two lists - // agree. + // one of the known modes was already flagged (unknown key) at parse time + // in `crate::config::keys::KeysConfig::from_value`, which hardcodes the + // same list — see that module's `MODAL_MODE_NAMES` doc and this module's + // tests for the cross-check that the two lists agree. (keymaps, warnings) } diff --git a/src/ui/modal_keys_config_tests.rs b/src/ui/modal_keys_config_tests.rs index 116af2b..5e01ff4 100644 --- a/src/ui/modal_keys_config_tests.rs +++ b/src/ui/modal_keys_config_tests.rs @@ -35,7 +35,7 @@ fn modal_mode_names_match_config_keys_hardcoded_list() { // and the count must match exactly (a name accepted by config that the // ui list doesn't know about would still slip past this check only if // it also appeared in `effective_modal_keys`'s match below, which is - // exhaustive over the thirteen `ModalKeymaps` fields — so a name drifting + // exhaustive over the `ModalKeymaps` fields — so a name drifting // out of sync in either direction fails this test or fails to compile). let toml = modal_keys::MODAL_MODE_NAMES .iter() @@ -88,6 +88,13 @@ fn no_overrides_yields_every_default_table_unchanged() { &modal_keys::PROJECT_SEARCH_RESULTS_HINTS, ); same(&effective.filter_edit, &modal_keys::FILTER_EDIT_KEYS); + same(&effective.thread_view, &modal_keys::THREAD_VIEW_KEYS); + same(&effective.submit_forge, &modal_keys::SUBMIT_FORGE_KEYS); + same(&effective.submit_result, &modal_keys::SUBMIT_RESULT_KEYS); + same( + &effective.cleanup_reviews, + &modal_keys::CLEANUP_REVIEWS_KEYS, + ); } // -- Replace: an action named in config gets exactly the listed keys -------- @@ -337,6 +344,197 @@ fn overriding_a_compose_control_action_leaves_the_rest_of_the_table_intact() { assert_eq!(effective.compose.len(), modal_keys::COMPOSE_HINTS.len()); } +// -- PR-flow modals: each section is wired to its own table ------------------ +// +// One test per newly wired section, because the defect each catches is +// distinct: that *this* mode's `[keys.]` table never reached +// `effective_modal_keys` (the remap silently does nothing). Each asserts the +// three observable consequences of a wired section — the new key drives the +// action, the displaced default no longer does, and the row's `key_label` +// (the exact string `super::help`'s overlay and `super::footer`'s strip +// print) shows the new key. + +#[test] +fn remapping_a_thread_view_action_moves_the_key_and_its_displayed_label() { + let keys = keys_with( + "thread-view", + "reply", + one(KeyCode::Char('a'), KeyModifiers::NONE), + ); + let (effective, warnings) = effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + assert_eq!( + modal_keys::resolve( + &effective.thread_view, + KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE) + ), + Some(modal_keys::ThreadViewAction::Reply) + ); + assert_eq!( + modal_keys::resolve( + &effective.thread_view, + KeyEvent::new(KeyCode::Char('r'), KeyModifiers::NONE) + ), + None, + "the displaced default must no longer reply" + ); + let row = effective + .thread_view + .iter() + .find(|b| b.action == modal_keys::ThreadViewAction::Reply) + .expect("reply row"); + assert_eq!(row.key_label(), "a"); +} + +#[test] +fn remapping_a_submit_forge_action_moves_the_key_and_its_displayed_label() { + let keys = keys_with( + "submit-forge", + "compose-summary", + one(KeyCode::Char('y'), KeyModifiers::CONTROL), + ); + let (effective, warnings) = effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + assert_eq!( + modal_keys::resolve( + &effective.submit_forge, + KeyEvent::new(KeyCode::Char('y'), KeyModifiers::CONTROL) + ), + Some(modal_keys::SubmitForgeAction::ComposeSummary) + ); + assert_eq!( + modal_keys::resolve( + &effective.submit_forge, + KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL) + ), + None, + "the displaced default must no longer open the composer" + ); + let row = effective + .submit_forge + .iter() + .find(|b| b.action == modal_keys::SubmitForgeAction::ComposeSummary) + .expect("compose-summary row"); + assert_eq!(row.key_label(), "Ctrl-y"); +} + +#[test] +fn remapping_a_submit_result_action_moves_the_key_and_its_displayed_label() { + let keys = keys_with( + "submit-result", + "retry", + one(KeyCode::Char('R'), KeyModifiers::NONE), + ); + let (effective, warnings) = effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + assert_eq!( + modal_keys::resolve( + &effective.submit_result, + KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE) + ), + Some(modal_keys::SubmitResultAction::Retry) + ); + assert_eq!( + modal_keys::resolve( + &effective.submit_result, + KeyEvent::new(KeyCode::Char('U'), KeyModifiers::NONE) + ), + None, + "the displaced default must no longer retry" + ); + let row = effective + .submit_result + .iter() + .find(|b| b.action == modal_keys::SubmitResultAction::Retry) + .expect("retry row"); + assert_eq!(row.key_label(), "R"); +} + +#[test] +fn remapping_a_cleanup_reviews_action_moves_the_key_and_its_displayed_label() { + let keys = keys_with( + "cleanup-reviews", + "toggle", + one(KeyCode::Char('t'), KeyModifiers::NONE), + ); + let (effective, warnings) = effective_modal_keys(&keys); + assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}"); + assert_eq!( + modal_keys::resolve( + &effective.cleanup_reviews, + KeyEvent::new(KeyCode::Char('t'), KeyModifiers::NONE) + ), + Some(modal_keys::CleanupReviewsAction::Toggle) + ); + assert_eq!( + modal_keys::resolve( + &effective.cleanup_reviews, + KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE) + ), + None, + "the displaced default must no longer toggle" + ); + let row = effective + .cleanup_reviews + .iter() + .find(|b| b.action == modal_keys::CleanupReviewsAction::Toggle) + .expect("toggle row"); + assert_eq!(row.key_label(), "t"); +} + +// -- Invalid values in the PR-flow sections surface the launcher's warnings -- + +/// A section that never reached [`effective_modal_keys`] fails differently: +/// `KeysConfig::from_value` would reject the whole `[keys.]` header as +/// an unknown key, so neither of the per-action warnings below could appear. +/// Table-driven over the four sections since the assertion is on the warning +/// shape, not on any mode's action types. +#[test] +fn each_pr_flow_section_reports_bad_values_like_the_launcher_section_does() { + for (mode, real_action) in [ + ("thread-view", "reply"), + ("submit-forge", "confirm"), + ("submit-result", "retry"), + ("cleanup-reviews", "toggle"), + ] { + // An unparseable key string is rejected at config-parse time. + let raw: toml::Table = format!("[{mode}]\n{real_action} = \"not-a-key\"\n") + .parse() + .expect("valid TOML"); + let mut warnings = Vec::new(); + let cfg = KeysConfig::from_value(toml::Value::Table(raw), &mut warnings); + assert_eq!(warnings.len(), 1, "[keys.{mode}]: {warnings:?}"); + match &warnings[0] { + crate::config::ConfigWarning::InvalidValue { section, key, .. } => { + assert_eq!(section, &format!("keys.{mode}")); + assert_eq!(key, real_action); + } + other => panic!("[keys.{mode}]: expected InvalidValue, got {other:?}"), + } + assert!( + cfg.modal.contains_key(mode), + "[keys.{mode}] must be a recognized section, not an unknown key" + ); + + // An unknown action name is rejected at merge time, naming the same + // section. + let keys = keys_with( + mode, + "not-a-real-action", + one(KeyCode::Esc, KeyModifiers::NONE), + ); + let (_effective, warnings) = effective_modal_keys(&keys); + assert_eq!(warnings.len(), 1, "[keys.{mode}]: {warnings:?}"); + match &warnings[0] { + crate::config::ConfigWarning::InvalidValue { section, key, .. } => { + assert_eq!(section, &format!("keys.{mode}")); + assert_eq!(key, "not-a-real-action"); + } + other => panic!("[keys.{mode}]: expected InvalidValue, got {other:?}"), + } + } +} + // -- docs/example-config.toml completeness ----------------------------------- // // The `[keys.]` sections are entirely commented out (like @@ -417,14 +615,27 @@ fn parse_doc_modal_blocks(text: &str) -> DocModalBlocks { /// Asserts `mode`'s doc block (from `docs/example-config.toml`) names /// exactly the modal action space `from_name` resolves, and that every key -/// string it lists parses under the grammar. +/// string it lists parses under the grammar. The expected action set is read +/// off `table` (the mode's default table) rather than hand-listed at the call +/// site: every variant appears in its table by construction — the same +/// argument `modal_keys`'s bijectivity test relies on — and a hand-kept +/// parallel list can go stale in step with the doc it's meant to police, +/// which is exactly how `[keys.review-launcher]`'s two newest actions went +/// undocumented. fn assert_doc_block_matches( blocks: &DocModalBlocks, mode: &str, - all_actions: &[A], + table: &[ModalBinding], name_of: fn(A) -> &'static str, from_name: fn(&str) -> Option, ) { + let mut all_actions: Vec = Vec::new(); + for b in table { + if !all_actions.contains(&b.action) { + all_actions.push(b.action); + } + } + let all_actions = &all_actions[..]; // The first block named `mode` is the canonical documentation block; // the doc's trailing "Example:" section deliberately reuses // `staging`/`switcher` for a live one-line demo and isn't meant to be @@ -478,248 +689,127 @@ fn example_config_documents_every_modal_action_exactly_once() { assert_doc_block_matches( &blocks, "list", - &[ - ListAction::MoveDown, - ListAction::MoveUp, - ListAction::HalfPageDown, - ListAction::HalfPageUp, - ListAction::FullPageDown, - ListAction::FullPageUp, - ListAction::JumpToTop, - ListAction::JumpToBottom, - ListAction::Jump, - ListAction::Edit, - ListAction::Delete, - ListAction::EnterFilter, - ListAction::Close, - ], + &LIST_KEYS, list_action_name, list_action_from_name, ); assert_doc_block_matches( &blocks, "staging", - &[ - StagingAction::MoveDown, - StagingAction::MoveUp, - StagingAction::HalfPageDown, - StagingAction::HalfPageUp, - StagingAction::FullPageDown, - StagingAction::FullPageUp, - StagingAction::JumpToTop, - StagingAction::JumpToBottom, - StagingAction::Unstage, - StagingAction::EnterFilter, - StagingAction::Close, - ], + &STAGING_KEYS, staging_action_name, staging_action_from_name, ); assert_doc_block_matches( &blocks, "peek", - &[ - PeekAction::MoveDown, - PeekAction::MoveUp, - PeekAction::HalfPageDown, - PeekAction::HalfPageUp, - PeekAction::FullPageDown, - PeekAction::FullPageUp, - PeekAction::JumpToTop, - PeekAction::JumpToBottom, - PeekAction::Enter, - PeekAction::Close, - ], + &PEEK_KEYS, peek_action_name, peek_action_from_name, ); assert_doc_block_matches( &blocks, "switcher", - &[ - SwitcherAction::ToggleTab, - SwitcherAction::MoveDown, - SwitcherAction::MoveUp, - SwitcherAction::HalfPageDown, - SwitcherAction::HalfPageUp, - SwitcherAction::FullPageDown, - SwitcherAction::FullPageUp, - SwitcherAction::JumpToTop, - SwitcherAction::JumpToBottom, - SwitcherAction::Confirm, - SwitcherAction::EnterFilter, - SwitcherAction::Close, - ], + &SWITCHER_KEYS, switcher_action_name, switcher_action_from_name, ); assert_doc_block_matches( &blocks, "review-launcher", - &[ - LauncherAction::ToggleTab, - LauncherAction::MoveDown, - LauncherAction::MoveUp, - LauncherAction::HalfPageDown, - LauncherAction::HalfPageUp, - LauncherAction::FullPageDown, - LauncherAction::FullPageUp, - LauncherAction::JumpToTop, - LauncherAction::JumpToBottom, - LauncherAction::Confirm, - LauncherAction::EnterFilter, - LauncherAction::Close, - ], + &REVIEW_LAUNCHER_KEYS, launcher_action_name, launcher_action_from_name, ); assert_doc_block_matches( &blocks, "help", - &[ - HelpAction::Close, - HelpAction::ScrollDown, - HelpAction::ScrollUp, - HelpAction::PageDown, - HelpAction::PageUp, - HelpAction::Top, - HelpAction::Bottom, - HelpAction::Search, - HelpAction::NextTab, - HelpAction::PrevTab, - ], + &HELP_KEYS, help_action_name, help_action_from_name, ); assert_doc_block_matches( &blocks, "help-search", - &[ - HelpSearchAction::Lock, - HelpSearchAction::Clear, - HelpSearchAction::DeleteChar, - ], + &HELP_SEARCH_HINTS, help_search_action_name, help_search_action_from_name, ); assert_doc_block_matches( &blocks, "filter-edit", - &[ - FilterEditAction::Lock, - FilterEditAction::Clear, - FilterEditAction::DeleteChar, - ], + &FILTER_EDIT_KEYS, filter_edit_action_name, filter_edit_action_from_name, ); assert_doc_block_matches( &blocks, "compose", - &[ - ComposeAction::Cancel, - ComposeAction::Submit, - ComposeAction::CycleClassification, - ComposeAction::Edit(BufferEditAction::Newline), - ComposeAction::Edit(BufferEditAction::MoveLeft), - ComposeAction::Edit(BufferEditAction::MoveRight), - ComposeAction::Edit(BufferEditAction::MoveUp), - ComposeAction::Edit(BufferEditAction::MoveDown), - ComposeAction::Edit(BufferEditAction::WordLeft), - ComposeAction::Edit(BufferEditAction::WordRight), - ComposeAction::Edit(BufferEditAction::LineStart), - ComposeAction::Edit(BufferEditAction::LineEnd), - ComposeAction::Edit(BufferEditAction::DocStart), - ComposeAction::Edit(BufferEditAction::DocEnd), - ComposeAction::Edit(BufferEditAction::DeleteBack), - ComposeAction::Edit(BufferEditAction::DeleteForward), - ComposeAction::Edit(BufferEditAction::DeleteWordBack), - ComposeAction::Edit(BufferEditAction::DeleteWordForward), - ], + &COMPOSE_HINTS, compose_action_name, compose_action_from_name, ); assert_doc_block_matches( &blocks, "commit-message", - &[ - CommitMessageAction::Cancel, - CommitMessageAction::Submit, - CommitMessageAction::Edit(BufferEditAction::Newline), - CommitMessageAction::Edit(BufferEditAction::MoveLeft), - CommitMessageAction::Edit(BufferEditAction::MoveRight), - CommitMessageAction::Edit(BufferEditAction::MoveUp), - CommitMessageAction::Edit(BufferEditAction::MoveDown), - CommitMessageAction::Edit(BufferEditAction::WordLeft), - CommitMessageAction::Edit(BufferEditAction::WordRight), - CommitMessageAction::Edit(BufferEditAction::LineStart), - CommitMessageAction::Edit(BufferEditAction::LineEnd), - CommitMessageAction::Edit(BufferEditAction::DocStart), - CommitMessageAction::Edit(BufferEditAction::DocEnd), - CommitMessageAction::Edit(BufferEditAction::DeleteBack), - CommitMessageAction::Edit(BufferEditAction::DeleteForward), - CommitMessageAction::Edit(BufferEditAction::DeleteWordBack), - CommitMessageAction::Edit(BufferEditAction::DeleteWordForward), - ], + &COMMIT_MESSAGE_HINTS, commit_message_action_name, commit_message_action_from_name, ); assert_doc_block_matches( &blocks, "search", - &[ - SearchAction::Confirm, - SearchAction::Cancel, - SearchAction::DeleteChar, - ], + &SEARCH_HINTS, search_action_name, search_action_from_name, ); assert_doc_block_matches( &blocks, "finder", - &[ - FinderAction::MoveUp, - FinderAction::MoveDown, - FinderAction::Open, - FinderAction::Close, - FinderAction::DeleteChar, - ], + &FINDER_HINTS, finder_action_name, finder_action_from_name, ); assert_doc_block_matches( &blocks, "project-search-input", - &[ - ProjectSearchInputAction::MoveUp, - ProjectSearchInputAction::MoveDown, - ProjectSearchInputAction::Open, - ProjectSearchInputAction::FocusResults, - ProjectSearchInputAction::ToggleFocus, - ProjectSearchInputAction::DeleteChar, - ProjectSearchInputAction::ToggleCase, - ProjectSearchInputAction::ToggleWholeWord, - ProjectSearchInputAction::ToggleLiteral, - ], + &PROJECT_SEARCH_INPUT_HINTS, project_search_input_action_name, project_search_input_action_from_name, ); assert_doc_block_matches( &blocks, "project-search-results", - &[ - ProjectSearchResultsAction::EditQuery, - ProjectSearchResultsAction::Close, - ProjectSearchResultsAction::MoveUp, - ProjectSearchResultsAction::MoveDown, - ProjectSearchResultsAction::Open, - ProjectSearchResultsAction::ToggleFocus, - ProjectSearchResultsAction::ToggleCase, - ProjectSearchResultsAction::ToggleWholeWord, - ProjectSearchResultsAction::ToggleLiteral, - ], + &PROJECT_SEARCH_RESULTS_HINTS, project_search_results_action_name, project_search_results_action_from_name, ); + assert_doc_block_matches( + &blocks, + "thread-view", + &THREAD_VIEW_KEYS, + thread_view_action_name, + thread_view_action_from_name, + ); + assert_doc_block_matches( + &blocks, + "submit-forge", + &SUBMIT_FORGE_KEYS, + submit_forge_action_name, + submit_forge_action_from_name, + ); + assert_doc_block_matches( + &blocks, + "submit-result", + &SUBMIT_RESULT_KEYS, + submit_result_action_name, + submit_result_action_from_name, + ); + assert_doc_block_matches( + &blocks, + "cleanup-reviews", + &CLEANUP_REVIEWS_KEYS, + cleanup_reviews_action_name, + cleanup_reviews_action_from_name, + ); }