diff --git a/crates/improvise-command/src/command/cmd/registry.rs b/crates/improvise-command/src/command/cmd/registry.rs index ead11991..f0e9e05b 100644 --- a/crates/improvise-command/src/command/cmd/registry.rs +++ b/crates/improvise-command/src/command/cmd/registry.rs @@ -4,23 +4,11 @@ use crate::ui::effect::Panel; /// Decode a mode-name string (as supplied by `enter-mode`/`edit-or-drill` /// keymap bindings) into an `AppMode`. +/// The table itself is `AppMode::from_name`, beside `AppMode` in the +/// protocol crate: the spellings are keymap vocabulary and the keymap is +/// shared, so the browser client decodes `enter-mode` from the same list. fn parse_mode_name(s: &str) -> Result { - match s { - "normal" => Ok(AppMode::Normal), - "help" => Ok(AppMode::Help), - "formula-panel" => Ok(AppMode::FormulaPanel), - "category-panel" => Ok(AppMode::CategoryPanel), - "view-panel" => Ok(AppMode::ViewPanel), - "tile-select" => Ok(AppMode::TileSelect), - "command" => Ok(AppMode::command_mode()), - "category-add" => Ok(AppMode::category_add()), - "editing" => Ok(AppMode::editing()), - "records-normal" => Ok(AppMode::RecordsNormal), - "records-editing" => Ok(AppMode::records_editing()), - "formula-edit" => Ok(AppMode::formula_edit()), - "export-prompt" => Ok(AppMode::export_prompt()), - other => Err(format!("Unknown mode: {other}")), - } + AppMode::from_name(s).ok_or_else(|| format!("Unknown mode: {s}")) } use super::cell::*; diff --git a/crates/improvise-command/src/command/keymap.rs b/crates/improvise-command/src/command/keymap.rs index 0c983f27..ba21c2bc 100644 --- a/crates/improvise-command/src/command/keymap.rs +++ b/crates/improvise-command/src/command/keymap.rs @@ -1,469 +1,89 @@ -use std::collections::HashMap; -use std::fmt; +//! The half of the keymap that needs a command registry. +//! +//! The **table** — `KeyPattern`, `Binding`, `Keymap`, `ModeKey`, `TextEntry`, +//! `PanelKeymap` and `Keymaps::default_keymaps()` — lives in +//! `improvise_protocol::keymap`, so the browser client looks up the same +//! bindings the terminal does rather than mirroring them. Read that module's +//! docs for why. +//! +//! What is here is what a thin client cannot have: resolving a bound *name* +//! into `Effect`s through a [`CmdRegistry`], and the `SetTransientPrefix` +//! effect that arms a prefix key. + use std::sync::Arc; -use crate::command::key::{KeyCode, KeyModifiers}; - -use crate::ui::app::{AppMode, Panel}; -use crate::ui::effect::Effect; +use crate::ui::app::App; +use crate::ui::effect::{Effect, EffectKind}; use super::cmd::{self, CmdContext, CmdRegistry}; -/// Format a KeyCode as a short human-readable label for which-key display. -fn format_key_label(code: &KeyCode) -> String { - match code { - KeyCode::Char(c) => c.to_string(), - KeyCode::Enter => "Enter".to_string(), - KeyCode::Esc => "Esc".to_string(), - KeyCode::Tab => "Tab".to_string(), - KeyCode::BackTab => "S-Tab".to_string(), - KeyCode::Backspace => "BS".to_string(), - KeyCode::Delete => "Del".to_string(), - KeyCode::Left => "←".to_string(), - KeyCode::Right => "→".to_string(), - KeyCode::Up => "↑".to_string(), - KeyCode::Down => "↓".to_string(), - KeyCode::Home => "Home".to_string(), - KeyCode::End => "End".to_string(), - KeyCode::PageUp => "PgUp".to_string(), - KeyCode::PageDown => "PgDn".to_string(), - KeyCode::F(n) => format!("F{n}"), - _ => format!("{code:?}"), - } -} -// `cmd` module imported for `default_registry()` in default_keymaps() - -/// A key pattern that can be matched against a KeyEvent. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum KeyPattern { - /// Single key with modifiers - Key(KeyCode, KeyModifiers), - /// Matches any Char key (for text-entry modes). - AnyChar, - /// Matches any key at all (lowest priority fallback). - Any, -} - -/// Identifies which mode a binding applies to. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum ModeKey { - Normal, - Help, - FormulaPanel, - CategoryPanel, - ViewPanel, - TileSelect, - Editing, - FormulaEdit, - CategoryAdd, - ItemAdd, - ExportPrompt, - CommandMode, - SearchMode, - ImportWizard, - RecordsNormal, - RecordsEditing, -} - -impl ModeKey { - pub fn from_app_mode(mode: &AppMode, search_mode: bool) -> Option { - match mode { - AppMode::Normal if search_mode => Some(ModeKey::SearchMode), - AppMode::Normal => Some(ModeKey::Normal), - AppMode::Help => Some(ModeKey::Help), - AppMode::FormulaPanel => Some(ModeKey::FormulaPanel), - AppMode::CategoryPanel => Some(ModeKey::CategoryPanel), - AppMode::ViewPanel => Some(ModeKey::ViewPanel), - AppMode::TileSelect => Some(ModeKey::TileSelect), - AppMode::Editing { .. } => Some(ModeKey::Editing), - AppMode::FormulaEdit { .. } => Some(ModeKey::FormulaEdit), - AppMode::CategoryAdd { .. } => Some(ModeKey::CategoryAdd), - AppMode::ItemAdd { .. } => Some(ModeKey::ItemAdd), - AppMode::ExportPrompt { .. } => Some(ModeKey::ExportPrompt), - AppMode::CommandMode { .. } => Some(ModeKey::CommandMode), - AppMode::ImportWizard => Some(ModeKey::ImportWizard), - AppMode::RecordsNormal => Some(ModeKey::RecordsNormal), - AppMode::RecordsEditing { .. } => Some(ModeKey::RecordsEditing), - // Exhaustive on purpose: a new AppMode without a keymap silently - // swallows every keypress, so the compiler must flag it here. - AppMode::Quit => None, - } - } -} - -/// What a key binding resolves to. -#[derive(Debug, Clone)] -pub enum Binding { - /// A command name + arguments, looked up in the registry at dispatch time. - Cmd { - name: &'static str, - args: Vec, - }, - /// A prefix sub-keymap (Emacs-style). - Prefix(Arc), - /// A sequence of commands executed in order, concatenating their effects. - Sequence(Vec<(&'static str, Vec)>), -} - -/// A keymap maps key patterns to bindings (command names or prefix sub-keymaps). -/// Supports Emacs-style parent keymap inheritance: if a key is not found in this -/// keymap, lookup falls through to the parent. -#[derive(Default)] -pub struct Keymap { - bindings: HashMap, - parent: Option>, -} - -impl fmt::Debug for Keymap { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Keymap") - .field("binding_count", &self.bindings.len()) - .finish() - } -} - -impl Keymap { - pub fn new() -> Self { - Self { - bindings: HashMap::new(), - parent: None, - } - } - - /// Create a keymap that inherits from a parent. Keys not found here - /// fall through to the parent (Emacs-style keymap inheritance). - pub fn with_parent(parent: Arc) -> Self { - Self { - bindings: HashMap::new(), - parent: Some(parent), - } - } - - /// Bind a key to a command name (no args). - pub fn bind(&mut self, key: KeyCode, mods: KeyModifiers, name: &'static str) { - self.bindings.insert( - KeyPattern::Key(key, mods), - Binding::Cmd { name, args: vec![] }, - ); - } - - /// Bind a key to a command name with arguments. - pub fn bind_args( - &mut self, - key: KeyCode, - mods: KeyModifiers, - name: &'static str, - args: Vec, - ) { - self.bindings - .insert(KeyPattern::Key(key, mods), Binding::Cmd { name, args }); - } - - /// Bind a prefix key that activates a sub-keymap. - pub fn bind_prefix(&mut self, key: KeyCode, mods: KeyModifiers, sub: Arc) { - self.bindings - .insert(KeyPattern::Key(key, mods), Binding::Prefix(sub)); - } - - /// Bind a key to a sequence of commands (executed in order). - pub fn bind_seq( - &mut self, - key: KeyCode, - mods: KeyModifiers, - steps: Vec<(&'static str, Vec)>, - ) { - self.bindings - .insert(KeyPattern::Key(key, mods), Binding::Sequence(steps)); - } - - /// Bind a catch-all for any Char key. - pub fn bind_any_char(&mut self, name: &'static str, args: Vec) { - self.bindings - .insert(KeyPattern::AnyChar, Binding::Cmd { name, args }); - } - - /// Bind a catch-all for any key at all. - pub fn bind_any(&mut self, name: &'static str) { - self.bindings - .insert(KeyPattern::Any, Binding::Cmd { name, args: vec![] }); - } - - /// Return human-readable hints for all concrete bindings in this keymap. - /// Used by the which-key popup to show available completions after a prefix key. - pub fn binding_hints(&self) -> Vec<(String, &'static str)> { - let mut hints: Vec<(String, &'static str)> = self - .bindings - .iter() - .filter_map(|(pattern, binding)| { - let label = match pattern { - KeyPattern::Key(code, _) => format_key_label(code), - KeyPattern::AnyChar | KeyPattern::Any => return None, - }; - let name = match binding { - Binding::Cmd { name, .. } => *name, - Binding::Prefix(_) => return None, - Binding::Sequence(steps) => { - if let Some((name, _)) = steps.first() { - *name - } else { - return None; - } - } - }; - Some((label, name)) - }) - .collect(); - hints.sort_by(|a, b| a.0.cmp(&b.0)); - hints - } - - /// Look up the binding for a key in this keymap's own bindings. - fn lookup_local(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> { - self.bindings - .get(&KeyPattern::Key(key, mods)) - .or_else(|| { - // Retry Char keys without modifiers (shift is implicit in the char) - if matches!(key, KeyCode::Char(_)) && mods != KeyModifiers::NONE { - self.bindings.get(&KeyPattern::Key(key, KeyModifiers::NONE)) - } else { - None - } - }) - .or_else(|| { - if matches!(key, KeyCode::Char(_)) { - self.bindings.get(&KeyPattern::AnyChar) - } else { - None - } - }) - .or_else(|| self.bindings.get(&KeyPattern::Any)) - } - - /// Look up the binding for a key, falling through to parent keymaps. - pub fn lookup(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> { - self.lookup_local(key, mods) - .or_else(|| self.parent.as_ref().and_then(|p| p.lookup(key, mods))) - } - - /// Dispatch a key: look up binding, resolve through registry, return effects. - pub fn dispatch( - &self, - registry: &CmdRegistry, - ctx: &CmdContext, - key: KeyCode, - mods: KeyModifiers, - ) -> Option>> { - let binding = self.lookup(key, mods)?; - match binding { - Binding::Cmd { name, args } => { - let cmd = registry.interactive(name, args, ctx).ok()?; - Some(cmd.execute(ctx)) - } - Binding::Prefix(_) => Some(vec![Box::new(SetTransientPrefix(key, mods))]), - Binding::Sequence(steps) => { - let mut effects: Vec> = Vec::new(); - for (name, args) in steps { - let cmd = registry.interactive(name, args, ctx).ok()?; - effects.extend(cmd.execute(ctx)); - } - Some(effects) - } - } - } -} - -/// Builder for a minibuffer mode's keymap. -/// -/// Every text-entry mode has the same spine: characters append to one named -/// buffer, Backspace pops from it, and Esc clears it before leaving for some -/// other mode. Only the buffer name, the exit mode and the commit keys differ. -/// Building them through here means a new text mode cannot accidentally type -/// into one buffer while clearing another, or drop the clear entirely — which -/// would leave the next entry pre-filled with the last one. -/// -/// `SearchMode` deliberately does not use this: it has no commit key, and its -/// query is owned by `exit-search-mode` rather than by `clear-buffer`. -pub struct TextEntry { - buffer: String, - km: Keymap, -} - -impl TextEntry { - /// The keymap for `mode`, returning to `exit` on Esc. The buffer comes - /// off the mode's own `MinibufferConfig`, so the buffer the prompt - /// describes and the buffer the keys edit are the same string by - /// construction rather than by agreement (improvise-k8h). - pub fn new(mode: &AppMode, exit: &'static str) -> Self { - Self::build(buffer_of(mode).to_string(), exit, Keymap::new()) - } - - /// As [`TextEntry::new`], but inheriting unbound keys from `parent`. - pub fn with_parent(mode: &AppMode, exit: &'static str, parent: Arc) -> Self { - Self::build( - buffer_of(mode).to_string(), - exit, - Keymap::with_parent(parent), - ) - } - - fn build(buffer: String, exit: &'static str, mut km: Keymap) -> Self { - let none = KeyModifiers::NONE; - km.bind_seq( - KeyCode::Esc, - none, - vec![ - ("clear-buffer", vec![buffer.clone()]), - ("enter-mode", vec![exit.into()]), - ], - ); - km.bind_args(KeyCode::Backspace, none, "pop-char", vec![buffer.clone()]); - km.bind_any_char("append-char", vec![buffer.clone()]); - Self { buffer, km } - } - - /// Bind `key` to "run `cmd`, then clear the buffer". The two halves are - /// welded together here so no caller can bind a commit without its clear. - pub fn commit(mut self, key: KeyCode, cmd: &'static str, args: Vec) -> Self { - self.km.bind_seq( - key, - KeyModifiers::NONE, - vec![(cmd, args), ("clear-buffer", vec![self.buffer.clone()])], - ); - self - } - - /// Replace a spine binding — used only by CommandMode, whose Backspace - /// exits the mode on an empty buffer instead of popping a character. - pub fn rebind(mut self, key: KeyCode, cmd: &'static str) -> Self { - self.km.bind(key, KeyModifiers::NONE, cmd); - self - } - - pub fn finish(self) -> Keymap { - self.km - } -} - -/// Builder for a side panel's keymap. -/// -/// The three panels share a spine: Esc and Tab leave for Normal, the arrows -/// and j/k move that panel's cursor, every panel's toggle key is bound so -/// focus can move between them, and `:` opens command mode. Only the keys -/// that act on the panel's contents differ — and they differ in shape, not -/// just in command name (Category's `n` takes an argument, View has no `a`, -/// Formula has no Enter), so they arrive through chained methods rather than -/// a fixed-arity signature (improvise-4pg). -pub struct PanelKeymap { - km: Keymap, -} - -impl PanelKeymap { - /// The spine every panel shares, with the cursor keys pointed at `panel`. - pub fn new(panel: Panel) -> Self { - let none = KeyModifiers::NONE; - let mut km = Keymap::new(); - km.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); - km.bind_args(KeyCode::Tab, none, "enter-mode", vec!["normal".into()]); - for (keys, delta) in [ - ([KeyCode::Up, KeyCode::Char('k')], "-1"), - ([KeyCode::Down, KeyCode::Char('j')], "1"), - ] { - for key in keys { - km.bind_args( - key, - none, - "move-panel-cursor", - vec![panel.name().into(), delta.into()], - ); - } - } - bind_panel_toggles(&mut km); - km.bind_args( - KeyCode::Char(':'), - none, - "enter-mode", - vec!["command".into()], - ); - Self { km } - } - - /// Bind a key that acts on this panel's contents. - pub fn bind(mut self, key: KeyCode, cmd: &'static str) -> Self { - self.km.bind(key, KeyModifiers::NONE, cmd); - self - } - - /// As [`PanelKeymap::bind`], for a command that takes arguments. - pub fn bind_args(mut self, key: KeyCode, cmd: &'static str, args: Vec) -> Self { - self.km.bind_args(key, KeyModifiers::NONE, cmd, args); - self - } - - /// `d` and Delete both remove whatever the cursor is on. Binding them - /// together means a panel cannot offer one without the other. - pub fn delete(self, cmd: &'static str) -> Self { - self.bind(KeyCode::Char('d'), cmd) - .bind(KeyCode::Delete, cmd) - } - - pub fn finish(self) -> Keymap { - self.km - } -} - -/// Bind every panel's toggle key. Used by the panel keymaps and by Normal, -/// so a new panel reaches all four with one `Panel::ALL` entry. -fn bind_panel_toggles(km: &mut Keymap) { - for panel in Panel::ALL { - km.bind_args( - KeyCode::Char(panel.key()), - KeyModifiers::NONE, - "toggle-panel-and-focus", - vec![panel.name().into()], - ); - } -} - -/// The buffer a text-entry mode types into. Every caller is one of the seven -/// modes that carry a `MinibufferConfig`; a mode without one has no minibuffer -/// to build a keymap for. -fn buffer_of(mode: &AppMode) -> &str { - &mode - .minibuffer() - .unwrap_or_else(|| panic!("{} has no minibuffer to type into", mode.label())) - .buffer_key -} +use improvise_protocol::key::{KeyCode, KeyModifiers}; +pub use improvise_protocol::keymap::{ + Binding, KeyPattern, Keymap, Keymaps, ModeKey, PanelKeymap, TextEntry, +}; /// Effect that arms a prefix key, so the next keypress is dispatched against /// that prefix's sub-keymap. /// /// It records the *address* of the prefix — the key that opened it — rather /// than the sub-keymap itself. `KeymapSet` already owns every keymap, so -/// holding an `Arc` in `ViewState` would only put a -/// `improvise-command` type into what is meant to become portable, shared -/// state (improvise-jc8). +/// holding an `Arc` in `ViewState` would only put a non-portable type +/// into what is meant to become portable, shared state (improvise-jc8). #[derive(Debug)] pub struct SetTransientPrefix(pub KeyCode, pub KeyModifiers); impl Effect for SetTransientPrefix { - fn apply(&self, app: &mut crate::ui::app::App) { + fn apply(&self, app: &mut App) { app.view_state.transient_prefix = Some((self.0, self.1)); } - fn kind(&self) -> crate::ui::effect::EffectKind { - crate::ui::effect::EffectKind::ViewOnly + fn kind(&self) -> EffectKind { + EffectKind::ViewOnly } } -/// Maps modes to their root keymaps + owns the command registry. +/// Resolve one bound name against the registry and run it. +fn run( + registry: &CmdRegistry, + ctx: &CmdContext, + name: &str, + args: &[String], +) -> Option>> { + let cmd = registry.interactive(name, args, ctx).ok()?; + Some(cmd.execute(ctx)) +} + +/// Turn a binding into effects. The one place a keymap lookup becomes a +/// mutation, and the reason `Binding` itself can be registry-free data. +fn effects_of( + binding: &Binding, + registry: &CmdRegistry, + ctx: &CmdContext, + key: KeyCode, + mods: KeyModifiers, +) -> Option>> { + match binding { + Binding::Cmd { name, args } => run(registry, ctx, name, args), + Binding::Prefix(_) => Some(vec![Box::new(SetTransientPrefix(key, mods))]), + Binding::Sequence(steps) => { + let mut effects: Vec> = Vec::new(); + for (name, args) in steps { + effects.extend(run(registry, ctx, name, args)?); + } + Some(effects) + } + } +} + +/// The shared keymap table plus the registry that gives its names meaning. pub struct KeymapSet { - mode_maps: HashMap>, + maps: Keymaps, registry: CmdRegistry, } impl KeymapSet { pub fn new(registry: CmdRegistry) -> Self { Self { - mode_maps: HashMap::new(), + maps: Keymaps::new(), registry, } } @@ -472,8 +92,13 @@ impl KeymapSet { &self.registry } + /// The bindings themselves — the same table the browser client reads. + pub fn maps(&self) -> &Keymaps { + &self.maps + } + pub fn insert(&mut self, mode: ModeKey, keymap: Arc) { - self.mode_maps.insert(mode, keymap); + self.maps.insert(mode, keymap); } /// Dispatch a key event: returns effects if a binding matched. @@ -484,26 +109,18 @@ impl KeymapSet { mods: KeyModifiers, ) -> Option>> { let mode_key = ModeKey::from_app_mode(ctx.mode, ctx.search_mode)?; - let keymap = self.mode_maps.get(&mode_key)?; - keymap.dispatch(&self.registry, ctx, key, mods) + let binding = self.maps.lookup(&mode_key, key, mods)?; + effects_of(binding, &self.registry, ctx, key, mods) } - /// The sub-keymap a prefix key opens, in `mode`. `None` when the mode - /// has no such binding, or when the binding is not a prefix. - /// - /// Resolution deliberately goes through `lookup`, which walks parent - /// keymaps: `RecordsNormal` has no prefixes of its own and reaches - /// Normal's through inheritance. + /// The sub-keymap a prefix key opens, in `mode`. pub fn prefix_keymap( &self, mode: &ModeKey, key: KeyCode, mods: KeyModifiers, ) -> Option> { - match self.mode_maps.get(mode)?.lookup(key, mods)? { - Binding::Prefix(sub) => Some(sub.clone()), - Binding::Cmd { .. } | Binding::Sequence(_) => None, - } + self.maps.prefix_keymap(mode, key, mods) } /// Dispatch against a specific keymap (for transient/prefix keymaps). @@ -514,443 +131,25 @@ impl KeymapSet { key: KeyCode, mods: KeyModifiers, ) -> Option>> { - keymap.dispatch(&self.registry, ctx, key, mods) + let binding = keymap.lookup(key, mods)?; + effects_of(binding, &self.registry, ctx, key, mods) } /// Build the default keymap set with all bindings. pub fn default_keymaps() -> Self { - let registry = cmd::default_registry(); - let mut set = Self::new(registry); - let none = KeyModifiers::NONE; - let ctrl = KeyModifiers::CONTROL; - - // ── Normal mode ────────────────────────────────────────────────── - let mut normal = Keymap::new(); - - // Navigation - for (key, dr, dc) in [ - (KeyCode::Up, -1, 0), - (KeyCode::Down, 1, 0), - (KeyCode::Left, 0, -1), - (KeyCode::Right, 0, 1), - ] { - normal.bind_args( - key, - none, - "move-selection", - vec![dr.to_string(), dc.to_string()], - ); + Self { + maps: Keymaps::default_keymaps(), + registry: cmd::default_registry(), } - for (ch, dr, dc) in [('k', -1, 0), ('j', 1, 0), ('h', 0, -1), ('l', 0, 1)] { - normal.bind_args( - KeyCode::Char(ch), - none, - "move-selection", - vec![dr.to_string(), dc.to_string()], - ); - } - - // Jump to boundaries - normal.bind(KeyCode::Char('G'), none, "jump-last-row"); - normal.bind(KeyCode::Char('0'), none, "jump-first-col"); - normal.bind(KeyCode::Char('$'), none, "jump-last-col"); - normal.bind(KeyCode::Home, none, "jump-first-col"); - normal.bind(KeyCode::End, none, "jump-last-col"); - - // Scroll - normal.bind_args(KeyCode::Char('d'), ctrl, "scroll-rows", vec!["5".into()]); - normal.bind_args(KeyCode::Char('u'), ctrl, "scroll-rows", vec!["-5".into()]); - normal.bind_args(KeyCode::PageDown, none, "page-scroll", vec!["1".into()]); - normal.bind_args(KeyCode::PageUp, none, "page-scroll", vec!["-1".into()]); - - // Cell operations - normal.bind(KeyCode::Char('x'), none, "clear-cell"); - normal.bind(KeyCode::Char('p'), none, "paste"); - - // View - normal.bind(KeyCode::Char('t'), none, "transpose"); - - // Mode changes - normal.bind(KeyCode::Char('q'), ctrl, "force-quit"); - normal.bind_args( - KeyCode::Char(':'), - none, - "enter-mode", - vec!["command".into()], - ); - normal.bind(KeyCode::Char('/'), none, "search"); - normal.bind(KeyCode::Char('s'), ctrl, "save"); - normal.bind(KeyCode::F(1), none, "enter-mode"); - normal.bind_args(KeyCode::F(1), none, "enter-mode", vec!["help".into()]); - normal.bind_args(KeyCode::Char('?'), none, "enter-mode", vec!["help".into()]); - - // Panel toggles — the same set the panel keymaps bind, so a new - // panel reaches Normal and all three panels at once. - bind_panel_toggles(&mut normal); - normal.bind_args( - KeyCode::Char('f'), - ctrl, - "toggle-panel-visibility", - vec!["formula".into()], - ); - normal.bind_args( - KeyCode::Char('c'), - ctrl, - "toggle-panel-visibility", - vec!["category".into()], - ); - normal.bind_args( - KeyCode::Char('v'), - ctrl, - "toggle-panel-visibility", - vec!["view".into()], - ); - normal.bind(KeyCode::Tab, none, "cycle-panel-focus"); - - // Editing entry — i/a drill into aggregated cells, else edit. - // The mode arg controls which editing mode is entered; records-normal - // overrides these to "records-editing" via its own bindings. - normal.bind_args( - KeyCode::Char('i'), - none, - "edit-or-drill", - vec!["editing".into()], - ); - normal.bind_args( - KeyCode::Char('a'), - none, - "edit-or-drill", - vec!["editing".into()], - ); - normal.bind(KeyCode::Enter, none, "enter-advance"); - normal.bind_args( - KeyCode::Char('e'), - ctrl, - "enter-mode", - vec!["export-prompt".into()], - ); - - // Search / category add - normal.bind_args( - KeyCode::Char('n'), - none, - "search-navigate", - vec!["forward".into()], - ); - normal.bind(KeyCode::Char('N'), none, "search-or-category-add"); - - // Page navigation - normal.bind(KeyCode::Char(']'), none, "page-next"); - normal.bind(KeyCode::Char('['), none, "page-prev"); - - // Group / hide - normal.bind(KeyCode::Char('z'), none, "toggle-group-under-cursor"); - normal.bind(KeyCode::Char('H'), none, "hide-selected-row-item"); - - // Drill into aggregated cell / view history - normal.bind(KeyCode::Char('>'), none, "drill-into-cell"); - normal.bind(KeyCode::Char('<'), none, "view-back"); - - // Records mode toggle and prune toggle - normal.bind(KeyCode::Char('R'), none, "toggle-records-mode"); - normal.bind(KeyCode::Char('P'), none, "toggle-prune-empty"); - - // Tile select - normal.bind(KeyCode::Char('T'), none, "enter-tile-select"); - normal.bind(KeyCode::Left, ctrl, "enter-tile-select"); - normal.bind(KeyCode::Right, ctrl, "enter-tile-select"); - normal.bind(KeyCode::Up, ctrl, "enter-tile-select"); - normal.bind(KeyCode::Down, ctrl, "enter-tile-select"); - - // Prefix keys - let mut g_map = Keymap::new(); - g_map.bind(KeyCode::Char('g'), none, "jump-first-row"); - g_map.bind(KeyCode::Char('z'), none, "toggle-col-group-under-cursor"); - normal.bind_prefix(KeyCode::Char('g'), none, Arc::new(g_map)); - - let mut y_map = Keymap::new(); - y_map.bind(KeyCode::Char('y'), none, "yank"); - normal.bind_prefix(KeyCode::Char('y'), none, Arc::new(y_map)); - - let mut z_map = Keymap::new(); - z_map.bind(KeyCode::Char('Z'), none, "wq"); - normal.bind_prefix(KeyCode::Char('Z'), none, Arc::new(z_map)); - - let normal = Arc::new(normal); - set.insert(ModeKey::Normal, normal.clone()); - - // ── Records normal mode (inherits from normal) ──────────────────── - let mut rn = Keymap::with_parent(normal); - // Override i/a so the edit branch produces records-editing mode - // instead of inheriting the normal-mode "editing" arg. - rn.bind_args( - KeyCode::Char('i'), - none, - "edit-or-drill", - vec!["records-editing".into()], - ); - rn.bind_args( - KeyCode::Char('a'), - none, - "edit-or-drill", - vec!["records-editing".into()], - ); - rn.bind_seq( - KeyCode::Char('o'), - none, - vec![ - ("add-record-row", vec![]), - ("enter-edit-at-cursor", vec!["records-editing".into()]), - ], - ); - set.insert(ModeKey::RecordsNormal, Arc::new(rn)); - - // ── Help mode ──────────────────────────────────────────────────── - let mut help = Keymap::new(); - help.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); - help.bind_args( - KeyCode::Char('q'), - none, - "enter-mode", - vec!["normal".into()], - ); - // Allow entering command mode from Help (important on empty-model launch) - help.bind_args( - KeyCode::Char(':'), - none, - "enter-mode", - vec!["command".into()], - ); - // Page navigation - help.bind(KeyCode::Right, none, "help-page-next"); - help.bind(KeyCode::Char('l'), none, "help-page-next"); - help.bind(KeyCode::Char('n'), none, "help-page-next"); - help.bind(KeyCode::Tab, none, "help-page-next"); - help.bind(KeyCode::Left, none, "help-page-prev"); - help.bind(KeyCode::Char('h'), none, "help-page-prev"); - help.bind(KeyCode::Char('p'), none, "help-page-prev"); - help.bind(KeyCode::BackTab, none, "help-page-prev"); - set.insert(ModeKey::Help, Arc::new(help)); - - // ── Panels ─────────────────────────────────────────────────────── - set.insert( - ModeKey::FormulaPanel, - Arc::new( - PanelKeymap::new(Panel::Formula) - .bind_args( - KeyCode::Char('a'), - "enter-mode", - vec!["formula-edit".into()], - ) - .bind_args( - KeyCode::Char('n'), - "enter-mode", - vec!["formula-edit".into()], - ) - .bind_args( - KeyCode::Char('o'), - "enter-mode", - vec!["formula-edit".into()], - ) - .delete("delete-formula-at-cursor") - .finish(), - ), - ); - - set.insert( - ModeKey::CategoryPanel, - Arc::new( - PanelKeymap::new(Panel::Category) - .bind(KeyCode::Enter, "filter-to-item") - .bind(KeyCode::Char(' '), "cycle-axis-at-cursor") - .bind_args( - KeyCode::Char('n'), - "enter-mode", - vec!["category-add".into()], - ) - .bind(KeyCode::Char('a'), "open-item-add-at-cursor") - .bind(KeyCode::Char('o'), "open-item-add-at-cursor") - .delete("delete-category-at-cursor") - .finish(), - ), - ); - - set.insert( - ModeKey::ViewPanel, - Arc::new( - PanelKeymap::new(Panel::View) - .bind(KeyCode::Enter, "switch-view-at-cursor") - .bind(KeyCode::Char('n'), "create-and-switch-view") - .bind(KeyCode::Char('o'), "create-and-switch-view") - .delete("delete-view-at-cursor") - .finish(), - ), - ); - - // ── Tile select ────────────────────────────────────────────────── - let mut ts = Keymap::new(); - ts.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); - ts.bind_args(KeyCode::Tab, none, "enter-mode", vec!["normal".into()]); - ts.bind_args(KeyCode::Left, none, "move-tile-cursor", vec!["-1".into()]); - ts.bind_args( - KeyCode::Char('h'), - none, - "move-tile-cursor", - vec!["-1".into()], - ); - ts.bind_args(KeyCode::Right, none, "move-tile-cursor", vec!["1".into()]); - ts.bind_args( - KeyCode::Char('l'), - none, - "move-tile-cursor", - vec!["1".into()], - ); - ts.bind(KeyCode::Enter, none, "cycle-axis-for-tile"); - ts.bind(KeyCode::Char(' '), none, "cycle-axis-for-tile"); - ts.bind_args( - KeyCode::Char('r'), - none, - "set-axis-for-tile", - vec!["row".into()], - ); - ts.bind_args( - KeyCode::Char('c'), - none, - "set-axis-for-tile", - vec!["column".into()], - ); - ts.bind_args( - KeyCode::Char('p'), - none, - "set-axis-for-tile", - vec!["page".into()], - ); - ts.bind_args( - KeyCode::Char('n'), - none, - "set-axis-for-tile", - vec!["none".into()], - ); - ts.bind_args( - KeyCode::Char(':'), - none, - "enter-mode", - vec!["command".into()], - ); - set.insert(ModeKey::TileSelect, Arc::new(ts)); - - // ── Minibuffer modes ───────────────────────────────────────────── - // All of these share the TextEntry spine (append / pop / Esc-clears); - // only the buffer, the exit mode and the commit keys differ. The - // commit-* commands take their target edit-mode as an argument so the - // command itself stays mode-agnostic. - let ed = Arc::new( - TextEntry::new(&AppMode::editing(), "normal") - .commit(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()]) - .commit( - KeyCode::Tab, - "commit-and-advance-right", - vec!["editing".into()], - ) - .finish(), - ); - set.insert(ModeKey::Editing, ed.clone()); - - // Records editing inherits from editing, overriding Esc/Enter/Tab so - // the post-commit re-enter targets records-editing. - set.insert( - ModeKey::RecordsEditing, - Arc::new( - TextEntry::with_parent(&AppMode::records_editing(), "records-normal", ed) - .commit( - KeyCode::Enter, - "commit-cell-edit", - vec!["records-editing".into()], - ) - .commit( - KeyCode::Tab, - "commit-and-advance-right", - vec!["records-editing".into()], - ) - .finish(), - ), - ); - - set.insert( - ModeKey::FormulaEdit, - Arc::new( - TextEntry::new(&AppMode::formula_edit(), "formula-panel") - .commit(KeyCode::Enter, "commit-buffer", vec!["formula".into()]) - .finish(), - ), - ); - - // Category and item add stay open after a commit, so Tab and Enter do - // the same thing: file this one and start the next. - set.insert( - ModeKey::CategoryAdd, - Arc::new( - TextEntry::new(&AppMode::category_add(), "category-panel") - .commit(KeyCode::Enter, "commit-buffer", vec!["category".into()]) - .commit(KeyCode::Tab, "commit-buffer", vec!["category".into()]) - .finish(), - ), - ); - - set.insert( - ModeKey::ItemAdd, - Arc::new( - // The category only shapes the prompt text, never the buffer, - // so the keymap does not need the real one. - TextEntry::new(&AppMode::item_add(String::new()), "category-panel") - .commit(KeyCode::Enter, "commit-buffer", vec!["item".into()]) - .commit(KeyCode::Tab, "commit-buffer", vec!["item".into()]) - .finish(), - ), - ); - - set.insert( - ModeKey::ExportPrompt, - Arc::new( - TextEntry::new(&AppMode::export_prompt(), "normal") - .commit(KeyCode::Enter, "commit-export", vec![]) - .finish(), - ), - ); - - set.insert( - ModeKey::CommandMode, - Arc::new( - TextEntry::new(&AppMode::command_mode(), "normal") - .commit(KeyCode::Enter, "execute-command", vec![]) - // Backspace on an empty `:` line leaves command mode - // rather than popping a character that isn't there. - .rebind(KeyCode::Backspace, "command-mode-backspace") - .finish(), - ), - ); - - // ── Search mode ────────────────────────────────────────────────── - let mut sm = Keymap::new(); - sm.bind(KeyCode::Esc, none, "exit-search-mode"); - sm.bind(KeyCode::Enter, none, "exit-search-mode"); - sm.bind_args(KeyCode::Backspace, none, "pop-char", vec!["search".into()]); - sm.bind_any_char("append-char", vec!["search".into()]); - set.insert(ModeKey::SearchMode, Arc::new(sm)); - - // ── Import wizard ──────────────────────────────────────────────── - let mut wiz = Keymap::new(); - wiz.bind_any("handle-wizard-key"); - set.insert(ModeKey::ImportWizard, Arc::new(wiz)); - - set } } - #[cfg(test)] mod tests { + use std::collections::HashMap; + use super::*; use crate::command::cmd::test_helpers; + use improvise_protocol::{AppMode, Panel}; // ── Keymap::lookup fallback chain ────────────────────────────────── @@ -1155,7 +354,7 @@ mod tests { ]; for mode in &expected_modes { assert!( - ks.mode_maps.contains_key(mode), + ks.maps().contains_key(mode), "Missing keymap for mode {:?}", mode ); @@ -1165,7 +364,7 @@ mod tests { #[test] fn normal_mode_has_basic_movement() { let ks = KeymapSet::default_keymaps(); - let normal = ks.mode_maps.get(&ModeKey::Normal).unwrap(); + let normal = ks.maps().get(&ModeKey::Normal).unwrap(); // hjkl should be bound for key in ['h', 'j', 'k', 'l'] { assert!( @@ -1189,7 +388,7 @@ mod tests { #[test] fn editing_mode_has_any_char_and_esc() { let ks = KeymapSet::default_keymaps(); - let editing = ks.mode_maps.get(&ModeKey::Editing).unwrap(); + let editing = ks.maps().get(&ModeKey::Editing).unwrap(); // Should have AnyChar for text input assert!( editing @@ -1203,7 +402,7 @@ mod tests { #[test] fn search_mode_has_any_char_and_esc() { let ks = KeymapSet::default_keymaps(); - let search = ks.mode_maps.get(&ModeKey::SearchMode).unwrap(); + let search = ks.maps().get(&ModeKey::SearchMode).unwrap(); assert!( search .lookup(KeyCode::Char('a'), KeyModifiers::NONE) @@ -1230,7 +429,7 @@ mod tests { let layout = test_helpers::make_layout(&wb); let mut checked = 0usize; - for (mode, km) in &ks.mode_maps { + for (mode, km) in ks.maps().iter() { // Build the context in the mode whose keymap this is: a few // parsers read `ctx.mode` (`commit-buffer item` resolves its // target category off `AppMode::ItemAdd`). @@ -1288,7 +487,7 @@ mod tests { fn every_hint_key_resolves_in_its_mode() { let ks = KeymapSet::default_keymaps(); let mut checked = 0usize; - for (mode, km) in &ks.mode_maps { + for (mode, km) in ks.maps().iter() { // SearchMode is Normal plus a flag and has no hint of its own; // `app_mode_for` hands back Normal's, which describes Normal. if *mode == ModeKey::SearchMode { @@ -1422,13 +621,13 @@ mod tests { #[test] fn prefix_keymaps_are_flat() { let ks = KeymapSet::default_keymaps(); - for (mode, km) in &ks.mode_maps { - for (pattern, binding) in &km.bindings { + for (mode, km) in ks.maps().iter() { + for (pattern, binding) in km.bindings() { let Binding::Prefix(sub) = binding else { continue; }; let nested: Vec<&KeyPattern> = sub - .bindings + .bindings() .iter() .filter(|(_, b)| matches!(b, Binding::Prefix(_))) .map(|(p, _)| p) @@ -1466,7 +665,7 @@ mod tests { /// The command and args `mode`'s keymap binds `key` to. fn bound(ks: &KeymapSet, mode: &ModeKey, key: KeyCode) -> (String, Vec) { - let km = ks.mode_maps.get(mode).expect("every mode has a keymap"); + let km = ks.maps().get(mode).expect("every mode has a keymap"); match km.lookup(key, KeyModifiers::NONE) { Some(Binding::Cmd { name, args }) => (name.to_string(), args.clone()), other => panic!("{mode:?}: {key:?} should be a plain command, got {other:?}"), @@ -1555,11 +754,11 @@ mod tests { let layout = make_layout(&wb); let reg = make_registry(); let ctx = make_ctx(&wb, &layout, ®); - let km = ks.mode_maps.get(&ModeKey::Normal).unwrap(); + let km = ks.maps().get(&ModeKey::Normal).unwrap(); for panel in Panel::ALL { - let effects = km - .dispatch(®, &ctx, KeyCode::Char(panel.key()), KeyModifiers::NONE) + let effects = ks + .dispatch_transient(km, &ctx, KeyCode::Char(panel.key()), KeyModifiers::NONE) .unwrap_or_else(|| { panic!("{} panel: {:?} did not dispatch", panel.name(), panel.key()) }); @@ -1599,7 +798,7 @@ mod tests { /// The buffer `append-char` is bound to in `mode`'s keymap. fn append_target(ks: &KeymapSet, mode: &AppMode) -> String { let key = ModeKey::from_app_mode(mode, false).expect("a text-entry mode key"); - let km = ks.mode_maps.get(&key).expect("every mode has a keymap"); + let km = ks.maps().get(&key).expect("every mode has a keymap"); match km.lookup(KeyCode::Char('x'), KeyModifiers::NONE) { Some(Binding::Cmd { name, args }) => { assert_eq!(*name, "append-char", "{key:?} should append chars"); @@ -1643,7 +842,7 @@ mod tests { .filter_map(|m| ModeKey::from_app_mode(m, false)) .collect(); let typing: Vec = ks - .mode_maps + .maps() .iter() .filter(|(mode, km)| { **mode != ModeKey::SearchMode @@ -1677,7 +876,7 @@ mod tests { for (app_mode, exits) in text_entry_modes_with_exits() { let mode = ModeKey::from_app_mode(&app_mode, false).unwrap(); let buffer = append_target(&ks, &app_mode); - let km = ks.mode_maps.get(&mode).unwrap(); + let km = ks.maps().get(&mode).unwrap(); for key in exits { let Some(Binding::Sequence(steps)) = km.lookup(*key, none) else { @@ -1725,9 +924,9 @@ mod tests { let mut ctx = make_ctx(&wb, &layout, ®); ctx.mode = mode; ctx.buffers = &bufs; - let km = ks.mode_maps.get(&mode_key).unwrap(); - let effects = km - .dispatch(®, &ctx, KeyCode::Enter, KeyModifiers::NONE) + let km = ks.maps().get(&mode_key).unwrap(); + let effects = ks + .dispatch_transient(km, &ctx, KeyCode::Enter, KeyModifiers::NONE) .unwrap_or_else(|| panic!("{mode_key:?}: Enter did not dispatch")); let dbg = effects_debug(&effects); assert!( @@ -1740,8 +939,110 @@ mod tests { #[test] fn import_wizard_has_any_catchall() { let ks = KeymapSet::default_keymaps(); - let wiz = ks.mode_maps.get(&ModeKey::ImportWizard).unwrap(); + let wiz = ks.maps().get(&ModeKey::ImportWizard).unwrap(); // Should catch any key at all assert!(wiz.lookup(KeyCode::F(5), KeyModifiers::NONE).is_some()); } } + +#[cfg(test)] +mod binding_dump { + use super::tests_support::*; + use super::*; + + /// The instrument `doctrine(binding_dump_diff)` asks for: every + /// `(mode, key, modifier)` lookup across all sixteen modes, written to + /// the file named by `IMPROVISE_BINDING_DUMP`. + /// + /// Most bindings have no direct test, so a green suite is not evidence + /// that a keymap refactor preserved them. Dump before, dump after, diff. + /// It is inert without the variable: a test that writes a file nobody + /// asked for is a surprise, not a check. + /// + /// This is what proved the keymap move to `improvise-protocol` changed + /// nothing — 4021 resolved lookups, byte-identical either side — and + /// the diff was shown to be discriminating by renaming one binding and + /// watching eight lines move. + #[test] + fn dump_every_binding() { + let Ok(path) = std::env::var("IMPROVISE_BINDING_DUMP") else { + return; + }; + let ks = KeymapSet::default_keymaps(); + let mut out = String::new(); + for mode in ALL_MODES { + for key in probe_keys_pub() { + for mods in [ + KeyModifiers::NONE, + KeyModifiers::CONTROL, + KeyModifiers::SHIFT, + KeyModifiers::ALT, + ] { + let km = mode_map(&ks, &mode); + let b = match km.lookup(key, mods) { + None => "-".to_string(), + Some(Binding::Cmd { name, args }) => format!("cmd {name} {args:?}"), + Some(Binding::Prefix(sub)) => { + let mut hints = sub.binding_hints(); + hints.sort(); + format!("prefix {hints:?}") + } + Some(Binding::Sequence(steps)) => format!("seq {steps:?}"), + }; + if b != "-" { + out.push_str(&format!("{mode:?}\t{key:?}\t{mods:?}\t{b}\n")); + } + } + } + } + std::fs::write(path, out).unwrap(); + } +} + +#[cfg(test)] +mod tests_support { + use super::*; + pub const ALL_MODES: [ModeKey; 16] = [ + ModeKey::Normal, + ModeKey::Help, + ModeKey::FormulaPanel, + ModeKey::CategoryPanel, + ModeKey::ViewPanel, + ModeKey::TileSelect, + ModeKey::Editing, + ModeKey::FormulaEdit, + ModeKey::CategoryAdd, + ModeKey::ItemAdd, + ModeKey::ExportPrompt, + ModeKey::CommandMode, + ModeKey::SearchMode, + ModeKey::ImportWizard, + ModeKey::RecordsNormal, + ModeKey::RecordsEditing, + ]; + pub fn mode_map<'a>(ks: &'a KeymapSet, mode: &ModeKey) -> &'a Keymap { + ks.maps().get(mode).expect("every mode has a keymap") + } + pub fn probe_keys_pub() -> Vec { + let mut keys: Vec = (0x20u8..0x7f).map(|c| KeyCode::Char(c as char)).collect(); + keys.extend([ + KeyCode::Esc, + KeyCode::Enter, + KeyCode::Up, + KeyCode::Down, + KeyCode::Left, + KeyCode::Right, + KeyCode::Tab, + KeyCode::BackTab, + KeyCode::Backspace, + KeyCode::Delete, + KeyCode::Home, + KeyCode::End, + KeyCode::PageUp, + KeyCode::PageDown, + KeyCode::F(1), + KeyCode::F(2), + ]); + keys + } +} diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 48d69e2d..7765798b 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -470,7 +470,14 @@ impl App { .unwrap_or_default() } - fn display_value_of(&self, key: &CellKey) -> String { + /// The value an editor opened on `key` starts with — unformatted, and + /// so **not** `CellDisplay::text` (design-principles §4: display + /// rounding is view-only and must not feed back into the model). + /// + /// Crate-visible so `project::cell_display` can put it on the wire as + /// `CellDisplay::edit_text`; a browser that un-formatted the drawn text + /// instead would be a second derivation, and a lossy one. + pub(crate) fn display_value_of(&self, key: &CellKey) -> String { match crate::view::synthetic_record_info(key) { Some((idx, col)) => self .layout diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index 7fd6db7b..3ea84702 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -265,6 +265,7 @@ fn cell_display(app: &App, row: usize, col: usize, comma: bool, decimals: u8) -> let text = layout.records_display(row, col).unwrap_or_default(); let numeric = !text.is_empty() && text.parse::().is_ok(); return CellDisplay { + edit_text: Some(text.clone()), text, numeric, error: false, @@ -282,6 +283,14 @@ fn cell_display(app: &App, row: usize, col: usize, comma: bool, decimals: u8) -> numeric: matches!(value, Some(CellValue::Number(_))), error: matches!(value, Some(CellValue::Error(_))), id: cell_id(app, row, col), + // The *unformatted* value, which is what the TUI's own + // `display_value_at_cursor` seeds its edit buffer from. Projecting it + // rather than letting a client un-format `text` is the rule from + // gotcha 47: where a client cannot compose the command, enrich the + // projection. + edit_text: layout + .cell_key(row, col) + .map(|key| app.display_value_of(&key)), } } diff --git a/crates/improvise-protocol/src/keymap.rs b/crates/improvise-protocol/src/keymap.rs new file mode 100644 index 00000000..48106856 --- /dev/null +++ b/crates/improvise-protocol/src/keymap.rs @@ -0,0 +1,939 @@ +//! The keymap: which key means what, in which mode. +//! +//! # One table, both ends +//! +//! This used to live in `improvise-command`, where only the terminal could +//! reach it, and the browser client carried a hand-written classifier that +//! answered the same question for the subset it could resolve. That is two +//! definitions of one vocabulary kept in step by a lockstep test over a +//! hand-written list of keys — and +//! `doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)` +//! says such a pair is one definition waiting to be made. It is made here. +//! +//! The move is the one gotcha 13 already anticipated: +//! `ViewState::transient_prefix` stores a prefix as an *address* rather than +//! an `Arc` precisely so that `ViewState` could stay free of +//! `improvise-command` types while the keymap came the other way. +//! +//! # The boundary that did not move +//! +//! A lookup yields a command **name and arguments**. Turning that into +//! something that runs needs a `CmdContext` — a `Model`, a `GridLayout`, an +//! `App` — which a thin client does not have, so *intent → mutation* stays +//! where it was: `improvise-command` resolves a name through its +//! `CmdRegistry` into `Effect`s, and a browser client resolves the subset it +//! can compose into `Command`s. +//! +//! That is a better seam than the one it replaces. The drift risk used to be +//! "which key does what" — 170 bindings, no direct test, invisible when +//! wrong. It is now "which command names can I execute", which is a list. +//! +//! What stayed in `improvise-command`, and why each had to: +//! +//! - `Keymap::dispatch` and `KeymapSet`, which pair this table with a +//! `CmdRegistry` and return `Box`. +//! - `SetTransientPrefix`, which is an `Effect` — it writes +//! `ViewState::transient_prefix`, and `Effect` is the command layer's trait. + +use std::collections::HashMap; +use std::fmt; +use std::sync::Arc; + +use crate::key::{KeyCode, KeyModifiers}; +use crate::{AppMode, Panel}; +/// Format a KeyCode as a short human-readable label for which-key display. +fn format_key_label(code: &KeyCode) -> String { + match code { + KeyCode::Char(c) => c.to_string(), + KeyCode::Enter => "Enter".to_string(), + KeyCode::Esc => "Esc".to_string(), + KeyCode::Tab => "Tab".to_string(), + KeyCode::BackTab => "S-Tab".to_string(), + KeyCode::Backspace => "BS".to_string(), + KeyCode::Delete => "Del".to_string(), + KeyCode::Left => "←".to_string(), + KeyCode::Right => "→".to_string(), + KeyCode::Up => "↑".to_string(), + KeyCode::Down => "↓".to_string(), + KeyCode::Home => "Home".to_string(), + KeyCode::End => "End".to_string(), + KeyCode::PageUp => "PgUp".to_string(), + KeyCode::PageDown => "PgDn".to_string(), + KeyCode::F(n) => format!("F{n}"), + _ => format!("{code:?}"), + } +} +// `cmd` module imported for `default_registry()` in default_keymaps() + +/// A key pattern that can be matched against a KeyEvent. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum KeyPattern { + /// Single key with modifiers + Key(KeyCode, KeyModifiers), + /// Matches any Char key (for text-entry modes). + AnyChar, + /// Matches any key at all (lowest priority fallback). + Any, +} + +/// Identifies which mode a binding applies to. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ModeKey { + Normal, + Help, + FormulaPanel, + CategoryPanel, + ViewPanel, + TileSelect, + Editing, + FormulaEdit, + CategoryAdd, + ItemAdd, + ExportPrompt, + CommandMode, + SearchMode, + ImportWizard, + RecordsNormal, + RecordsEditing, +} + +impl ModeKey { + pub fn from_app_mode(mode: &AppMode, search_mode: bool) -> Option { + match mode { + AppMode::Normal if search_mode => Some(ModeKey::SearchMode), + AppMode::Normal => Some(ModeKey::Normal), + AppMode::Help => Some(ModeKey::Help), + AppMode::FormulaPanel => Some(ModeKey::FormulaPanel), + AppMode::CategoryPanel => Some(ModeKey::CategoryPanel), + AppMode::ViewPanel => Some(ModeKey::ViewPanel), + AppMode::TileSelect => Some(ModeKey::TileSelect), + AppMode::Editing { .. } => Some(ModeKey::Editing), + AppMode::FormulaEdit { .. } => Some(ModeKey::FormulaEdit), + AppMode::CategoryAdd { .. } => Some(ModeKey::CategoryAdd), + AppMode::ItemAdd { .. } => Some(ModeKey::ItemAdd), + AppMode::ExportPrompt { .. } => Some(ModeKey::ExportPrompt), + AppMode::CommandMode { .. } => Some(ModeKey::CommandMode), + AppMode::ImportWizard => Some(ModeKey::ImportWizard), + AppMode::RecordsNormal => Some(ModeKey::RecordsNormal), + AppMode::RecordsEditing { .. } => Some(ModeKey::RecordsEditing), + // Exhaustive on purpose: a new AppMode without a keymap silently + // swallows every keypress, so the compiler must flag it here. + AppMode::Quit => None, + } + } +} + +/// What a key binding resolves to. +#[derive(Debug, Clone)] +pub enum Binding { + /// A command name + arguments, looked up in the registry at dispatch time. + Cmd { + name: &'static str, + args: Vec, + }, + /// A prefix sub-keymap (Emacs-style). + Prefix(Arc), + /// A sequence of commands executed in order, concatenating their effects. + Sequence(Vec<(&'static str, Vec)>), +} + +/// A keymap maps key patterns to bindings (command names or prefix sub-keymaps). +/// Supports Emacs-style parent keymap inheritance: if a key is not found in this +/// keymap, lookup falls through to the parent. +#[derive(Default)] +pub struct Keymap { + bindings: HashMap, + parent: Option>, +} + +impl fmt::Debug for Keymap { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Keymap") + .field("binding_count", &self.bindings.len()) + .finish() + } +} + +impl Keymap { + pub fn new() -> Self { + Self { + bindings: HashMap::new(), + parent: None, + } + } + + /// Create a keymap that inherits from a parent. Keys not found here + /// fall through to the parent (Emacs-style keymap inheritance). + pub fn with_parent(parent: Arc) -> Self { + Self { + bindings: HashMap::new(), + parent: Some(parent), + } + } + + /// Bind a key to a command name (no args). + pub fn bind(&mut self, key: KeyCode, mods: KeyModifiers, name: &'static str) { + self.bindings.insert( + KeyPattern::Key(key, mods), + Binding::Cmd { name, args: vec![] }, + ); + } + + /// Bind a key to a command name with arguments. + pub fn bind_args( + &mut self, + key: KeyCode, + mods: KeyModifiers, + name: &'static str, + args: Vec, + ) { + self.bindings + .insert(KeyPattern::Key(key, mods), Binding::Cmd { name, args }); + } + + /// Bind a prefix key that activates a sub-keymap. + pub fn bind_prefix(&mut self, key: KeyCode, mods: KeyModifiers, sub: Arc) { + self.bindings + .insert(KeyPattern::Key(key, mods), Binding::Prefix(sub)); + } + + /// Bind a key to a sequence of commands (executed in order). + pub fn bind_seq( + &mut self, + key: KeyCode, + mods: KeyModifiers, + steps: Vec<(&'static str, Vec)>, + ) { + self.bindings + .insert(KeyPattern::Key(key, mods), Binding::Sequence(steps)); + } + + /// Bind a catch-all for any Char key. + pub fn bind_any_char(&mut self, name: &'static str, args: Vec) { + self.bindings + .insert(KeyPattern::AnyChar, Binding::Cmd { name, args }); + } + + /// Bind a catch-all for any key at all. + pub fn bind_any(&mut self, name: &'static str) { + self.bindings + .insert(KeyPattern::Any, Binding::Cmd { name, args: vec![] }); + } + + /// Return human-readable hints for all concrete bindings in this keymap. + /// Used by the which-key popup to show available completions after a prefix key. + pub fn binding_hints(&self) -> Vec<(String, &'static str)> { + let mut hints: Vec<(String, &'static str)> = self + .bindings + .iter() + .filter_map(|(pattern, binding)| { + let label = match pattern { + KeyPattern::Key(code, _) => format_key_label(code), + KeyPattern::AnyChar | KeyPattern::Any => return None, + }; + let name = match binding { + Binding::Cmd { name, .. } => *name, + Binding::Prefix(_) => return None, + Binding::Sequence(steps) => { + if let Some((name, _)) = steps.first() { + *name + } else { + return None; + } + } + }; + Some((label, name)) + }) + .collect(); + hints.sort_by(|a, b| a.0.cmp(&b.0)); + hints + } + + /// This keymap's own bindings, not counting anything inherited. + /// + /// For tests that must walk the table rather than probe it — the + /// flatness check on prefixes, and the "which modes bind AnyChar" walk. + pub fn bindings(&self) -> &HashMap { + &self.bindings + } + + /// Look up the binding for a key in this keymap's own bindings. + fn lookup_local(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> { + self.bindings + .get(&KeyPattern::Key(key, mods)) + .or_else(|| { + // Retry Char keys without modifiers (shift is implicit in the char) + if matches!(key, KeyCode::Char(_)) && mods != KeyModifiers::NONE { + self.bindings.get(&KeyPattern::Key(key, KeyModifiers::NONE)) + } else { + None + } + }) + .or_else(|| { + if matches!(key, KeyCode::Char(_)) { + self.bindings.get(&KeyPattern::AnyChar) + } else { + None + } + }) + .or_else(|| self.bindings.get(&KeyPattern::Any)) + } + + /// Look up the binding for a key, falling through to parent keymaps. + pub fn lookup(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> { + self.lookup_local(key, mods) + .or_else(|| self.parent.as_ref().and_then(|p| p.lookup(key, mods))) + } +} + +/// Builder for a minibuffer mode's keymap. +/// +/// Every text-entry mode has the same spine: characters append to one named +/// buffer, Backspace pops from it, and Esc clears it before leaving for some +/// other mode. Only the buffer name, the exit mode and the commit keys differ. +/// Building them through here means a new text mode cannot accidentally type +/// into one buffer while clearing another, or drop the clear entirely — which +/// would leave the next entry pre-filled with the last one. +/// +/// `SearchMode` deliberately does not use this: it has no commit key, and its +/// query is owned by `exit-search-mode` rather than by `clear-buffer`. +pub struct TextEntry { + buffer: String, + km: Keymap, +} + +impl TextEntry { + /// The keymap for `mode`, returning to `exit` on Esc. The buffer comes + /// off the mode's own `MinibufferConfig`, so the buffer the prompt + /// describes and the buffer the keys edit are the same string by + /// construction rather than by agreement (improvise-k8h). + pub fn new(mode: &AppMode, exit: &'static str) -> Self { + Self::build(buffer_of(mode).to_string(), exit, Keymap::new()) + } + + /// As [`TextEntry::new`], but inheriting unbound keys from `parent`. + pub fn with_parent(mode: &AppMode, exit: &'static str, parent: Arc) -> Self { + Self::build( + buffer_of(mode).to_string(), + exit, + Keymap::with_parent(parent), + ) + } + + fn build(buffer: String, exit: &'static str, mut km: Keymap) -> Self { + let none = KeyModifiers::NONE; + km.bind_seq( + KeyCode::Esc, + none, + vec![ + ("clear-buffer", vec![buffer.clone()]), + ("enter-mode", vec![exit.into()]), + ], + ); + km.bind_args(KeyCode::Backspace, none, "pop-char", vec![buffer.clone()]); + km.bind_any_char("append-char", vec![buffer.clone()]); + Self { buffer, km } + } + + /// Bind `key` to "run `cmd`, then clear the buffer". The two halves are + /// welded together here so no caller can bind a commit without its clear. + pub fn commit(mut self, key: KeyCode, cmd: &'static str, args: Vec) -> Self { + self.km.bind_seq( + key, + KeyModifiers::NONE, + vec![(cmd, args), ("clear-buffer", vec![self.buffer.clone()])], + ); + self + } + + /// Replace a spine binding — used only by CommandMode, whose Backspace + /// exits the mode on an empty buffer instead of popping a character. + pub fn rebind(mut self, key: KeyCode, cmd: &'static str) -> Self { + self.km.bind(key, KeyModifiers::NONE, cmd); + self + } + + pub fn finish(self) -> Keymap { + self.km + } +} + +/// Builder for a side panel's keymap. +/// +/// The three panels share a spine: Esc and Tab leave for Normal, the arrows +/// and j/k move that panel's cursor, every panel's toggle key is bound so +/// focus can move between them, and `:` opens command mode. Only the keys +/// that act on the panel's contents differ — and they differ in shape, not +/// just in command name (Category's `n` takes an argument, View has no `a`, +/// Formula has no Enter), so they arrive through chained methods rather than +/// a fixed-arity signature (improvise-4pg). +pub struct PanelKeymap { + km: Keymap, +} + +impl PanelKeymap { + /// The spine every panel shares, with the cursor keys pointed at `panel`. + pub fn new(panel: Panel) -> Self { + let none = KeyModifiers::NONE; + let mut km = Keymap::new(); + km.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); + km.bind_args(KeyCode::Tab, none, "enter-mode", vec!["normal".into()]); + for (keys, delta) in [ + ([KeyCode::Up, KeyCode::Char('k')], "-1"), + ([KeyCode::Down, KeyCode::Char('j')], "1"), + ] { + for key in keys { + km.bind_args( + key, + none, + "move-panel-cursor", + vec![panel.name().into(), delta.into()], + ); + } + } + bind_panel_toggles(&mut km); + km.bind_args( + KeyCode::Char(':'), + none, + "enter-mode", + vec!["command".into()], + ); + Self { km } + } + + /// Bind a key that acts on this panel's contents. + pub fn bind(mut self, key: KeyCode, cmd: &'static str) -> Self { + self.km.bind(key, KeyModifiers::NONE, cmd); + self + } + + /// As [`PanelKeymap::bind`], for a command that takes arguments. + pub fn bind_args(mut self, key: KeyCode, cmd: &'static str, args: Vec) -> Self { + self.km.bind_args(key, KeyModifiers::NONE, cmd, args); + self + } + + /// `d` and Delete both remove whatever the cursor is on. Binding them + /// together means a panel cannot offer one without the other. + pub fn delete(self, cmd: &'static str) -> Self { + self.bind(KeyCode::Char('d'), cmd) + .bind(KeyCode::Delete, cmd) + } + + pub fn finish(self) -> Keymap { + self.km + } +} + +/// Bind every panel's toggle key. Used by the panel keymaps and by Normal, +/// so a new panel reaches all four with one `Panel::ALL` entry. +fn bind_panel_toggles(km: &mut Keymap) { + for panel in Panel::ALL { + km.bind_args( + KeyCode::Char(panel.key()), + KeyModifiers::NONE, + "toggle-panel-and-focus", + vec![panel.name().into()], + ); + } +} + +/// The buffer a text-entry mode types into. Every caller is one of the seven +/// modes that carry a `MinibufferConfig`; a mode without one has no minibuffer +/// to build a keymap for. +fn buffer_of(mode: &AppMode) -> &str { + &mode + .minibuffer() + .unwrap_or_else(|| panic!("{} has no minibuffer to type into", mode.label())) + .buffer_key +} + +/// Maps modes to their root keymaps. +/// +/// The table alone. `improvise-command`'s `KeymapSet` wraps one of these with +/// a `CmdRegistry` to get from a bound name to an `Effect`; a browser client +/// wraps the same table with its own much smaller resolver. Neither owns the +/// bindings. +#[derive(Debug, Default)] +pub struct Keymaps { + mode_maps: HashMap>, +} + +impl Keymaps { + pub fn new() -> Self { + Self::default() + } + + pub fn insert(&mut self, mode: ModeKey, keymap: Arc) { + self.mode_maps.insert(mode, keymap); + } + + /// The root keymap for a mode. + pub fn get(&self, mode: &ModeKey) -> Option<&Arc> { + self.mode_maps.get(mode) + } + + /// Every mode that has a keymap, for tests that must walk all of them. + pub fn modes(&self) -> impl Iterator { + self.mode_maps.keys() + } + + pub fn iter(&self) -> impl Iterator)> { + self.mode_maps.iter() + } + + pub fn contains_key(&self, mode: &ModeKey) -> bool { + self.mode_maps.contains_key(mode) + } + + /// The binding `mode` gives `key`, walking parent keymaps. + pub fn lookup(&self, mode: &ModeKey, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> { + self.mode_maps.get(mode)?.lookup(key, mods) + } + + /// The sub-keymap a prefix key opens, in `mode`. `None` when the mode + /// has no such binding, or when the binding is not a prefix. + /// + /// Resolution deliberately goes through `lookup`, which walks parent + /// keymaps: `RecordsNormal` has no prefixes of its own and reaches + /// Normal's through inheritance. + pub fn prefix_keymap( + &self, + mode: &ModeKey, + key: KeyCode, + mods: KeyModifiers, + ) -> Option> { + match self.mode_maps.get(mode)?.lookup(key, mods)? { + Binding::Prefix(sub) => Some(sub.clone()), + Binding::Cmd { .. } | Binding::Sequence(_) => None, + } + } + + /// Build the default keymap set with all bindings. + pub fn default_keymaps() -> Self { + let mut set = Self::new(); + let none = KeyModifiers::NONE; + let ctrl = KeyModifiers::CONTROL; + + // ── Normal mode ────────────────────────────────────────────────── + let mut normal = Keymap::new(); + + // Navigation + for (key, dr, dc) in [ + (KeyCode::Up, -1, 0), + (KeyCode::Down, 1, 0), + (KeyCode::Left, 0, -1), + (KeyCode::Right, 0, 1), + ] { + normal.bind_args( + key, + none, + "move-selection", + vec![dr.to_string(), dc.to_string()], + ); + } + for (ch, dr, dc) in [('k', -1, 0), ('j', 1, 0), ('h', 0, -1), ('l', 0, 1)] { + normal.bind_args( + KeyCode::Char(ch), + none, + "move-selection", + vec![dr.to_string(), dc.to_string()], + ); + } + + // Jump to boundaries + normal.bind(KeyCode::Char('G'), none, "jump-last-row"); + normal.bind(KeyCode::Char('0'), none, "jump-first-col"); + normal.bind(KeyCode::Char('$'), none, "jump-last-col"); + normal.bind(KeyCode::Home, none, "jump-first-col"); + normal.bind(KeyCode::End, none, "jump-last-col"); + + // Scroll + normal.bind_args(KeyCode::Char('d'), ctrl, "scroll-rows", vec!["5".into()]); + normal.bind_args(KeyCode::Char('u'), ctrl, "scroll-rows", vec!["-5".into()]); + normal.bind_args(KeyCode::PageDown, none, "page-scroll", vec!["1".into()]); + normal.bind_args(KeyCode::PageUp, none, "page-scroll", vec!["-1".into()]); + + // Cell operations + normal.bind(KeyCode::Char('x'), none, "clear-cell"); + normal.bind(KeyCode::Char('p'), none, "paste"); + + // View + normal.bind(KeyCode::Char('t'), none, "transpose"); + + // Mode changes + normal.bind(KeyCode::Char('q'), ctrl, "force-quit"); + normal.bind_args( + KeyCode::Char(':'), + none, + "enter-mode", + vec!["command".into()], + ); + normal.bind(KeyCode::Char('/'), none, "search"); + normal.bind(KeyCode::Char('s'), ctrl, "save"); + normal.bind(KeyCode::F(1), none, "enter-mode"); + normal.bind_args(KeyCode::F(1), none, "enter-mode", vec!["help".into()]); + normal.bind_args(KeyCode::Char('?'), none, "enter-mode", vec!["help".into()]); + + // Panel toggles — the same set the panel keymaps bind, so a new + // panel reaches Normal and all three panels at once. + bind_panel_toggles(&mut normal); + normal.bind_args( + KeyCode::Char('f'), + ctrl, + "toggle-panel-visibility", + vec!["formula".into()], + ); + normal.bind_args( + KeyCode::Char('c'), + ctrl, + "toggle-panel-visibility", + vec!["category".into()], + ); + normal.bind_args( + KeyCode::Char('v'), + ctrl, + "toggle-panel-visibility", + vec!["view".into()], + ); + normal.bind(KeyCode::Tab, none, "cycle-panel-focus"); + + // Editing entry — i/a drill into aggregated cells, else edit. + // The mode arg controls which editing mode is entered; records-normal + // overrides these to "records-editing" via its own bindings. + normal.bind_args( + KeyCode::Char('i'), + none, + "edit-or-drill", + vec!["editing".into()], + ); + normal.bind_args( + KeyCode::Char('a'), + none, + "edit-or-drill", + vec!["editing".into()], + ); + normal.bind(KeyCode::Enter, none, "enter-advance"); + normal.bind_args( + KeyCode::Char('e'), + ctrl, + "enter-mode", + vec!["export-prompt".into()], + ); + + // Search / category add + normal.bind_args( + KeyCode::Char('n'), + none, + "search-navigate", + vec!["forward".into()], + ); + normal.bind(KeyCode::Char('N'), none, "search-or-category-add"); + + // Page navigation + normal.bind(KeyCode::Char(']'), none, "page-next"); + normal.bind(KeyCode::Char('['), none, "page-prev"); + + // Group / hide + normal.bind(KeyCode::Char('z'), none, "toggle-group-under-cursor"); + normal.bind(KeyCode::Char('H'), none, "hide-selected-row-item"); + + // Drill into aggregated cell / view history + normal.bind(KeyCode::Char('>'), none, "drill-into-cell"); + normal.bind(KeyCode::Char('<'), none, "view-back"); + + // Records mode toggle and prune toggle + normal.bind(KeyCode::Char('R'), none, "toggle-records-mode"); + normal.bind(KeyCode::Char('P'), none, "toggle-prune-empty"); + + // Tile select + normal.bind(KeyCode::Char('T'), none, "enter-tile-select"); + normal.bind(KeyCode::Left, ctrl, "enter-tile-select"); + normal.bind(KeyCode::Right, ctrl, "enter-tile-select"); + normal.bind(KeyCode::Up, ctrl, "enter-tile-select"); + normal.bind(KeyCode::Down, ctrl, "enter-tile-select"); + + // Prefix keys + let mut g_map = Keymap::new(); + g_map.bind(KeyCode::Char('g'), none, "jump-first-row"); + g_map.bind(KeyCode::Char('z'), none, "toggle-col-group-under-cursor"); + normal.bind_prefix(KeyCode::Char('g'), none, Arc::new(g_map)); + + let mut y_map = Keymap::new(); + y_map.bind(KeyCode::Char('y'), none, "yank"); + normal.bind_prefix(KeyCode::Char('y'), none, Arc::new(y_map)); + + let mut z_map = Keymap::new(); + z_map.bind(KeyCode::Char('Z'), none, "wq"); + normal.bind_prefix(KeyCode::Char('Z'), none, Arc::new(z_map)); + + let normal = Arc::new(normal); + set.insert(ModeKey::Normal, normal.clone()); + + // ── Records normal mode (inherits from normal) ──────────────────── + let mut rn = Keymap::with_parent(normal); + // Override i/a so the edit branch produces records-editing mode + // instead of inheriting the normal-mode "editing" arg. + rn.bind_args( + KeyCode::Char('i'), + none, + "edit-or-drill", + vec!["records-editing".into()], + ); + rn.bind_args( + KeyCode::Char('a'), + none, + "edit-or-drill", + vec!["records-editing".into()], + ); + rn.bind_seq( + KeyCode::Char('o'), + none, + vec![ + ("add-record-row", vec![]), + ("enter-edit-at-cursor", vec!["records-editing".into()]), + ], + ); + set.insert(ModeKey::RecordsNormal, Arc::new(rn)); + + // ── Help mode ──────────────────────────────────────────────────── + let mut help = Keymap::new(); + help.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); + help.bind_args( + KeyCode::Char('q'), + none, + "enter-mode", + vec!["normal".into()], + ); + // Allow entering command mode from Help (important on empty-model launch) + help.bind_args( + KeyCode::Char(':'), + none, + "enter-mode", + vec!["command".into()], + ); + // Page navigation + help.bind(KeyCode::Right, none, "help-page-next"); + help.bind(KeyCode::Char('l'), none, "help-page-next"); + help.bind(KeyCode::Char('n'), none, "help-page-next"); + help.bind(KeyCode::Tab, none, "help-page-next"); + help.bind(KeyCode::Left, none, "help-page-prev"); + help.bind(KeyCode::Char('h'), none, "help-page-prev"); + help.bind(KeyCode::Char('p'), none, "help-page-prev"); + help.bind(KeyCode::BackTab, none, "help-page-prev"); + set.insert(ModeKey::Help, Arc::new(help)); + + // ── Panels ─────────────────────────────────────────────────────── + set.insert( + ModeKey::FormulaPanel, + Arc::new( + PanelKeymap::new(Panel::Formula) + .bind_args( + KeyCode::Char('a'), + "enter-mode", + vec!["formula-edit".into()], + ) + .bind_args( + KeyCode::Char('n'), + "enter-mode", + vec!["formula-edit".into()], + ) + .bind_args( + KeyCode::Char('o'), + "enter-mode", + vec!["formula-edit".into()], + ) + .delete("delete-formula-at-cursor") + .finish(), + ), + ); + + set.insert( + ModeKey::CategoryPanel, + Arc::new( + PanelKeymap::new(Panel::Category) + .bind(KeyCode::Enter, "filter-to-item") + .bind(KeyCode::Char(' '), "cycle-axis-at-cursor") + .bind_args( + KeyCode::Char('n'), + "enter-mode", + vec!["category-add".into()], + ) + .bind(KeyCode::Char('a'), "open-item-add-at-cursor") + .bind(KeyCode::Char('o'), "open-item-add-at-cursor") + .delete("delete-category-at-cursor") + .finish(), + ), + ); + + set.insert( + ModeKey::ViewPanel, + Arc::new( + PanelKeymap::new(Panel::View) + .bind(KeyCode::Enter, "switch-view-at-cursor") + .bind(KeyCode::Char('n'), "create-and-switch-view") + .bind(KeyCode::Char('o'), "create-and-switch-view") + .delete("delete-view-at-cursor") + .finish(), + ), + ); + + // ── Tile select ────────────────────────────────────────────────── + let mut ts = Keymap::new(); + ts.bind_args(KeyCode::Esc, none, "enter-mode", vec!["normal".into()]); + ts.bind_args(KeyCode::Tab, none, "enter-mode", vec!["normal".into()]); + ts.bind_args(KeyCode::Left, none, "move-tile-cursor", vec!["-1".into()]); + ts.bind_args( + KeyCode::Char('h'), + none, + "move-tile-cursor", + vec!["-1".into()], + ); + ts.bind_args(KeyCode::Right, none, "move-tile-cursor", vec!["1".into()]); + ts.bind_args( + KeyCode::Char('l'), + none, + "move-tile-cursor", + vec!["1".into()], + ); + ts.bind(KeyCode::Enter, none, "cycle-axis-for-tile"); + ts.bind(KeyCode::Char(' '), none, "cycle-axis-for-tile"); + ts.bind_args( + KeyCode::Char('r'), + none, + "set-axis-for-tile", + vec!["row".into()], + ); + ts.bind_args( + KeyCode::Char('c'), + none, + "set-axis-for-tile", + vec!["column".into()], + ); + ts.bind_args( + KeyCode::Char('p'), + none, + "set-axis-for-tile", + vec!["page".into()], + ); + ts.bind_args( + KeyCode::Char('n'), + none, + "set-axis-for-tile", + vec!["none".into()], + ); + ts.bind_args( + KeyCode::Char(':'), + none, + "enter-mode", + vec!["command".into()], + ); + set.insert(ModeKey::TileSelect, Arc::new(ts)); + + // ── Minibuffer modes ───────────────────────────────────────────── + // All of these share the TextEntry spine (append / pop / Esc-clears); + // only the buffer, the exit mode and the commit keys differ. The + // commit-* commands take their target edit-mode as an argument so the + // command itself stays mode-agnostic. + let ed = Arc::new( + TextEntry::new(&AppMode::editing(), "normal") + .commit(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()]) + .commit( + KeyCode::Tab, + "commit-and-advance-right", + vec!["editing".into()], + ) + .finish(), + ); + set.insert(ModeKey::Editing, ed.clone()); + + // Records editing inherits from editing, overriding Esc/Enter/Tab so + // the post-commit re-enter targets records-editing. + set.insert( + ModeKey::RecordsEditing, + Arc::new( + TextEntry::with_parent(&AppMode::records_editing(), "records-normal", ed) + .commit( + KeyCode::Enter, + "commit-cell-edit", + vec!["records-editing".into()], + ) + .commit( + KeyCode::Tab, + "commit-and-advance-right", + vec!["records-editing".into()], + ) + .finish(), + ), + ); + + set.insert( + ModeKey::FormulaEdit, + Arc::new( + TextEntry::new(&AppMode::formula_edit(), "formula-panel") + .commit(KeyCode::Enter, "commit-buffer", vec!["formula".into()]) + .finish(), + ), + ); + + // Category and item add stay open after a commit, so Tab and Enter do + // the same thing: file this one and start the next. + set.insert( + ModeKey::CategoryAdd, + Arc::new( + TextEntry::new(&AppMode::category_add(), "category-panel") + .commit(KeyCode::Enter, "commit-buffer", vec!["category".into()]) + .commit(KeyCode::Tab, "commit-buffer", vec!["category".into()]) + .finish(), + ), + ); + + set.insert( + ModeKey::ItemAdd, + Arc::new( + // The category only shapes the prompt text, never the buffer, + // so the keymap does not need the real one. + TextEntry::new(&AppMode::item_add(String::new()), "category-panel") + .commit(KeyCode::Enter, "commit-buffer", vec!["item".into()]) + .commit(KeyCode::Tab, "commit-buffer", vec!["item".into()]) + .finish(), + ), + ); + + set.insert( + ModeKey::ExportPrompt, + Arc::new( + TextEntry::new(&AppMode::export_prompt(), "normal") + .commit(KeyCode::Enter, "commit-export", vec![]) + .finish(), + ), + ); + + set.insert( + ModeKey::CommandMode, + Arc::new( + TextEntry::new(&AppMode::command_mode(), "normal") + .commit(KeyCode::Enter, "execute-command", vec![]) + // Backspace on an empty `:` line leaves command mode + // rather than popping a character that isn't there. + .rebind(KeyCode::Backspace, "command-mode-backspace") + .finish(), + ), + ); + + // ── Search mode ────────────────────────────────────────────────── + let mut sm = Keymap::new(); + sm.bind(KeyCode::Esc, none, "exit-search-mode"); + sm.bind(KeyCode::Enter, none, "exit-search-mode"); + sm.bind_args(KeyCode::Backspace, none, "pop-char", vec!["search".into()]); + sm.bind_any_char("append-char", vec!["search".into()]); + set.insert(ModeKey::SearchMode, Arc::new(sm)); + + // ── Import wizard ──────────────────────────────────────────────── + let mut wiz = Keymap::new(); + wiz.bind_any("handle-wizard-key"); + set.insert(ModeKey::ImportWizard, Arc::new(wiz)); + + set + } +} diff --git a/crates/improvise-protocol/src/lib.rs b/crates/improvise-protocol/src/lib.rs index d67cd9dc..30235e1d 100644 --- a/crates/improvise-protocol/src/lib.rs +++ b/crates/improvise-protocol/src/lib.rs @@ -31,6 +31,7 @@ use std::collections::HashMap; pub mod basis; pub mod frame; pub mod key; +pub mod keymap; mod reduce; pub mod render; pub mod viewmodel; @@ -38,6 +39,7 @@ pub mod viewmodel; pub use basis::{Basis, BasisCache, Rejection}; pub use frame::{ClientFrame, ServerFrame}; pub use key::{KeyCode, KeyEvent, KeyModifiers}; +pub use keymap::{Binding, KeyPattern, Keymap, Keymaps, ModeKey}; pub use reduce::{Command, HelpPage, ModelCommand, reduce_view}; pub use render::{ CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, CellId, FormulaCache, @@ -188,6 +190,38 @@ pub enum AppMode { } impl AppMode { + /// The mode a keymap's `enter-mode ` argument names. + /// + /// Lives beside `AppMode` rather than in the command layer's registry + /// because the *names* are part of the keymap vocabulary, and the keymap + /// is shared (see [`crate::keymap`]): a browser client resolving + /// `enter-mode` has to decode the same spellings the registry does, and + /// two tables would be the mirror that move existed to delete. + /// + /// Not every variant is nameable. `Quit`, `Help`, `ImportWizard`, + /// `ItemAdd` and the three panel modes that *are* nameable are exactly + /// what the registry accepted before the move, and this is that list + /// unchanged — `ItemAdd` is absent because its category is resolved from + /// the runtime mode, never from a script line (gotcha 20). + pub fn from_name(s: &str) -> Option { + match s { + "normal" => Some(Self::Normal), + "help" => Some(Self::Help), + "formula-panel" => Some(Self::FormulaPanel), + "category-panel" => Some(Self::CategoryPanel), + "view-panel" => Some(Self::ViewPanel), + "tile-select" => Some(Self::TileSelect), + "command" => Some(Self::command_mode()), + "category-add" => Some(Self::category_add()), + "editing" => Some(Self::editing()), + "records-normal" => Some(Self::RecordsNormal), + "records-editing" => Some(Self::records_editing()), + "formula-edit" => Some(Self::formula_edit()), + "export-prompt" => Some(Self::export_prompt()), + _ => None, + } + } + /// Short uppercase name shown in the title bar. pub fn label(&self) -> &'static str { match self { diff --git a/crates/improvise-protocol/src/render.rs b/crates/improvise-protocol/src/render.rs index 82c0a223..7e8e8633 100644 --- a/crates/improvise-protocol/src/render.rs +++ b/crates/improvise-protocol/src/render.rs @@ -113,6 +113,25 @@ pub struct CellDisplay { /// category column renames a coordinate across the whole row, gotcha 1, /// which is a different intent from `SetCell`). pub id: Option, + /// What an editor opened on this cell should start with — the value as + /// the user last typed it, **not** as it is drawn. + /// + /// The two differ and the difference corrupts data. `text` has been + /// through `format_f64`, so a stored `1200` is drawn `1,200` under a + /// comma-grouping format; seeding an editor with that and committing it + /// parses as `CellValue::Text("1,200")`, and the number is gone. Display + /// rounding is view-only and must never be fed back into the model + /// (design-principles §4) — an editor's seed is exactly a feedback path. + /// + /// It is the projection of `App::display_value_at_cursor`, which is what + /// `EditOrDrill` seeds the TUI's buffer from, so both front ends start + /// from one string rather than each deriving one. `None` where there is + /// nothing to edit, which is the same set of positions `id` is `None` + /// for plus the cells whose value is absent. + /// + /// A browser found this: `tests/lockstep.rs` drove `i` through the host + /// and the client over the same grid and they disagreed by a comma. + pub edit_text: Option, } impl CellDisplay { @@ -124,20 +143,25 @@ impl CellDisplay { } pub fn number(s: impl Into) -> Self { + let text = s.into(); Self { - text: s.into(), + edit_text: Some(text.clone()), + text, numeric: true, error: false, id: None, } } + /// An error cell is not editable back into the model — its text is a + /// diagnostic, not a value — so it carries no `edit_text`. pub fn error(s: impl Into) -> Self { Self { text: s.into(), numeric: false, error: true, id: None, + edit_text: None, } } diff --git a/crates/improvise-wasm-client/src/keys.rs b/crates/improvise-wasm-client/src/keys.rs index 787465a2..371ab885 100644 --- a/crates/improvise-wasm-client/src/keys.rs +++ b/crates/improvise-wasm-client/src/keys.rs @@ -1,97 +1,92 @@ -//! Browser `KeyboardEvent` → improvise key, and the one question that -//! follows: can this client answer the keypress itself? +//! Browser `KeyboardEvent` → improvise key → `Command`. //! -//! # The keymap is the client's, and the server must not know it +//! # The keymap is not this module's //! -//! That is the user's ruling on improvise-bpq2, and it follows from the -//! standing principle — the model is shared, the UI is not, and a keymap is -//! UI. It buys something real: two clients may hold different bindings over -//! one document, which is impossible if the server owns the keymap. +//! It used to be. `improvise-bpq2` gave this file a hand-written classifier +//! that decided, per key and per mode, what a keypress meant — a second +//! spelling of `default_keymaps()`, kept in step by a lockstep test over a +//! hand-written list. The user's ruling is that the web frontend uses the +//! **same** keymap as the TUI, not a mirror of it, and +//! `doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)` +//! says why: two definitions held honest by a compatibility test are one +//! definition waiting to be made. //! -//! It also closed a question that had been answered the other way. A binding -//! names an **intent** resolved against a `CmdContext`; a `Command` is the -//! **resolved mutation** (gotcha 38), and a thin client holds no `Model` and -//! no `GridLayout` — so keys used to travel upstream unresolved. What made -//! that untenable is not latency. `improvise-9x6` demonstrated, in -//! `a_key_resolved_on_the_host_moves_state_no_client_can_hear`, that -//! **nothing in the projection stream carries the host's own `ViewState`**: a -//! client's slice moves only by folding `Command`s, and resolving a keypress -//! on the host emits none. Host-side resolution does not merely cost a round -//! trip, it **diverges** — and today it does not even do that, because -//! `Host::handle` answers `ClientFrame::Key` with an empty vec. An upstream -//! key is a **black hole**, not a slow path. +//! So the table moved to `improvise_protocol::keymap` and this module reads +//! it. Which key means what is now correct **by construction**: a rebinding +//! in `default_keymaps()` reaches the browser with no change here, and a new +//! binding for a command this module can already execute simply works. //! -//! So the measure of this module is how much of the keymap it can answer, -//! and every [`KeyOutcome::Upstream`] below is a key that currently *does -//! nothing at all* in a browser. The method for growing it is the ticket's: -//! walk `KeymapSet::default_keymaps()` binding by binding and ask what the -//! client would need; where the answer is a fact it does not hold, **enrich -//! the projection** rather than moving resolution back to the host. +//! # What is left here, and why it is the better boundary //! -//! # What is answered here, and what each one needed +//! A lookup yields a command **name and arguments**. Turning that into +//! something that runs needs a `CmdContext` — a `Model`, a `GridLayout`, an +//! `App` — which this client does not have. So *key → intent* is shared and +//! *intent → `Command`* is ours, for the subset we can compose. [`step`] is +//! that subset, and it is the list that replaces the old list of keys. //! -//! - **Minibuffer text entry.** The buffer a character lands in is -//! `AppMode::minibuffer().buffer_key`, which lives on the mode this client -//! already holds — and the host's keymap reads the *same* field, because -//! `TextEntry::new` takes the `AppMode` so the prompt and the keys cannot -//! name different buffers (gotcha 11, improvise-k8h). Derived from the same -//! fact, not copied from the rule. -//! - **Grid cursor movement**, which needed [`GridExtent`]. `GridCache` is -//! viewport-indexed, so a client could not tell "the last visible row" from -//! "the last row" and could not clamp; the extent is the whole grid and is -//! projected for exactly this. The *window* is the other half and is the -//! client's own fact — it is what this client told the host in its -//! `ClientFrame::Viewport`, recorded by `Client::set_viewport`. A client -//! that has not announced one resolves nothing here, which is truthful -//! rather than guessed. -//! - **Panel cursor movement**, which needed nothing. The ticket lists it as -//! a gap on the grounds that re-deriving the category flattening is how two -//! ends drift — but improvise-edp already moved that derivation into -//! `improvise_protocol::viewmodel::flatten_cat_tree`, which both ends call. -//! It is the *shared* derivation, not a second one. The formula and view -//! panels are flat lists in their own caches. -//! - **Tile cursor movement**, which needed nothing: `TileBarCache::tiles` is -//! the list `MoveTileCursor` clamps against. +//! That is a list of names rather than of keys, which is strictly better in +//! two ways: it is enumerable, and it does not go stale when a binding moves. //! -//! # Modifiers, and what the old rule was really about +//! # What this client can execute, and what each one needed //! -//! The rule used to read "this client resolves only unmodified characters", -//! justified by `KeyPattern::AnyChar` matching a `Char` under *any* -//! modifiers. That justification is narrower than the rule: `AnyChar` is -//! bound **only in the text-entry modes** (and `Any` only in `ImportWizard`). -//! In `Normal` there is no such fallback, so `Ctrl-d` has an exact binding -//! and mirroring it is a faithful subset, not a superset. The rule is -//! therefore kept where it was earned — a modified character in a text-entry -//! mode still goes upstream — and not applied where it was never true. +//! - **Text entry** (`append-char`, `pop-char`, `clear-buffer`). The buffer +//! is the binding's own argument now — no longer read off +//! `AppMode::minibuffer()` and hoped to agree with what the host's keymap +//! named. `TextEntry::new` derives one from the other (gotcha 11), so the +//! two were equal; they are now the *same string*. +//! - **Mode entry** (`enter-mode`). The name decodes through +//! `AppMode::from_name`, which moved next to `AppMode` for the same reason +//! the keymap did. +//! - **Grid, panel and tile cursors**, which needed `GridExtent` and the +//! window this client announced. Unchanged from bpq2 except that the key +//! they answer to is now the command name. +//! - **Entering an edit** (`edit-or-drill`), which needed ADR 002: the +//! branch `EditOrDrill` takes on "is this cell aggregated" is exactly the +//! `CellId` variant the server already projects. +//! - **Committing one** (`commit-cell-edit`, `commit-and-advance-right`), +//! which needed the record surrogate — again ADR 002 — and nothing else +//! for the plain-grid case. //! -//! # What is still upstream, and why +//! # What still needs the host //! -//! - **Prefix keys** (`g`, `y`, `Z`, and so everything under them: `gg`, -//! `gz`, `yy`, `ZZ`). `ViewState::transient_prefix` holds the *address* of -//! a prefix, never the sub-keymap (gotcha 13), and resolving one means -//! holding the keymap. A client that owns its keymap outright will own this -//! too; nothing here fakes it. -//! - **Anything naming a row, column, item or group.** `hide-selected-row-item` -//! needs the category a row label belongs to; `toggle-group-under-cursor` -//! needs group membership and collapsed state; `delete-formula-at-cursor` -//! needs a formula's parsed target and target category. None of the three -//! is projected, and `GridCache::row_labels` slash-joins a multi-level -//! tuple (gotcha 44), so there is nothing to take one apart with. -//! - **Anything touching storage** (`save`, `wq`, `commit-export`) or an -//! arbitrary command line (`:`). These are the host's by definition. -//! - **Backspace in `CommandMode`**, the single user of `TextEntry::rebind`: -//! it leaves the mode on an empty buffer instead of popping (gotcha 11), so -//! a client that popped locally would sit in `:` after the host had left it. -//! - **Search mode.** It is `Normal` plus a flag, and its query is its own -//! `ViewState` field rather than an entry in `buffers` (gotcha 11). +//! [`the_command_names_that_still_need_the_host_are_these`] names them, and +//! `Host::handle` answers a `Key` frame with an empty vec, so each one is a +//! binding that **does nothing at all** in a browser +//! (`doctrine(an_upstream_key_is_a_black_hole)`). The classes: +//! +//! - **Prefix keys.** `Command::SetTransientPrefix` exists and this client +//! could arm one; what it cannot do is what the *second* key then runs, +//! which is `yank`, `wq` and `toggle-col-group-under-cursor`. Arming a +//! prefix whose payload is unreachable is worse than not arming it. +//! - **Anything naming a row, column, item or group** — `hide-selected-row-item`, +//! `toggle-group-under-cursor`, `delete-formula-at-cursor`. Not projected, +//! and `GridCache::row_labels` slash-joins a multi-level tuple (gotcha 44), +//! so there is nothing to take one apart with. +//! - **Anything touching storage or an arbitrary command line.** The host's +//! by definition. +//! - **The records-mode commit.** A wide-records row is one coordinate tuple +//! and editing a category column moves every measure cell in it (gotcha 1); +//! `commit_plain_records_edit` needs `GridLayout::record_col` to know which. +//! - **Drill.** Session-local residue (gotcha 30), and the one case here that +//! does **not** go upstream silently — see [`step`]'s `edit-or-drill` arm. +use std::sync::OnceLock; + +use improvise_protocol::keymap::{Binding, Keymaps, ModeKey}; use improvise_protocol::viewmodel::flatten_cat_tree; use improvise_protocol::{ - AppMode, Command, GridExtent, KeyCode, KeyEvent, KeyModifiers, Panel, ViewState, + AppMode, CellId, Command, GridExtent, KeyCode, KeyEvent, KeyModifiers, ModelCommand, Panel, + ViewState, }; use crate::state::ClientState; +/// The one keymap, built once. +fn keymaps() -> &'static Keymaps { + static KEYMAPS: OnceLock = OnceLock::new(); + KEYMAPS.get_or_init(Keymaps::default_keymaps) +} + /// A browser `KeyboardEvent`, flattened to the fields that matter. /// /// `key` is `KeyboardEvent.key` — the printed character for a character key, @@ -156,9 +151,9 @@ impl BrowserKey { } /// Shift is deliberately dropped for character keys: the browser already - /// applied it (`"A"`, not shift+`"a"`), and the host's `lookup_local` - /// retries character keys without modifiers for that very reason. Keeping - /// the flag would only make an exact binding miss. + /// applied it (`"A"`, not shift+`"a"`), and the shared keymap's + /// `lookup_local` retries character keys without modifiers for that very + /// reason. Keeping the flag would only make an exact binding miss. fn modifiers(&self, code: KeyCode) -> KeyModifiers { let mut mods = KeyModifiers::NONE; if self.ctrl { @@ -196,15 +191,17 @@ pub enum KeyOutcome { /// frame and in this order (gotcha 36: the asker's own fold is its own; /// the host forwards only to the others). /// - /// A `Vec` because one keypress is one `Cmd` but several `Effect`s: a + /// A `Vec` because one keypress is one binding but several `Effect`s: a /// cursor move is `SetSelected` plus whichever scroll offsets follow it, - /// exactly as `viewport_effects` returns them. Never empty — a keypress - /// this client can answer with nothing is [`KeyOutcome::Ignored`]. + /// exactly as `viewport_effects` returns them, and a `Binding::Sequence` + /// is several commands to begin with. Never empty — a keypress this + /// client can answer with nothing is [`KeyOutcome::Ignored`]. Local(Vec), - /// Not resolvable from this client's state. Forwarded to the host, which - /// today answers nothing — see the module docs. + /// Bound to a command this client cannot compose. Forwarded to the host, + /// which today answers nothing — see the module docs. Upstream(KeyEvent), - /// Nothing to send. + /// Nothing to send: either nothing is bound, or what is bound resolves + /// to no change. Ignored, } @@ -224,85 +221,227 @@ impl KeyOutcome { /// Decide a keypress against this client's own state. /// -/// Pure. The dispatch is by mode, because the host's is: `ModeKey::from_app_mode` -/// picks one keymap and the keymaps do not overlap. Order matters in one -/// place — a panel's text-entry sub-mode reports both a minibuffer and a -/// panel (`AppMode::panel` counts `CategoryAdd` as the category panel), and -/// typing wins. +/// Pure. The lookup is the shared one: `ModeKey::from_app_mode` picks the +/// keymap — including the `SearchMode` override, which is `Normal` plus a +/// flag rather than a mode — and [`Keymaps::lookup`] walks parent keymaps +/// and the `AnyChar`/`Any` fallbacks exactly as the terminal's dispatch +/// does. What differs is only what happens to the **name** that comes back. pub fn resolve(state: &ClientState, event: KeyEvent) -> KeyOutcome { - let view = &state.view; - match view.mode { - // `SearchMode` is `Normal` plus a flag, so it must be tested before - // the mode and not as part of it. - _ if view.search_mode => KeyOutcome::Upstream(event), - _ if view.mode.minibuffer().is_some() => text_entry(view, event), - _ if view.mode.panel().is_some() => panel_cursor(state, event), - AppMode::Normal | AppMode::RecordsNormal => grid_cursor(state, event), - AppMode::TileSelect => tile_cursor(state, event), - // Everything else. A wildcard rather than an exhaustive list because - // upstream really is the default here, not a per-mode decision: a - // mode this client has never been taught is one it must not guess at. - _ => KeyOutcome::Upstream(event), + // `Quit` has no keymap, on purpose: `ModeKey::from_app_mode` is + // exhaustive so that a new mode without bindings is a compile error + // rather than a mode that swallows every key. + let Some(mode) = ModeKey::from_app_mode(&state.view.mode, state.view.search_mode) else { + return KeyOutcome::Ignored; + }; + // Nothing bound is not "the host might know better" — the host looks the + // key up in this same table. It is `Ignored`, and that is new: it used to + // go upstream into an empty vec. + let Some(binding) = keymaps().lookup(&mode, event.code, event.modifiers) else { + return KeyOutcome::Ignored; + }; + match binding { + // Arming a prefix is composable (`Command::SetTransientPrefix`); what + // the second key then runs is not. See the module docs. + Binding::Prefix(_) => KeyOutcome::Upstream(event), + Binding::Cmd { name, args } => match step(state, event, name, args) { + Some(commands) => KeyOutcome::of(commands), + None => KeyOutcome::Upstream(event), + }, + // All or nothing. A sequence is one keypress, and half of `Esc` — + // clearing the buffer without leaving the mode — is worse than none + // of it. Each step is resolved against the *unchanged* state, because + // the host resolves every step of a sequence against one immutable + // `CmdContext`; the commands are then folded in order. + Binding::Sequence(steps) => match sequence(state, event, steps) { + Some(commands) => KeyOutcome::of(commands), + None => KeyOutcome::Upstream(event), + }, } } -// ── minibuffer text entry ─────────────────────────────────────────────── +/// Every step of a `Binding::Sequence`, or none of them. +fn sequence( + state: &ClientState, + event: KeyEvent, + steps: &[(&'static str, Vec)], +) -> Option> { + let mut commands = Vec::new(); + for (name, args) in steps { + commands.extend(step(state, event, name, args)?); + } + Some(commands) +} -/// The rule reads `AppMode::minibuffer()`, which is the same field the -/// host's `TextEntry` keymap builder reads. -fn text_entry(view: &ViewState, event: KeyEvent) -> KeyOutcome { - let Some(minibuf) = view.mode.minibuffer() else { - return KeyOutcome::Upstream(event); - }; - // `read_buffer`/`write_buffer` treat the name "search" as the - // `search_query` field rather than an entry in `buffers`. No text-entry - // mode uses that name (`no_text_entry_mode_types_into_the_search_buffer` - // holds it), so the rule below does not need the special case — but it - // must not silently acquire it either. - if minibuf.buffer_key == "search" { - return KeyOutcome::Upstream(event); - } - let current = || { - view.buffers - .get(&minibuf.buffer_key) - .cloned() - .unwrap_or_default() - }; - let set = |value: String| { - KeyOutcome::Local(vec![Command::SetBuffer { - name: minibuf.buffer_key.clone(), - value, - }]) - }; - match event.code { - // A modified character is the one place the `AnyChar` argument still - // bites: in a text-entry mode the host's fallback catches `Ctrl-x` - // as `append-char` unless the mode binds it exactly, and mirroring - // that fallback order is not worth the round trip it saves. - KeyCode::Char(c) if event.modifiers == KeyModifiers::NONE => { - let mut value = current(); +/// The subset of command names this client can turn into `Command`s. +/// +/// `None` means "the host's", and every `None` is a dead binding in a +/// browser until something projects what it needs. Adding a name here is the +/// whole of growing what the browser can do. +/// +/// Each arm mirrors its `Cmd` line for line, **including where siblings +/// disagree** (`doctrine(mirror_the_cmd_line_for_line)`): `Move` emits +/// `SetSelected` even when it clamps and `MovePanelCursor` does not, and +/// tidying that into one rule would make this a second derivation. +fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Option> { + match name { + // ── the grid cursor ────────────────────────────────────────── + "move-selection" => grid_move(state, GridMove::Relative(num(args, 0)?, num(args, 1)?)), + "scroll-rows" => grid_move(state, GridMove::Relative(num(args, 0)?, 0)), + "jump-first-row" => grid_move(state, GridMove::ToStart(true)), + "jump-first-col" => grid_move(state, GridMove::ToStart(false)), + "jump-last-row" => grid_move(state, GridMove::ToEnd(true)), + "jump-last-col" => grid_move(state, GridMove::ToEnd(false)), + "page-scroll" => grid_move(state, GridMove::Page(num(args, 0)?)), + + // ── the other two cursors ──────────────────────────────────── + "move-panel-cursor" => panel_cursor(state, args.first()?, num(args, 1)?), + "move-tile-cursor" => tile_cursor(state, num(args, 0)?), + + // ── minibuffer text entry ──────────────────────────────────── + "append-char" => { + let KeyCode::Char(c) = event.code else { + return Some(Vec::new()); + }; + let mut value = read_buffer(&state.view, args.first()?); value.push(c); - set(value) + Some(vec![write_buffer(args.first()?, value)]) } - // CommandMode rebinds Backspace to a command that leaves the mode on - // an empty buffer, so it is not ours to answer. - KeyCode::Backspace - if event.modifiers == KeyModifiers::NONE - && !matches!(view.mode, AppMode::CommandMode { .. }) => - { - let mut value = current(); + "pop-char" => { + let mut value = read_buffer(&state.view, args.first()?); value.pop(); - set(value) + Some(vec![write_buffer(args.first()?, value)]) } - _ => KeyOutcome::Upstream(event), + "clear-buffer" => Some(vec![write_buffer(args.first()?, String::new())]), + + // ── mode entry ─────────────────────────────────────────────── + "enter-mode" => { + let mode = AppMode::from_name(args.first()?)?; + Some(entered(mode)) + } + + // ── panels ─────────────────────────────────────────────────── + "toggle-panel-visibility" => { + let panel = named_panel(args.first()?)?; + Some(vec![Command::SetPanelOpen { + panel, + open: !panel_open(&state.view, panel), + }]) + } + // The interactive arm of `TogglePanelAndFocus`: open-and-focus unless + // this panel is already both, in which case close and unfocus. The + // decision is made in `registry.rs` on the host and here on the + // client, from the same two `ViewState` facts. + "toggle-panel-and-focus" => { + let panel = named_panel(args.first()?)?; + let closing = panel_open(&state.view, panel) && state.view.mode.panel() == Some(panel); + Some(vec![ + Command::SetPanelOpen { + panel, + open: !closing, + }, + Command::ChangeMode(match closing { + true => AppMode::Normal, + false => panel.mode(), + }), + ]) + } + + // `EnterTileSelect` gates on `model.category_names().len() > 0`, + // which counts the **virtual** categories too — `_Index`, `_Dim` and + // `_Measure` always exist (gotcha 7), so the gate is invariantly + // true and there is nothing here to mirror it against. Re-deriving + // it from `CategoryTreeCache`, which carries the panel's rows rather + // than `category_names()`, would be a second derivation that + // disagrees on exactly the empty document. + "enter-tile-select" => Some(vec![ + Command::SetTileCatIdx(0), + Command::ChangeMode(AppMode::TileSelect), + ]), + + // ── the view's shape ───────────────────────────────────────── + // + // `TransposeAxes` also emits `mark_dirty`, which is host-local + // bookkeeping and has no wire variant (gotcha 45). The cursor reset + // is not this client's to send either: the effect calls + // `App::reset_cursor`, and `adopt_cursor_epoch` broadcasts + // `Command::ResetCursor` to *everyone* in answer (gotcha 24) — so + // sending one here would be a second, differently-addressed copy. + "transpose" => Some(vec![Command::Model { + basis: state.basis.claim([])?, + cmd: ModelCommand::TransposeAxes, + }]), + + // ── editing ────────────────────────────────────────────────── + "edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?), + "commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down), + "commit-and-advance-right" => { + commit(state, AppMode::from_name(args.first()?)?, Advance::Right) + } + + _ => None, } } +/// The panel a `` argument names — `parse_panel`'s spellings, which +/// `Panel::name` is required to match (gotcha 12). +fn named_panel(name: &str) -> Option { + Panel::ALL.into_iter().find(|p| p.name() == name) +} + +fn panel_open(view: &ViewState, panel: Panel) -> bool { + match panel { + Panel::Formula => view.formula_panel_open, + Panel::Category => view.category_panel_open, + Panel::View => view.view_panel_open, + } +} + +/// One numeric argument, as the registry's interactive parsers read it. +fn num(args: &[String], i: usize) -> Option { + args.get(i)?.parse().ok() +} + +// ── the named buffers ─────────────────────────────────────────────────── +// +// `cmd::core::read_buffer`/`write_buffer` are the only code on the host that +// knows a runtime-named buffer may not be an entry in `buffers` at all +// (improvise-gdl): the name "search" is `ViewState::search_query`. The +// shared keymap binds `append-char ["search"]` in `SearchMode`, so this +// client needs the same pair — and having it is what lets search typing +// resolve here for the first time. + +fn read_buffer(view: &ViewState, name: &str) -> String { + match name { + "search" => view.search_query.clone(), + other => view.buffers.get(other).cloned().unwrap_or_default(), + } +} + +fn write_buffer(name: &str, value: String) -> Command { + match name { + "search" => Command::SetSearchQuery(value), + other => Command::SetBuffer { + name: other.to_string(), + value, + }, + } +} + +/// `EnterMode::execute`: clear the incoming mode's minibuffer, then change. +fn entered(mode: AppMode) -> Vec { + let mut commands = Vec::new(); + if let Some(mb) = mode.minibuffer() { + commands.push(write_buffer(&mb.buffer_key, String::new())); + } + commands.push(Command::ChangeMode(mode)); + commands +} + // ── the grid cursor ───────────────────────────────────────────────────── -/// How a key moves the grid cursor. A mirror of `navigation.rs`'s `MoveKind`, -/// spelled again because `improvise-command` is not on a browser's -/// dependency list and never will be. +/// A mirror of `navigation.rs`'s `MoveKind`, spelled again because +/// `improvise-command` is not on a browser's dependency list and never will +/// be. The *keys* that reach it are no longer spelled twice. #[derive(Debug, Clone, Copy)] enum GridMove { Relative(i32, i32), @@ -313,38 +452,7 @@ enum GridMove { Page(i32), } -/// The Normal-mode movement keys, as `default_keymaps()` binds them. -/// -/// `gg` (`jump-first-row`) is absent and that is not an oversight: it sits -/// behind the `g` prefix, which this client does not resolve. `Enter` -/// (`enter-advance`) is absent too — it aborts the rest of its batch at the -/// bottom-right corner, a control-flow fact about the host's effect chain -/// rather than a cursor position. -fn grid_move(event: KeyEvent) -> Option { - let plain = event.modifiers == KeyModifiers::NONE; - let ctrl = event.modifiers == KeyModifiers::CONTROL; - match event.code { - KeyCode::Char('h') | KeyCode::Left if plain => Some(GridMove::Relative(0, -1)), - KeyCode::Char('j') | KeyCode::Down if plain => Some(GridMove::Relative(1, 0)), - KeyCode::Char('k') | KeyCode::Up if plain => Some(GridMove::Relative(-1, 0)), - KeyCode::Char('l') | KeyCode::Right if plain => Some(GridMove::Relative(0, 1)), - KeyCode::Char('0') | KeyCode::Home if plain => Some(GridMove::ToStart(false)), - KeyCode::Char('$') | KeyCode::End if plain => Some(GridMove::ToEnd(false)), - KeyCode::Char('G') if plain => Some(GridMove::ToEnd(true)), - KeyCode::PageDown if plain => Some(GridMove::Page(1)), - KeyCode::PageUp if plain => Some(GridMove::Page(-1)), - // `scroll-rows`. Bound exactly, under CONTROL, with no `AnyChar` in - // this keymap to fall through to — so mirroring it is a subset. - KeyCode::Char('d') if ctrl => Some(GridMove::Relative(5, 0)), - KeyCode::Char('u') if ctrl => Some(GridMove::Relative(-5, 0)), - _ => None, - } -} - -fn grid_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { - let Some(kind) = grid_move(event) else { - return KeyOutcome::Upstream(event); - }; +fn grid_move(state: &ClientState, kind: GridMove) -> Option> { let GridExtent { rows, cols } = state.grid.extent; let (window_rows, window_cols) = state.window; // Two shapes with nothing to say, kept apart from "not ours". An empty @@ -352,7 +460,7 @@ fn grid_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { // viewport has been sent no grid either, so there is nothing on screen // for a cursor to be in. Neither is a key the host could answer better. if rows == 0 || cols == 0 || window_rows == 0 || window_cols == 0 { - return KeyOutcome::Ignored; + return Some(Vec::new()); } let cursor = state.view.cursor(); let row_max = (rows - 1) as i32; @@ -372,7 +480,7 @@ fn grid_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { ((row + delta).clamp(0, row_max) as usize, cursor.selected.1) } }; - KeyOutcome::of(scrolled_to(state, nr, nc)) + Some(scrolled_to(state, nr, nc)) } /// `SetSelected`, plus whichever scroll offsets have to follow it to keep @@ -407,24 +515,17 @@ fn scrolled_to(state: &ClientState, nr: usize, nc: usize) -> Vec { // ── the panel cursors ─────────────────────────────────────────────────── -/// `move-panel-cursor ±1`, clamped against the same length the host -/// clamps against. +/// `MovePanelCursor::execute`, and deliberately line for line: an empty +/// panel produces nothing, a cursor past the end is clamped *before* the +/// delta is applied, and a move that lands where it started says nothing. /// /// The category panel's length is `flatten_cat_tree`, which depends on /// `expanded_cats` — per-client, which is why it is derived rather than sent, /// and why the derivation lives in `improvise-protocol` where both ends /// reach it (improvise-edp). The other two panels are flat lists. -fn panel_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { +fn panel_cursor(state: &ClientState, panel: &str, delta: i32) -> Option> { let view = &state.view; - let Some(panel) = view.mode.panel() else { - return KeyOutcome::Upstream(event); - }; - let plain = event.modifiers == KeyModifiers::NONE; - let delta: i32 = match event.code { - KeyCode::Char('j') | KeyCode::Down if plain => 1, - KeyCode::Char('k') | KeyCode::Up if plain => -1, - _ => return KeyOutcome::Upstream(event), - }; + let panel = named_panel(panel)?; let (current, max) = match panel { Panel::Formula => (view.formula_cursor, state.formulas.formulas.len()), Panel::Category => ( @@ -433,22 +534,15 @@ fn panel_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { ), Panel::View => (view.view_panel_cursor, state.views.views.len()), }; - KeyOutcome::of(moved_panel_cursor(panel, delta, current, max)) -} - -/// `MovePanelCursor::execute`, and deliberately line for line: an empty -/// panel produces nothing, a cursor past the end is clamped *before* the -/// delta is applied, and a move that lands where it started says nothing. -fn moved_panel_cursor(panel: Panel, delta: i32, current: usize, max: usize) -> Vec { if max == 0 { - return Vec::new(); + return Some(Vec::new()); } let clamped = current.min(max - 1); let new = (clamped as i32 + delta).clamp(0, (max - 1) as i32) as usize; - match new == current { + Some(match new == current { true => Vec::new(), false => vec![Command::SetPanelCursor { panel, cursor: new }], - } + }) } // ── the tile cursor ───────────────────────────────────────────────────── @@ -459,23 +553,174 @@ fn moved_panel_cursor(panel: Panel, delta: i32, current: usize, max: usize) -> V /// Unlike the panel cursor this emits even when the index does not move, /// because `MoveTileCursor` does — a fold that lands on the value already /// there is not worth a second rule. -fn tile_cursor(state: &ClientState, event: KeyEvent) -> KeyOutcome { - let plain = event.modifiers == KeyModifiers::NONE; - let delta: i32 = match event.code { - KeyCode::Char('l') | KeyCode::Right if plain => 1, - KeyCode::Char('h') | KeyCode::Left if plain => -1, - _ => return KeyOutcome::Upstream(event), - }; +fn tile_cursor(state: &ClientState, delta: i32) -> Option> { let count = state.tiles.tiles.len(); if count == 0 { - return KeyOutcome::Ignored; + return Some(Vec::new()); } let new = (state.view.tile_cat_idx as i32 + delta).clamp(0, (count - 1) as i32) as usize; - KeyOutcome::Local(vec![Command::SetTileCatIdx(new)]) + Some(vec![Command::SetTileCatIdx(new)]) } + +// ── editing a cell ────────────────────────────────────────────────────── + +/// The cell under the cursor, as this client's viewport-indexed cache holds +/// it. `None` when the selection is scrolled out of the window this client +/// was sent — which is not a state the cursor keys can produce, since they +/// scroll to follow, but is one a projection race can. +fn cell_at_cursor(state: &ClientState) -> Option<&improvise_protocol::CellDisplay> { + let cursor = state.view.cursor(); + let row = cursor.selected.0.checked_sub(cursor.row_offset)?; + let col = cursor.selected.1.checked_sub(cursor.col_offset)?; + state.grid.cell(row, col) +} + +/// `EditOrDrill::execute`, with ADR 002 answering the branch. +/// +/// The host asks whether a regular category sits on `Axis::None` and whether +/// the cursor is on a synthetic records cell. This client asks neither: the +/// server already published the answer as the `CellId` variant, and +/// `doctrine(a_demo_is_checked_against_the_type_not_the_axes)` says to read +/// the type rather than re-derive the rule. `Record` is editable, `Aggregate` +/// is a drill target. +/// +/// **Drill does not go upstream.** Every `Upstream` is a black hole, and a +/// key that visibly does nothing is the worst of the three answers. Drill is +/// session-local residue (gotcha 30) and reaching it needs a `CellKey` this +/// client cannot build, so the honest answer is to say so: a `SetStatus`, +/// which is a view-half command — `Sessions::apply` folds it into the +/// asker's own mirror and broadcasts nothing, so it reaches this user and no +/// one else. +fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option> { + let cell = cell_at_cursor(state)?; + match cell.id.as_ref()? { + // `edit_text`, never `text`: the drawn text has been through + // `format_f64`, so a stored 1200 reads "1,200" under a + // comma-grouping format and committing it would store the *string* + // "1,200". `tests/lockstep.rs` found exactly that. + CellId::Record(_) => Some(vec![ + Command::SetBuffer { + name: "edit".to_string(), + value: cell.edit_text.clone().unwrap_or_default(), + }, + Command::ChangeMode(mode), + ]), + CellId::Aggregate(_) => Some(vec![Command::SetStatus( + "drilling into an aggregate needs the host — not in the browser yet".to_string(), + )]), + } +} + +/// Which way `CommitAndAdvance` moves after writing. +#[derive(Debug, Clone, Copy)] +enum Advance { + Down, + Right, +} + +/// `CommitAndAdvance::execute`, for the one case a thin client can compose. +/// +/// The host's version calls `commit_cell_value`, which forks three ways on +/// `synthetic_record_info` and `has_drill_state` (gotcha 1, gotcha 19). Two +/// of those forks need `GridLayout::record_col` and a staging map this client +/// does not have, so this covers the third: a plain grid cell the server +/// addressed with a `CellId::Record`. Anything else is the host's, and +/// [`the_command_names_that_still_need_the_host_are_these`] says so. +/// +/// The composition of the write itself is **not spelled again here** — it is +/// `Client::typed_cell`'s, the same one click-to-edit uses, so a browser +/// keyboard commit and a browser mouse commit cannot disagree about what +/// `""` or `"42"` means. +/// +/// After the write the mirror is line for line: +/// `change_mode(exit_mode_for(edit_mode))`, then `EnterAdvance` — which +/// **aborts the rest of the batch** at the bottom-right corner, so the +/// re-entry below is skipped and the user lands in normal mode — then +/// `EnterEditAtCursor`, which seeds the `edit` buffer from the cell it +/// landed on. The trailing `clear-buffer` that the keymap welds to every +/// commit then empties that buffer again (gotcha 11); that is what the TUI +/// does and mirroring it is the point. +fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option> { + let cursor = state.view.cursor(); + let (window_rows, window_cols) = state.window; + if window_rows == 0 || window_cols == 0 { + return None; + } + // Records mode is the fork this client does not have: a row is one + // coordinate tuple and a category column renames every measure cell in + // it, which needs `record_col`. + if state.view.mode.is_records() { + return None; + } + let text = read_buffer(&state.view, "edit"); + let mut commands = vec![Command::Model { + basis: state.basis.claim([record_at_cursor(state)?])?, + cmd: crate::socket::typed_cell_command(cell_at_cursor(state)?.id.as_ref()?, &text)?, + }]; + commands.push(Command::ChangeMode(AppMode::Normal)); + + let GridExtent { rows, cols } = state.grid.extent; + let row_max = rows.saturating_sub(1); + let col_max = cols.saturating_sub(1); + let (row, col) = cursor.selected; + let landed = match advance { + Advance::Down if row < row_max => Some((row + 1, col)), + Advance::Down if col < col_max => Some((0, col + 1)), + // `EnterAdvance` returns `AbortChain` at the bottom-right corner, so + // the re-entry never runs and the commit exits editing. That abort is + // a fact about the host's effect chain; here it is simply the arm + // that appends nothing more. + Advance::Down => None, + // `Right` has no abort: it clamps at the rightmost column and the + // re-entry always runs. + Advance::Right => Some((row, (col + 1).min(col_max))), + }; + let Some((nr, nc)) = landed else { + return Some(commands); + }; + commands.extend(scrolled_to(state, nr, nc)); + // `EnterEditAtCursor`: seed the buffer from where we landed, then change + // mode. Reading the *new* cell out of a cache that has not yet seen the + // write is correct — it is a different cell — and a cell scrolled out of + // the window seeds empty, as `display_value_at_cursor` does off the grid. + let seeded = seed_at(state, nr, nc); + commands.push(Command::SetBuffer { + name: "edit".to_string(), + value: seeded, + }); + commands.push(Command::ChangeMode(edit_mode)); + Some(commands) +} + +/// The surrogate naming the record under the cursor, or `None` when the +/// cursor is not on a writable cell. +fn record_at_cursor(state: &ClientState) -> Option { + match cell_at_cursor(state)?.id.as_ref()? { + CellId::Record(cell) => Some(cell.record), + CellId::Aggregate(_) => None, + } +} + +/// The display text at an absolute grid position, as the cache holds it. +fn seed_at(state: &ClientState, row: usize, col: usize) -> String { + let cursor = state.view.cursor(); + let Some(r) = row.checked_sub(cursor.row_offset) else { + return String::new(); + }; + let Some(c) = col.checked_sub(cursor.col_offset) else { + return String::new(); + }; + state + .grid + .cell(r, c) + .and_then(|cell| cell.edit_text.clone()) + .unwrap_or_default() +} + #[cfg(test)] mod tests { use super::*; + use improvise_protocol::{CellDisplay, GridUpdate, Projection, RecordCell, Snapshot}; fn ev(code: KeyCode) -> KeyEvent { KeyEvent::new(code, KeyModifiers::NONE) @@ -496,10 +741,7 @@ mod tests { } /// A client in `mode` with **empty caches** — no grid, no panels, no - /// tiles, and no announced window. Everything the grid/panel/tile - /// resolvers need is therefore absent, which is what makes this the right - /// fixture for the text-entry rules and the wrong one for the cursors - /// (those are driven against a real host in `tests/lockstep.rs`). + /// tiles, and no announced window. fn in_mode(mode: AppMode) -> ClientState { ClientState { view: ViewState { @@ -551,9 +793,9 @@ mod tests { assert_eq!(shifted.to_key_event().unwrap().code, KeyCode::BackTab); } - /// Shift is already in the character, and the host's lookup retries - /// character keys without modifiers — carrying the flag can only make an - /// exact binding miss. + /// Shift is already in the character, and the shared keymap's lookup + /// retries character keys without modifiers — carrying the flag can only + /// make an exact binding miss. #[test] fn shift_is_dropped_for_characters_and_kept_for_named_keys() { let shifted_char = BrowserKey { @@ -603,7 +845,7 @@ mod tests { assert_eq!(BrowserKey::new("Unidentified").to_key_event(), None); } - // ── local vs upstream ─────────────────────────────────────────── + // ── text entry ────────────────────────────────────────────────── #[test] fn typing_a_character_in_any_text_entry_mode_resolves_locally() { @@ -623,6 +865,27 @@ mod tests { } } + /// The old classifier sent a modified character upstream in a text-entry + /// mode, because the host's `AnyChar` fallback matches under **any** + /// modifier and a hand-written rule could only be a subset of it safely. + /// Reading the same table removes the question: `Ctrl-x` falls through to + /// `AnyChar` here exactly as it does there. + #[test] + fn a_modified_character_now_types_because_the_fallback_is_the_shared_one() { + let mut state = in_mode(AppMode::editing()); + state.view.buffers.insert("edit".into(), "R".into()); + assert_eq!( + resolve( + &state, + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL) + ), + KeyOutcome::Local(vec![Command::SetBuffer { + name: "edit".into(), + value: "Rx".into(), + }]) + ); + } + #[test] fn backspace_pops_locally_everywhere_except_command_mode() { for mode in text_entry_modes() { @@ -650,97 +913,229 @@ mod tests { } } + /// `Esc` is a `Binding::Sequence` — clear the buffer, then leave — and + /// this client now resolves both halves. Half of it would be worse than + /// none: a cleared buffer in a mode the user is still in. #[test] - fn a_commit_or_cancel_key_is_never_resolved_locally() { - for mode in text_entry_modes() { - let state = in_mode(mode.clone()); - for code in [KeyCode::Enter, KeyCode::Esc, KeyCode::Tab, KeyCode::Up] { - assert_eq!( - resolve(&state, ev(code)), - KeyOutcome::Upstream(ev(code)), - "{} / {code:?} needs the host", - mode.label() - ); - } - } + fn escape_clears_the_buffer_and_leaves_the_mode() { + let mut state = in_mode(AppMode::editing()); + state + .view + .buffers + .insert("edit".into(), "half typed".into()); + assert_eq!( + resolve(&state, ev(KeyCode::Esc)), + KeyOutcome::Local(vec![ + Command::SetBuffer { + name: "edit".into(), + value: String::new(), + }, + Command::ChangeMode(AppMode::Normal), + ]) + ); } - /// Local resolution may be a strict subset of the host's behaviour and - /// never a superset; `AnyChar` matches under any modifier on the host, so - /// a modified character is left to it. + /// Search typing resolves here for the first time, and only because the + /// keymap is shared: `SearchMode` binds `append-char ["search"]`, and the + /// name "search" is `ViewState::search_query` rather than an entry in + /// `buffers` (gotcha 11, improvise-gdl). The old classifier sent every + /// search key upstream into an empty vec. #[test] - fn a_modified_character_goes_upstream_even_in_a_text_entry_mode() { - let state = in_mode(AppMode::editing()); - let event = KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL); - assert_eq!(resolve(&state, event), KeyOutcome::Upstream(event)); + fn search_typing_writes_the_query_and_not_a_buffer() { + let mut state = in_mode(AppMode::Normal); + state.view.search_mode = true; + state.view.search_query = "Nor".into(); + state.grid.extent = GridExtent { rows: 9, cols: 9 }; + state.window = (9, 9); + assert_eq!( + resolve(&state, ev(KeyCode::Char('t'))), + KeyOutcome::Local(vec![Command::SetSearchQuery("Nort".into())]), + "j/t in search mode types; it must not move the cursor" + ); + assert_eq!( + resolve(&state, ev(KeyCode::Esc)), + KeyOutcome::Upstream(ev(KeyCode::Esc)), + "exit-search-mode owns the query and is the host's" + ); } - /// The keys `Normal` mode still hands to the host, named one by one so - /// that closing one of these gaps has to come here and say so. + // ── what is bound and what is not ─────────────────────────────── + + /// The command **names** this client cannot execute, one by one, so that + /// closing one of these gaps has to come here and say so. /// - /// Every one is a binding whose `Command` needs something no projection - /// carries: a prefix's sub-keymap (`g`, `y`, `Z`), the category a row - /// label belongs to (`H`), group membership (`z`), the host's `Storage` - /// (`Ctrl-s`), an arbitrary command line (`:`), or the abort semantics of - /// an effect chain (`Enter`). See the module docs. + /// This replaced a list of *keys*, and that is the whole gain of sharing + /// the keymap: a rebinding no longer invalidates the list, and a new + /// binding for an already-executable command works in a browser with no + /// change to this crate. + /// + /// Each entry is driven through the key that reaches it, because a name + /// that is not bound anywhere would otherwise sit here forever looking + /// like a gap. #[test] - fn the_normal_mode_keys_that_still_need_the_host_are_these() { - let state = in_mode(AppMode::Normal); - for event in [ - ev(KeyCode::Char('g')), - ev(KeyCode::Char('y')), - ev(KeyCode::Char('Z')), - ev(KeyCode::Char('H')), - ev(KeyCode::Char('z')), - ev(KeyCode::Char('x')), - ev(KeyCode::Char('p')), - ev(KeyCode::Char('t')), - ev(KeyCode::Char('[')), - ev(KeyCode::Char(':')), - ev(KeyCode::Enter), - KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL), + fn the_command_names_that_still_need_the_host_are_these() { + let mut state = in_mode(AppMode::Normal); + state.grid.extent = GridExtent { rows: 4, cols: 4 }; + state.window = (4, 4); + for (key, name) in [ + (ev(KeyCode::Char('g')), ""), + (ev(KeyCode::Char('y')), ""), + (ev(KeyCode::Char('Z')), ""), + (ev(KeyCode::Char('H')), "hide-selected-row-item"), + (ev(KeyCode::Char('z')), "toggle-group-under-cursor"), + (ev(KeyCode::Char('x')), "clear-cell"), + (ev(KeyCode::Char('p')), "paste"), + (ev(KeyCode::Char('[')), "page-prev"), + (ev(KeyCode::Char(']')), "page-next"), + (ev(KeyCode::Char('>')), "drill-into-cell"), + (ev(KeyCode::Char('<')), "view-back"), + (ev(KeyCode::Char('/')), "search"), + (ev(KeyCode::Char('n')), "search-navigate"), + (ev(KeyCode::Char('N')), "search-or-category-add"), + (ev(KeyCode::Char('R')), "toggle-records-mode"), + (ev(KeyCode::Char('P')), "toggle-prune-empty"), + (ev(KeyCode::Tab), "cycle-panel-focus"), + (ev(KeyCode::Enter), "enter-advance"), + ( + KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL), + "save", + ), + ( + KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL), + "force-quit", + ), ] { assert_eq!( - resolve(&state, event), - KeyOutcome::Upstream(event), - "{:?} was resolved locally; if that is deliberate, this list \ - is where it stops being a gap", - event.code + resolve(&state, key), + KeyOutcome::Upstream(key), + "{name} ({:?}) was resolved locally; if that is deliberate, \ + this list is where it stops being a gap", + key.code ); } } + /// The four keys an independent browser smoke test found inert, plus the + /// two the editing path needs. Each asserts the *shape* of what is now + /// composed, because "resolves locally" alone would pass on a wrong + /// command. + #[test] + fn the_keys_the_smoke_test_found_inert_now_resolve() { + let state = in_mode(AppMode::Normal); + + assert_eq!( + resolve(&state, ev(KeyCode::Char('?'))), + KeyOutcome::Local(vec![Command::ChangeMode(AppMode::Help)]), + "? opens help" + ); + assert_eq!( + resolve(&state, ev(KeyCode::Char('T'))), + KeyOutcome::Local(vec![ + Command::SetTileCatIdx(0), + Command::ChangeMode(AppMode::TileSelect), + ]), + "T enters tile select" + ); + assert_eq!( + resolve(&state, ev(KeyCode::Char('F'))), + KeyOutcome::Local(vec![ + Command::SetPanelOpen { + panel: Panel::Formula, + open: true, + }, + Command::ChangeMode(AppMode::FormulaPanel), + ]), + "F opens and focuses the formula panel" + ); + assert_eq!( + resolve( + &state, + KeyEvent::new(KeyCode::Char('f'), KeyModifiers::CONTROL) + ), + KeyOutcome::Local(vec![Command::SetPanelOpen { + panel: Panel::Formula, + open: true, + }]), + "Ctrl-f shows the panel without focusing it" + ); + } + + /// `F` on an already-focused panel closes it, which is the interactive + /// arm of `TogglePanelAndFocus` and not its script arm. + #[test] + fn a_focused_panel_toggles_shut() { + let mut state = in_mode(AppMode::FormulaPanel); + state.view.formula_panel_open = true; + assert_eq!( + resolve(&state, ev(KeyCode::Char('F'))), + KeyOutcome::Local(vec![ + Command::SetPanelOpen { + panel: Panel::Formula, + open: false, + }, + Command::ChangeMode(AppMode::Normal), + ]) + ); + } + + /// `t` is a document mutation, so it carries a basis — and until one has + /// been published there is nothing truthful to claim, which is the + /// fourth `None` shape `Client::set_cell` already has (ADR 001). + #[test] + fn transpose_needs_a_published_basis() { + let state = in_mode(AppMode::Normal); + assert_eq!( + resolve(&state, ev(KeyCode::Char('t'))), + KeyOutcome::Upstream(ev(KeyCode::Char('t'))), + "with no basis this client has nothing it may claim" + ); + + let client = client_on(record_cell()); + assert_eq!( + resolve(client.state(), ev(KeyCode::Char('t'))), + KeyOutcome::Local(vec![Command::Model { + basis: client.state().basis.claim([]).unwrap(), + cmd: ModelCommand::TransposeAxes, + }]) + ); + } + /// A mode this client was never taught answers nothing itself. `Help` and /// `ImportWizard` are the two: the wizard's whole keymap is one `Any` /// binding into `HandleWizardKey`, and a half-open dialogue is addressed /// to the session that opened it (gotcha 32). #[test] fn an_untaught_mode_resolves_nothing() { - for mode in [AppMode::Help, AppMode::ImportWizard] { - let state = in_mode(mode.clone()); - for code in [KeyCode::Char('j'), KeyCode::Down, KeyCode::Enter] { - assert_eq!( - resolve(&state, ev(code)), - KeyOutcome::Upstream(ev(code)), - "{} / {code:?}", - mode.label() - ); - } + for code in [KeyCode::Char('j'), KeyCode::Down, KeyCode::Enter] { + assert_eq!( + resolve(&in_mode(AppMode::ImportWizard), ev(code)), + KeyOutcome::Upstream(ev(code)), + "the wizard is the host's" + ); } } + /// A key nothing binds is `Ignored`, not `Upstream` — and that is new. + /// This client reads the same table the host does, so "nothing is bound" + /// is a fact it now holds rather than a guess it defers. + #[test] + fn an_unbound_key_is_ignored_rather_than_sent_into_the_void() { + let state = in_mode(AppMode::Normal); + assert_eq!(resolve(&state, ev(KeyCode::F(9))), KeyOutcome::Ignored); + assert_eq!( + resolve(&state, KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)), + KeyOutcome::Ignored + ); + } + /// An empty widget is not an unanswerable key: the client knows perfectly - /// well that there is nowhere to go, and says `Ignored` rather than - /// forwarding a key the host would also answer with nothing. - /// - /// The three are separate because they are three different emptinesses — - /// a grid with no rows, a panel with no entries, a tile bar with no - /// tiles — and a resolver that forgot one would fail only on that one. + /// well that there is nowhere to go. #[test] fn an_empty_widget_answers_ignored_rather_than_forwarding() { for (mode, code) in [ (AppMode::Normal, KeyCode::Char('j')), (AppMode::TileSelect, KeyCode::Char('l')), + (AppMode::FormulaPanel, KeyCode::Char('j')), ] { assert_eq!( resolve(&in_mode(mode.clone()), ev(code)), @@ -749,24 +1144,11 @@ mod tests { mode.label() ); } - // A panel cursor over an empty panel says nothing for the same - // reason `MovePanelCursor` returns no effects when `max` is zero. - assert_eq!( - resolve(&in_mode(AppMode::FormulaPanel), ev(KeyCode::Char('j'))), - KeyOutcome::Ignored - ); } /// Which widgets say nothing about a move that goes nowhere is **not a /// rule of this module** — it is whatever the `Cmd` being mirrored does, /// and the three differ. - /// - /// `Move` clamps and emits `SetSelected` regardless, so a `k` at the top - /// row still says "be at row 0". `MovePanelCursor` returns no effects - /// when the cursor does not move. `MoveTileCursor` emits regardless, like - /// `Move`. Tempting to unify — and wrong: the value of mirroring line for - /// line is that a lockstep test compares two things that were derived the - /// same way, and a tidier client is a second derivation. #[test] fn a_move_that_goes_nowhere_follows_the_command_it_mirrors() { let mut grid = in_mode(AppMode::Normal); @@ -803,45 +1185,221 @@ mod tests { ); } - /// The host's `read_buffer`/`write_buffer` route the name "search" to - /// `search_query` rather than to `buffers`. This client does not carry - /// that special case, which is only safe while no text-entry mode uses - /// the name — search is `Normal` plus a flag, not a minibuffer mode. - #[test] - fn no_text_entry_mode_types_into_the_search_buffer() { - for mode in text_entry_modes() { - assert_ne!( - mode.minibuffer().unwrap().buffer_key, - "search", - "{} would need the search special case", - mode.label() - ); - } + // ── the editing path ──────────────────────────────────────────── + + fn surrogate() -> improvise_protocol::RecordHash { + serde_json::from_str("\"fe4c8e5e9cbd578e0d388f4b2b5a7879\"").expect("a surrogate is hex") } - /// Search mode is `Normal` with a flag, so the guard above cannot fire — - /// but if a future mode carried both, the flag wins and the key goes to - /// the host that owns the query. - /// - /// Driven in two modes on purpose. `editing()` is the one the buffer - /// guard would also catch, so on its own it cannot tell the flag from the - /// buffer name; `Normal` is the mode search actually occurs in, where - /// `j` would otherwise move the cursor instead of typing into the query. + fn record_cell() -> CellId { + CellId::Record(RecordCell { + record: surrogate(), + measure: Some("Revenue".into()), + }) + } + + /// A one-by-two grid whose first cell carries `id`, with a published + /// basis naming its record — the shape a client is in when the user is + /// about to type. + fn client_on(id: CellId) -> crate::Client { + let mut first = CellDisplay::number("1200"); + first.id = Some(id); + let mut client = crate::init(); + client.bootstrap(Snapshot { + view: ViewState::default(), + seed: vec![ + Projection::Grid(GridUpdate::RowLabels(vec!["North".into(), "South".into()])), + Projection::Grid(GridUpdate::ColLabels(vec!["Revenue".into()])), + Projection::Grid(GridUpdate::Cells(vec![ + ((0, 0), first), + ((1, 0), CellDisplay::number("900")), + ])), + Projection::Grid(GridUpdate::Extent(GridExtent { rows: 2, cols: 1 })), + Projection::Basis(published()), + ], + }); + client.state_mut().window = (2, 1); + client + } + + fn published() -> improvise_protocol::Basis { + improvise_protocol::basis::published(7, 3, [(surrogate(), 4)].into_iter().collect()) + } + + /// `i` on a `Record` cell seeds the edit buffer from what is on screen + /// and enters `Editing` — `EditOrDrill`'s non-aggregated branch, in the + /// order that command emits it. #[test] - fn search_mode_always_goes_upstream() { - for mode in [AppMode::editing(), AppMode::Normal] { - let mut state = in_mode(mode.clone()); - state.view.search_mode = true; - state.grid.extent = GridExtent { rows: 9, cols: 9 }; - state.window = (9, 9); - for code in [KeyCode::Char('q'), KeyCode::Char('j')] { - assert_eq!( - resolve(&state, ev(code)), - KeyOutcome::Upstream(ev(code)), - "{} / {code:?} belongs to the query, not to this client", - mode.label() - ); + fn i_on_a_record_cell_enters_editing_seeded_from_the_cell() { + let client = client_on(record_cell()); + assert_eq!( + resolve(client.state(), ev(KeyCode::Char('i'))), + KeyOutcome::Local(vec![ + Command::SetBuffer { + name: "edit".into(), + value: "1200".into(), + }, + Command::ChangeMode(AppMode::editing()), + ]) + ); + } + + /// `a` is bound to the same command with the same argument, so it must + /// behave identically — and it does so without a second arm here, which + /// is the point of sharing the table. + #[test] + fn a_and_i_are_one_binding_with_two_keys() { + let client = client_on(record_cell()); + assert_eq!( + resolve(client.state(), ev(KeyCode::Char('a'))), + resolve(client.state(), ev(KeyCode::Char('i'))) + ); + } + + /// `i` on an aggregate says so. It must not go upstream: `Host::handle` + /// answers a `Key` frame with an empty vec, so that would be a keypress + /// the user sees do nothing at all + /// (`doctrine(an_upstream_key_is_a_black_hole)`). + #[test] + fn i_on_an_aggregate_declines_out_loud() { + let client = client_on(CellId::Aggregate( + serde_json::from_str("\"aa4c8e5e9cbd578e0d388f4b2b5a7879\"").unwrap(), + )); + let KeyOutcome::Local(commands) = resolve(client.state(), ev(KeyCode::Char('i'))) else { + panic!("an aggregate must not be a silent no-op"); + }; + assert!( + matches!(&commands[..], [Command::SetStatus(msg)] if msg.contains("drill")), + "expected a status naming drill, got {commands:?}" + ); + } + + /// Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]`, + /// and every part of that sequence shows up here — including the + /// trailing clear that empties the buffer `EnterEditAtCursor` has just + /// seeded. That ordering looks wrong and is exactly what the TUI does + /// (gotcha 11); mirroring it line for line is the contract. + #[test] + fn enter_commits_the_cell_advances_and_re_enters_editing() { + let mut client = client_on(record_cell()); + client.state_mut().view.mode = AppMode::editing(); + client + .state_mut() + .view + .buffers + .insert("edit".into(), "42".into()); + let KeyOutcome::Local(commands) = resolve(client.state(), ev(KeyCode::Enter)) else { + panic!("Enter in Editing must resolve here"); + }; + // The write, composed through the same function click-to-edit uses. + assert_eq!( + commands[0], + Command::Model { + basis: published(), + cmd: ModelCommand::SetCell { + cell: RecordCell { + record: surrogate(), + measure: Some("Revenue".into()), + }, + value: improvise_protocol::CellValue::Number(42.0), + }, } - } + ); + assert_eq!(commands[1], Command::ChangeMode(AppMode::Normal)); + assert_eq!(commands[2], Command::SetSelected { row: 1, col: 0 }); + assert!( + commands.contains(&Command::ChangeMode(AppMode::editing())), + "the advance re-enters editing on the cell it landed on: {commands:?}" + ); + assert_eq!( + commands.last(), + Some(&Command::SetBuffer { + name: "edit".into(), + value: String::new(), + }), + "the keymap welds `clear-buffer` to every commit: {commands:?}" + ); + } + + /// An empty buffer is `ClearCell`, not `SetCell` of an empty string — + /// the decision `typed()` makes, reached from the keyboard through the + /// same function the mouse reaches it through. + #[test] + fn committing_an_empty_buffer_clears_the_cell() { + let mut client = client_on(record_cell()); + client.state_mut().view.mode = AppMode::editing(); + client + .state_mut() + .view + .buffers + .insert("edit".into(), String::new()); + let KeyOutcome::Local(commands) = resolve(client.state(), ev(KeyCode::Enter)) else { + panic!("Enter in Editing must resolve here"); + }; + assert!(matches!( + &commands[0], + Command::Model { + cmd: ModelCommand::ClearCell(_), + .. + } + )); + } + + /// At the bottom-right corner `EnterAdvance` returns `AbortChain`, so + /// the re-entry is skipped and the commit leaves the user in normal + /// mode. That abort is a fact about the host's effect chain and it has + /// to be mirrored, or the browser would sit in an editor on a cell the + /// terminal has left. + #[test] + fn a_commit_in_the_last_cell_exits_editing_instead_of_advancing() { + // A one-by-one grid, so the only cell *is* the corner. + let mut cell = CellDisplay::number("1200"); + cell.id = Some(record_cell()); + let mut client = crate::init(); + client.bootstrap(Snapshot { + view: ViewState::default(), + seed: vec![ + Projection::Grid(GridUpdate::RowLabels(vec!["North".into()])), + Projection::Grid(GridUpdate::ColLabels(vec!["Revenue".into()])), + Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)])), + Projection::Grid(GridUpdate::Extent(GridExtent { rows: 1, cols: 1 })), + Projection::Basis(published()), + ], + }); + client.state_mut().window = (1, 1); + client.state_mut().view.mode = AppMode::editing(); + client + .state_mut() + .view + .buffers + .insert("edit".into(), "7".into()); + + let KeyOutcome::Local(commands) = resolve(client.state(), ev(KeyCode::Enter)) else { + panic!("Enter in Editing must resolve here"); + }; + assert!( + matches!(&commands[0], Command::Model { .. }), + "the write still happens: {commands:?}" + ); + assert!( + !commands.contains(&Command::ChangeMode(AppMode::editing())), + "the corner must not re-enter editing: {commands:?}" + ); + assert!( + commands.contains(&Command::ChangeMode(AppMode::Normal)), + "it drops to normal instead: {commands:?}" + ); + } + + /// Records mode is the fork this client does not have (gotcha 1), so its + /// commit goes upstream rather than writing the wrong cell. + #[test] + fn a_records_mode_commit_is_the_hosts() { + let mut client = client_on(record_cell()); + client.state_mut().view.mode = AppMode::records_editing(); + assert_eq!( + resolve(client.state(), ev(KeyCode::Enter)), + KeyOutcome::Upstream(ev(KeyCode::Enter)) + ); } } diff --git a/crates/improvise-wasm-client/src/render.rs b/crates/improvise-wasm-client/src/render.rs index c44d8f4c..6a564f43 100644 --- a/crates/improvise-wasm-client/src/render.rs +++ b/crates/improvise-wasm-client/src/render.rs @@ -56,6 +56,26 @@ pub struct RenderModel { pub categories: Vec, /// Whether the panel is open, so the page does not decide. pub category_panel_open: bool, + /// The keys this mode offers, off `AppMode::hint_text()` — the same + /// string the TUI's footer draws. + /// + /// The page shows it rather than spelling any keys of its own, which is + /// what keeps a second front end from growing its own idea of the + /// bindings (design-principles §5). It also makes the modes this client + /// can *enter* but not yet *draw* — `Help` above all — state what they + /// are instead of looking like a broken grid. + pub hint: String, + /// What the user has typed so far, while a keyboard edit is open on the + /// selected cell — `None` when none is. + /// + /// The page draws this in place of the selected cell's own text. The + /// *decision* — is an edit open, and what is in it — is made here, + /// because it is `AppMode::is_editing()` and the `edit` buffer, and a + /// second front end must not have to know either + /// (design-principles §5). Without it a browser that resolved `i` + /// locally would change its mode indicator and then swallow every + /// keystroke invisibly. + pub editing: Option, } /// A grid row: its header label and its cells. @@ -76,6 +96,10 @@ pub struct RenderCell { /// alone. pub editable: bool, pub selected: bool, + /// What an editor opened on this cell should start with — the stored + /// value, not the formatted one. See `CellDisplay::edit_text`: seeding + /// from `text` and committing turns `1200` into the string `"1,200"`. + pub edit_text: String, } impl RenderCell { @@ -86,6 +110,7 @@ impl RenderCell { error: cell.error, editable: matches!(cell.id, Some(CellId::Record(_))), selected, + edit_text: cell.edit_text.clone().unwrap_or_default(), } } } @@ -152,6 +177,12 @@ impl ClientState { .map(|(i, entry)| RenderCatRow::of(entry, i == self.view.cat_panel_cursor)) .collect(), category_panel_open: self.view.category_panel_open, + hint: self.view.mode.hint_text().to_string(), + editing: self + .view + .mode + .is_editing() + .then(|| self.view.buffers.get("edit").cloned().unwrap_or_default()), } } } @@ -371,9 +402,51 @@ mod tests { let model = client.model(); assert_eq!(model.mode, improvise_protocol::AppMode::Normal.label()); assert_eq!(model.status, "saved"); + assert!( + model.hint.contains("i:edit"), + "the page draws the mode's own hint rather than spelling keys: {}", + model.hint + ); assert_eq!(model.revision, client.revision()); } + /// A keyboard edit has to be *visible*, and what is visible is a + /// decision made here rather than in the page. + /// + /// The failure this rules out is quiet: a client that resolved `i` + /// locally would change its mode label, put the typing into + /// `buffers["edit"]`, and draw the cell's old value the whole time — so + /// the user sees a keystroke do nothing, types over it, and commits + /// something they never read. `None` outside an editing mode is the + /// other half: a stale buffer must not draw an editor that is not open. + #[test] + fn a_keyboard_edit_in_progress_is_part_of_the_render_model() { + let mut client = client_with_grid(ViewState::default()); + assert_eq!( + client.model().editing, + None, + "nothing is being edited in Normal" + ); + + client + .state_mut() + .fold(&Command::ChangeMode(improvise_protocol::AppMode::editing())); + client.state_mut().fold(&Command::SetBuffer { + name: "edit".into(), + value: "12".into(), + }); + assert_eq!(client.model().editing.as_deref(), Some("12")); + + client + .state_mut() + .fold(&Command::ChangeMode(improvise_protocol::AppMode::Normal)); + assert_eq!( + client.model().editing, + None, + "leaving the mode closes the editor even with the buffer still set" + ); + } + /// The whole model crosses as JSON, so the page needs no bindings. #[test] fn the_render_model_crosses_as_json() { @@ -384,5 +457,9 @@ mod tests { assert_eq!(value["rows"][0]["cells"][0]["editable"], true); assert_eq!(value["rows"][0]["cells"][1]["editable"], false); assert_eq!(value["col_labels"][0], "Revenue"); + assert_eq!( + value["rows"][0]["cells"][0]["edit_text"], "1200", + "an editor seeds from the stored value, never from the drawn one" + ); } } diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index 7dd6cb3c..27dd49e2 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -77,6 +77,30 @@ fn typed(text: &str) -> Option { } } +/// The `ModelCommand` for "the user typed `text` into this cell". +/// +/// The one composition, shared by the mouse path ([`Client::typed_cell_frame`]) +/// and the keyboard path (`keys::commit`). Writing it twice is how a browser +/// commit by Enter and a browser commit by click would come to disagree about +/// what an empty string means. +/// +/// `None` for an aggregate: `ModelCommand::SetCell` does not take that kind +/// of id, so this is the pattern failing to bind rather than a check (ADR +/// 002). +pub(crate) fn typed_cell_command( + id: &improvise_protocol::CellId, + text: &str, +) -> Option { + let improvise_protocol::CellId::Record(cell) = id else { + return None; + }; + let cell = cell.clone(); + Some(match typed(text) { + Some(value) => ModelCommand::SetCell { cell, value }, + None => ModelCommand::ClearCell(cell), + }) +} + impl Client { /// The handshake frame, and every resize after it. /// @@ -124,10 +148,7 @@ impl Client { let basis = self.state().basis.claim([cell.record])?; Some(ClientFrame::command(Command::Model { basis, - cmd: match typed(text) { - Some(value) => ModelCommand::SetCell { cell, value }, - None => ModelCommand::ClearCell(cell), - }, + cmd: typed_cell_command(&improvise_protocol::CellId::Record(cell), text)?, })) } } diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index e0cea325..1c0427be 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -428,3 +428,179 @@ fn a_locally_resolved_tile_cursor_agrees_with_the_host() { } assert!(client.view.tile_cat_idx > 0, "the fixture moved nothing"); } + +// ── the editing path (improvise-bpq2's second half) ───────────────────── + +/// A document whose cells are **records**, so `i` has something to edit. +/// +/// One regular category on Row, `_Measure` on Column, nothing regular on +/// None — the shape gotcha 46 built `web/demo.improv` to satisfy, and the +/// reason is the same: with a regular category on None every cell is an +/// `Aggregate` and the whole editing path is unreachable. +fn editable_host() -> App { + let mut wb = Workbook::new("Test"); + wb.add_category("Region").unwrap(); + wb.add_item("Region", "North").unwrap(); + wb.add_item("Region", "South").unwrap(); + wb.add_item("_Measure", "Revenue").unwrap(); + wb.active_view_mut().set_axis("Region", Axis::Row); + wb.active_view_mut().set_axis("_Measure", Axis::Column); + wb.model.set_cell( + improvise_command::model::cell::CellKey::new(vec![ + ("Region".to_string(), "North".to_string()), + ("_Measure".to_string(), "Revenue".to_string()), + ]), + improvise_protocol::CellValue::Number(1200.0), + ); + let mut app = App::new(wb, None); + app.term_height = 2 + 8; + app.rebuild_layout(); + app +} + +/// [`client_of`], plus the published basis a real client folds before it is +/// allowed to compose a write (ADR 001). The tiers are the ones this test +/// never checks — the host applies the keypress directly rather than the +/// client's `Command::Model` — but `BasisCache::claim` withholds until it +/// has *something*, so a client without one resolves no commit at all. +fn client_ready_to_edit(app: &App, window: (usize, usize)) -> ClientState { + let mut client = client_of(app, window); + let records: std::collections::BTreeMap<_, _> = client + .grid + .cells + .iter() + .flatten() + .filter_map(|cell| match cell.id.as_ref() { + Some(improvise_protocol::CellId::Record(r)) => Some((r.record, 0u64)), + _ => None, + }) + .collect(); + assert!( + !records.is_empty(), + "the fixture must project at least one Record cell, or nothing here is testing editing" + ); + improvise_protocol::WidgetCache::apply( + &mut client.basis, + &improvise_protocol::basis::published(0, 0, records), + ); + client +} + +/// The headline claim for the editing half: `i`, three characters and `Esc` +/// leave the client and the host in the same mode with the same buffers. +/// +/// `Esc` rather than `Enter` for the cancel arm, because it is a +/// `Binding::Sequence` and so exercises the all-or-nothing rule: half of it — +/// a cleared buffer in a mode the user has not left — is worse than none. +#[test] +fn entering_typing_and_cancelling_an_edit_agrees_with_the_host() { + let window = (2, 1); + let mut app = editable_host(); + let mut client = client_ready_to_edit(&app, window); + + for key in ["i", "9", "9", "9", "Escape"] { + let event = BrowserKey::new(key).to_key_event().unwrap(); + let KeyOutcome::Local(cmds) = resolve(&client, event) else { + panic!("{key}: the client must answer this key"); + }; + for cmd in &cmds { + client.fold(cmd); + } + reduce_key(&mut app, event); + + assert_eq!( + format!("{:?}", client.view.mode), + format!("{:?}", app.view_state.mode), + "{key}: client and host disagree about the mode" + ); + assert_eq!( + client.view.buffers, app.view_state.buffers, + "{key}: client and host disagree about the buffers" + ); + } + + assert!( + matches!(app.view_state.mode, AppMode::Normal), + "Esc must have left editing, or the walk above proved nothing" + ); +} + +/// `i` seeds the edit buffer from the cell's **display value**, and the two +/// ends must agree on what that is: the host reads +/// `App::display_value_at_cursor`, the client reads the text the server +/// projected into its `GridCache`. Those are two paths to one string, and +/// this is the assertion that they are the same string. +#[test] +fn the_buffer_i_seeds_is_the_value_the_host_would_seed() { + let window = (2, 1); + let mut app = editable_host(); + let mut client = client_ready_to_edit(&app, window); + + let event = BrowserKey::new("i").to_key_event().unwrap(); + let KeyOutcome::Local(cmds) = resolve(&client, event) else { + panic!("i on a record cell must resolve locally"); + }; + for cmd in &cmds { + client.fold(cmd); + } + reduce_key(&mut app, event); + + assert_eq!( + client.view.buffers.get("edit"), + app.view_state.buffers.get("edit"), + "the seeded buffer must be the host's display value" + ); + assert_eq!( + client.view.buffers.get("edit").map(String::as_str), + Some("1200"), + "and the fixture must actually have a value, or this passes on two empties" + ); +} + +/// The commit: `Enter` in `Editing` moves both ends to the same mode, the +/// same cursor and the same buffers. +/// +/// What is *not* compared is the document, and deliberately: the client +/// answers with a `Command::Model`, which `reduce_view` drops, while +/// `reduce_key` writes through the effect pipeline. The write's composition +/// is asserted in `keys.rs` against `typed_cell`'s own — the shared one — so +/// what is left to prove here is the half a unit test cannot see, which is +/// that the advance and the re-entry land in the same place. +#[test] +fn a_locally_committed_edit_leaves_both_ends_in_the_same_place() { + let window = (2, 1); + let mut app = editable_host(); + let mut client = client_ready_to_edit(&app, window); + + for key in ["i", "7", "Enter"] { + let event = BrowserKey::new(key).to_key_event().unwrap(); + let KeyOutcome::Local(cmds) = resolve(&client, event) else { + panic!("{key}: the client must answer this key"); + }; + for cmd in &cmds { + client.fold(cmd); + } + reduce_key(&mut app, event); + + assert_eq!( + format!("{:?}", client.view.mode), + format!("{:?}", app.view_state.mode), + "{key}: client and host disagree about the mode" + ); + assert_eq!( + client.view.cursor(), + app.view_state.cursor(), + "{key}: client and host disagree about the cursor" + ); + assert_eq!( + client.view.buffers, app.view_state.buffers, + "{key}: client and host disagree about the buffers" + ); + } + + assert_eq!( + app.view_state.cursor().selected, + (1, 0), + "the commit must have advanced a row, or the walk proved nothing" + ); +} diff --git a/web/app.js b/web/app.js index c37a053d..03fee4d1 100644 --- a/web/app.js +++ b/web/app.js @@ -84,6 +84,7 @@ const el = { grid: document.getElementById("grid"), categories: document.getElementById("categories"), mode: document.getElementById("mode"), + hint: document.getElementById("hint"), status: document.getElementById("status"), viewName: document.getElementById("view-name"), }; @@ -233,6 +234,9 @@ function repaint() { painted = model.revision; el.mode.textContent = model.mode; + // The mode's own hint, off `AppMode::hint_text()`. The page spells no + // keys itself; see `RenderModel::hint`. + el.hint.textContent = model.hint; modelStatus = model.status; paintStatus(); el.viewName.textContent = model.view_name; @@ -261,15 +265,23 @@ function gridTable(model) { row.cells.forEach((cell, c) => { const td = tr.insertCell(); - td.textContent = cell.text; - td.classList.toggle("numeric", cell.numeric); + // A keyboard edit is open on the selected cell when `model.editing` is + // a string; it holds what has been typed so far. Both facts are + // decided in Rust (`RenderModel::editing`) — the page does not know + // which modes are editing modes or which buffer they type into. + // Without this the mode indicator would say INSERT while every + // keystroke vanished into a buffer nobody drew. + const typing = cell.selected && typeof model.editing === "string"; + td.textContent = typing ? model.editing + "\u258f" : cell.text; + td.classList.toggle("numeric", cell.numeric && !typing); td.classList.toggle("error", cell.error); td.classList.toggle("selected", cell.selected); + td.classList.toggle("typing", typing); // `editable` is the server's answer relayed through Rust: a stored // cell. An aggregate is a drill target and drill is not on the wire // yet, so it is simply not offered here. td.classList.toggle("editable", cell.editable); - if (cell.editable) td.addEventListener("click", () => edit(td, r, c)); + if (cell.editable) td.addEventListener("click", () => edit(td, r, c, cell.edit_text)); }); }); return table; @@ -277,10 +289,13 @@ function gridTable(model) { /** Open an inline editor. Committing sends a frame; the value that comes * back is the server's, never the string typed here. */ -function edit(td, row, col) { +function edit(td, row, col, seed) { if (editing) return; editing = { row, col }; - const original = td.textContent; + // `seed` is `RenderCell::edit_text` -- the STORED value, not the drawn + // one. `td.textContent` has been through the number format, so a stored + // 1200 reads "1,200" and committing that stores the string "1,200". + const original = seed; const input = document.createElement("input"); input.value = original; td.replaceChildren(input); diff --git a/web/index.html b/web/index.html index 51188856..197a2128 100644 --- a/web/index.html +++ b/web/index.html @@ -36,6 +36,7 @@

improvise

+ diff --git a/web/style.css b/web/style.css index 0c3e9f7f..901908f1 100644 --- a/web/style.css +++ b/web/style.css @@ -89,6 +89,18 @@ td.selected { outline: 1px solid var(--accent); } +#hint { + opacity: 0.7; + font-size: 0.85em; +} + +/* A keyboard edit in progress. Left-aligned even in a numeric column, + because what is drawn is the text being typed rather than a value. */ +td.typing { + text-align: left; + outline: 2px solid var(--accent); +} + #grid { flex: 1; overflow: auto;