From 030865a0fff13a9be48305e1b8dd3b19184eb8ed Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 15 Apr 2026 21:32:35 -0700 Subject: [PATCH] feat(records): implement records mode for data entry Implement a new "Records" mode for data entry. - Add `RecordsNormal` and `RecordsEditing` to `AppMode` and `ModeKey` . - `DataStore` now uses `IndexMap` and supports `sort_by_key()` to ensure deterministic row order. - `ToggleRecordsMode` command now sorts data and switches to `RecordsNormal` . - `EnterEditMode` command now respects records editing variants. - `RecordsNormal` mode includes a new `o` keybinding to add a record row. - `RecordsEditing` mode inherits from `Editing` and adds an `Esc` binding to return to `RecordsNormal` . - Added `SortData` effect to trigger data sorting. - Updated UI to display "RECORDS" and "RECORDS INSERT" mode names and styles. - Updated keymaps, command registry, and view navigation to support these new modes. - Added comprehensive tests for records mode behavior, including sorting and boundary conditions for Tab/Enter. Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf) --- src/command/cmd/commit.rs | 35 ++++++-- src/command/cmd/grid.rs | 4 + src/command/cmd/mode.rs | 9 +- src/command/cmd/registry.rs | 2 + src/command/keymap.rs | 40 +++++++-- src/draw.rs | 6 +- src/model/cell.rs | 20 +++++ src/ui/app.rs | 167 ++++++++++++++++++++++++++++++++++-- src/ui/effect.rs | 14 ++- src/view/layout.rs | 85 ++++++++++++++++-- 10 files changed, 350 insertions(+), 32 deletions(-) diff --git a/src/command/cmd/commit.rs b/src/command/cmd/commit.rs index 7fe2ca2..e4c67f1 100644 --- a/src/command/cmd/commit.rs +++ b/src/command/cmd/commit.rs @@ -3,6 +3,7 @@ use crate::ui::app::AppMode; use crate::ui::effect::{self, Effect}; use super::core::{Cmd, CmdContext}; +use super::grid::AddRecordRow; use super::navigation::{CursorState, EnterAdvance, viewport_effects}; #[cfg(test)] @@ -259,15 +260,31 @@ impl Cmd for CommitAndAdvance { } AdvanceDir::Right => { let col_max = self.cursor.col_count.saturating_sub(1); - let nc = (self.cursor.col + 1).min(col_max); - effects.extend(viewport_effects( - self.cursor.row, - nc, - self.cursor.row_offset, - self.cursor.col_offset, - self.cursor.visible_rows, - self.cursor.visible_cols, - )); + let row_max = self.cursor.row_count.saturating_sub(1); + let at_bottom_right = self.cursor.row >= row_max && self.cursor.col >= col_max; + + if at_bottom_right && ctx.is_records_mode() { + let add = AddRecordRow; + effects.extend(add.execute(ctx)); + effects.extend(viewport_effects( + self.cursor.row + 1, + 0, + self.cursor.row_offset, + self.cursor.col_offset, + self.cursor.visible_rows, + self.cursor.visible_cols, + )); + } else { + let nc = (self.cursor.col + 1).min(col_max); + effects.extend(viewport_effects( + self.cursor.row, + nc, + self.cursor.row_offset, + self.cursor.col_offset, + self.cursor.visible_rows, + self.cursor.visible_cols, + )); + } } } effects.push(Box::new(effect::EnterEditAtCursor)); diff --git a/src/command/cmd/grid.rs b/src/command/cmd/grid.rs index 178b58d..4e418d3 100644 --- a/src/command/cmd/grid.rs +++ b/src/command/cmd/grid.rs @@ -1,4 +1,5 @@ use crate::model::cell::CellValue; +use crate::ui::app::AppMode; use crate::ui::effect::{self, Effect}; use crate::view::AxisEntry; @@ -414,6 +415,8 @@ impl Cmd for ToggleRecordsMode { let mut effects: Vec> = Vec::new(); let records_name = "_Records".to_string(); + effects.push(Box::new(effect::SortData)); + // Create (or replace) a _Records view and switch to it effects.push(Box::new(effect::CreateView(records_name.clone()))); effects.push(Box::new(effect::SwitchView(records_name))); @@ -435,6 +438,7 @@ impl Cmd for ToggleRecordsMode { })); } } + effects.push(effect::change_mode(AppMode::RecordsNormal)); effects.push(effect::set_status("Records mode")); effects } diff --git a/src/command/cmd/mode.rs b/src/command/cmd/mode.rs index 0a705f3..84ed870 100644 --- a/src/command/cmd/mode.rs +++ b/src/command/cmd/mode.rs @@ -197,13 +197,18 @@ impl Cmd for EnterEditMode { fn name(&self) -> &'static str { "enter-edit-mode" } - fn execute(&self, _ctx: &CmdContext) -> Vec> { + fn execute(&self, ctx: &CmdContext) -> Vec> { + let edit_mode = if ctx.mode.is_records() { + AppMode::records_editing() + } else { + AppMode::editing() + }; vec![ Box::new(effect::SetBuffer { name: "edit".to_string(), value: self.initial_value.clone(), }), - effect::change_mode(AppMode::editing()), + effect::change_mode(edit_mode), ] } } diff --git a/src/command/cmd/registry.rs b/src/command/cmd/registry.rs index 7e61141..13a727f 100644 --- a/src/command/cmd/registry.rs +++ b/src/command/cmd/registry.rs @@ -320,6 +320,8 @@ pub fn default_registry() -> CmdRegistry { "command" => AppMode::command_mode(), "category-add" => AppMode::category_add(), "editing" => AppMode::editing(), + "records-normal" => AppMode::RecordsNormal, + "records-editing" => AppMode::records_editing(), "formula-edit" => AppMode::formula_edit(), "export-prompt" => AppMode::export_prompt(), other => return Err(format!("Unknown mode: {other}")), diff --git a/src/command/keymap.rs b/src/command/keymap.rs index d7bdac5..a0276e3 100644 --- a/src/command/keymap.rs +++ b/src/command/keymap.rs @@ -61,6 +61,8 @@ pub enum ModeKey { CommandMode, SearchMode, ImportWizard, + RecordsNormal, + RecordsEditing, } impl ModeKey { @@ -80,6 +82,8 @@ impl ModeKey { 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), _ => None, } } @@ -450,14 +454,9 @@ impl KeymapSet { 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 / add row + // Drill into aggregated cell / view history normal.bind(KeyCode::Char('>'), none, "drill-into-cell"); normal.bind(KeyCode::Char('<'), none, "view-back"); - normal.bind_seq( - KeyCode::Char('o'), - none, - vec![("add-record-row", vec![]), ("enter-edit-at-cursor", vec![])], - ); // Records mode toggle and prune toggle normal.bind(KeyCode::Char('R'), none, "toggle-records-mode"); @@ -484,7 +483,17 @@ impl KeymapSet { z_map.bind(KeyCode::Char('Z'), none, "wq"); normal.bind_prefix(KeyCode::Char('Z'), none, Arc::new(z_map)); - set.insert(ModeKey::Normal, Arc::new(normal)); + let normal = Arc::new(normal); + set.insert(ModeKey::Normal, normal.clone()); + + // ── Records normal mode (inherits from normal) ──────────────────── + let mut rn = Keymap::with_parent(normal); + rn.bind_seq( + KeyCode::Char('o'), + none, + vec![("add-record-row", vec![]), ("enter-edit-at-cursor", vec![])], + ); + set.insert(ModeKey::RecordsNormal, Arc::new(rn)); // ── Help mode ──────────────────────────────────────────────────── let mut help = Keymap::new(); @@ -754,7 +763,20 @@ impl KeymapSet { ); ed.bind_args(KeyCode::Backspace, none, "pop-char", vec!["edit".into()]); ed.bind_any_char("append-char", vec!["edit".into()]); - set.insert(ModeKey::Editing, Arc::new(ed)); + let ed = Arc::new(ed); + set.insert(ModeKey::Editing, ed.clone()); + + // ── Records editing mode (inherits from editing) ────────────────── + let mut re = Keymap::with_parent(ed); + re.bind_seq( + KeyCode::Esc, + none, + vec![ + ("clear-buffer", vec!["edit".into()]), + ("enter-mode", vec!["records-normal".into()]), + ], + ); + set.insert(ModeKey::RecordsEditing, Arc::new(re)); // ── Formula edit ───────────────────────────────────────────────── let mut fe = Keymap::new(); @@ -1106,6 +1128,8 @@ mod tests { ModeKey::CommandMode, ModeKey::SearchMode, ModeKey::ImportWizard, + ModeKey::RecordsNormal, + ModeKey::RecordsEditing, ]; for mode in &expected_modes { assert!( diff --git a/src/draw.rs b/src/draw.rs index 1d8bcd3..3eedc96 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -133,12 +133,16 @@ fn mode_name(mode: &AppMode) -> &'static str { AppMode::CommandMode { .. } => "COMMAND", AppMode::Help => "HELP", AppMode::Quit => "QUIT", + AppMode::RecordsNormal => "RECORDS", + AppMode::RecordsEditing { .. } => "RECORDS INSERT", } } fn mode_style(mode: &AppMode) -> Style { match mode { - AppMode::Editing { .. } => Style::default().fg(Color::Black).bg(Color::Green), + AppMode::Editing { .. } | AppMode::RecordsEditing { .. } => { + Style::default().fg(Color::Black).bg(Color::Green) + } AppMode::CommandMode { .. } => Style::default().fg(Color::Black).bg(Color::Yellow), AppMode::TileSelect => Style::default().fg(Color::Black).bg(Color::Magenta), _ => Style::default().fg(Color::Black).bg(Color::DarkGray), diff --git a/src/model/cell.rs b/src/model/cell.rs index e1cca01..063251c 100644 --- a/src/model/cell.rs +++ b/src/model/cell.rs @@ -162,6 +162,26 @@ impl DataStore { ) } + /// Sort cells by their CellKey for deterministic display order. + /// Call once on entry into records mode so existing data is ordered; + /// subsequent inserts append at the end. + pub fn sort_by_key(&mut self) { + let symbols = &self.symbols; + self.cells.sort_by(|a, _, b, _| { + let resolve = |k: &InternedKey| -> Vec<(String, String)> { + k.0.iter() + .map(|(c, i)| { + ( + symbols.resolve(*c).to_string(), + symbols.resolve(*i).to_string(), + ) + }) + .collect() + }; + resolve(a).cmp(&resolve(b)) + }); + } + pub fn set(&mut self, key: CellKey, value: CellValue) { let ikey = self.intern_key(&key); // Update index for each coordinate pair diff --git a/src/ui/app.rs b/src/ui/app.rs index 1d75bcc..ff87803 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -79,6 +79,12 @@ pub enum AppMode { }, Help, Quit, + /// Records-mode normal: inherits from Normal with records-specific bindings. + RecordsNormal, + /// Records-mode editing: inherits from Editing with boundary-aware Tab/Enter. + RecordsEditing { + minibuf: MinibufferConfig, + }, } impl AppMode { @@ -86,6 +92,7 @@ impl AppMode { pub fn minibuffer(&self) -> Option<&MinibufferConfig> { match self { Self::Editing { minibuf, .. } + | Self::RecordsEditing { minibuf, .. } | Self::FormulaEdit { minibuf, .. } | Self::CommandMode { minibuf, .. } | Self::CategoryAdd { minibuf, .. } @@ -110,6 +117,21 @@ impl AppMode { } } + pub fn records_editing() -> Self { + Self::RecordsEditing { + minibuf: MinibufferConfig { + buffer_key: "edit", + prompt: "edit: ".into(), + color: Color::Green, + }, + } + } + + /// True when this mode is a records-mode variant. + pub fn is_records(&self) -> bool { + matches!(self, Self::RecordsNormal | Self::RecordsEditing { .. }) + } + pub fn formula_edit() -> Self { Self::FormulaEdit { minibuf: MinibufferConfig { @@ -408,7 +430,12 @@ impl App { AppMode::Normal => { "hjkl:nav i:edit R:records P:prune F/C/V:panels T:tiles [:]:page >:drill ::cmd" } - AppMode::Editing { .. } => "Enter:commit Tab:commit+right Esc:cancel", + AppMode::Editing { .. } | AppMode::RecordsEditing { .. } => { + "Enter:commit Tab:commit+right Esc:cancel" + } + AppMode::RecordsNormal => { + "hjkl:nav i:edit o:add-row R:pivot P:prune <:back ::cmd" + } AppMode::FormulaPanel => "n:new d:delete jk:nav Esc:back", AppMode::FormulaEdit { .. } => "Enter:save Esc:cancel — type: Name = expression", AppMode::CategoryPanel => { @@ -726,6 +753,43 @@ mod tests { } /// Regression: pressing `o` in an empty records view should create the + /// Pressing R to enter records mode should sort existing data by CellKey + /// so display order is deterministic regardless of insertion order. + #[test] + fn entering_records_mode_sorts_existing_data() { + use crate::model::cell::{CellKey, CellValue}; + let mut m = Model::new("T"); + m.add_category("Region").unwrap(); + m.category_mut("Region").unwrap().add_item("North"); + m.category_mut("Region").unwrap().add_item("East"); + // Insert in reverse-alphabetical order + m.set_cell( + CellKey::new(vec![("Region".into(), "North".into())]), + CellValue::Number(1.0), + ); + m.set_cell( + CellKey::new(vec![("Region".into(), "East".into())]), + CellValue::Number(2.0), + ); + let mut app = App::new(m, None); + app.handle_key(KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE)) + .unwrap(); + assert!(app.layout.is_records_mode()); + let region_col = (0..app.layout.col_count()) + .find(|&c| app.layout.col_label(c) == "Region") + .unwrap(); + let row0 = app.layout.records_display(0, region_col).unwrap(); + let row1 = app.layout.records_display(1, region_col).unwrap(); + assert_eq!( + row0, "East", + "R should sort existing data: first row should be East" + ); + assert_eq!( + row1, "North", + "R should sort existing data: second row should be North" + ); + } + /// first synthetic row instead of only entering edit mode on empty space. #[test] fn add_record_row_in_empty_records_view_creates_first_row() { @@ -748,7 +812,7 @@ mod tests { "o should create the first record row in an empty records view" ); assert!( - matches!(app.mode, AppMode::Editing { .. }), + app.mode.is_editing(), "o should leave the app in edit mode, got {:?}", app.mode ); @@ -787,14 +851,107 @@ mod tests { .unwrap(); assert_eq!( - app.model.get_cell(&CellKey::new(vec![ - ("_Measure".to_string(), "Rev".to_string()), - ])), + app.model.get_cell(&CellKey::new(vec![( + "_Measure".to_string(), + "Rev".to_string() + ),])), Some(&crate::model::cell::CellValue::Number(5.0)), "editing a synthetic row in plain records mode should write the value" ); } + /// Build a records-mode app with two data rows for testing Tab/Enter + /// behavior at boundaries. Row 0 has _Measure=meas2, row 1 has _Measure=meas1. + fn records_model_with_two_rows() -> App { + use crate::model::cell::{CellKey, CellValue}; + let mut m = Model::new("T"); + m.add_category("Region").unwrap(); + m.category_mut("Region").unwrap().add_item("North"); + m.category_mut("_Measure").unwrap().add_item("meas1"); + m.category_mut("_Measure").unwrap().add_item("meas2"); + m.set_cell( + CellKey::new(vec![ + ("Region".into(), "North".into()), + ("_Measure".into(), "meas2".into()), + ]), + CellValue::Number(10.0), + ); + m.set_cell( + CellKey::new(vec![ + ("Region".into(), "North".into()), + ("_Measure".into(), "meas1".into()), + ]), + CellValue::Number(20.0), + ); + let mut app = App::new(m, None); + app.handle_key(KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE)) + .unwrap(); + assert!( + app.layout.is_records_mode(), + "setup: should be records mode" + ); + assert_eq!(app.layout.row_count(), 2, "setup: should have 2 records"); + let cols: Vec = (0..app.layout.col_count()) + .map(|i| app.layout.col_label(i)) + .collect(); + assert!( + cols.contains(&"Region".to_string()), + "setup: should have Region column; got {:?}", + cols + ); + assert!( + cols.contains(&"_Measure".to_string()), + "setup: should have _Measure column; got {:?}", + cols + ); + assert_eq!( + cols.last().unwrap(), + "Value", + "setup: Value must be last column; got {:?}", + cols + ); + app + } + + /// improvise-hmu: TAB on the bottom-right cell of records view should + /// insert a new record below and move to the first cell of the new row + /// in edit mode. + #[test] + fn tab_on_bottom_right_of_records_inserts_below() { + let mut app = records_model_with_two_rows(); + let initial_rows = app.layout.row_count(); + assert!(initial_rows >= 1, "setup: need at least 1 record"); + + let last_row = initial_rows - 1; + let last_col = app.layout.col_count() - 1; + app.model.active_view_mut().selected = (last_row, last_col); + + // Enter edit mode on the bottom-right cell + app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)) + .unwrap(); + assert!(app.mode.is_editing(), "setup: should be editing"); + + // TAB should commit, insert below, move to first cell of new row + app.handle_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)) + .unwrap(); + + assert_eq!( + app.layout.row_count(), + initial_rows + 1, + "TAB on bottom-right should insert a record below" + ); + assert_eq!( + app.model.active_view().selected, + (initial_rows, 0), + "TAB should move to first cell of the new row" + ); + assert!( + app.mode.is_editing(), + "should enter edit mode on the new cell, got {:?}", + app.mode + ); + } + /// Drill-view edits should stay staged in drill state until the user /// navigates back, at which point ApplyAndClearDrill writes them through. #[test] diff --git a/src/ui/effect.rs b/src/ui/effect.rs index ae2d801..872abde 100644 --- a/src/ui/effect.rs +++ b/src/ui/effect.rs @@ -59,6 +59,14 @@ impl Effect for AddItemInGroup { } } +#[derive(Debug)] +pub struct SortData; +impl Effect for SortData { + fn apply(&self, app: &mut App) { + app.model.data.sort_by_key(); + } +} + #[derive(Debug)] pub struct SetCell(pub CellKey, pub CellValue); impl Effect for SetCell { @@ -127,7 +135,11 @@ impl Effect for EnterEditAtCursor { let value = ctx.display_value.clone(); drop(ctx); app.buffers.insert("edit".to_string(), value); - app.mode = AppMode::editing(); + app.mode = if app.mode.is_records() { + AppMode::records_editing() + } else { + AppMode::editing() + }; } } diff --git a/src/view/layout.rs b/src/view/layout.rs index a549b08..8cc6516 100644 --- a/src/view/layout.rs +++ b/src/view/layout.rs @@ -132,14 +132,12 @@ impl GridLayout { page_coords: Vec<(String, String)>, none_cats: Vec, ) -> Self { - let mut records: Vec<(CellKey, CellValue)> = model + let records: Vec<(CellKey, CellValue)> = model .data .matching_cells(&page_coords) .into_iter() .map(|(k, v)| (k, v.clone())) .collect(); - // Sort for deterministic ordering - records.sort_by(|a, b| a.0.0.cmp(&b.0.0)); // Synthesize row items: one per record, labeled with its index let row_items: Vec = (0..records.len()) @@ -152,7 +150,10 @@ impl GridLayout { .into_iter() .filter(|c| { let kind = model.category(c).map(|cat| cat.kind); - !matches!(kind, Some(CategoryKind::VirtualIndex | CategoryKind::VirtualDim)) + !matches!( + kind, + Some(CategoryKind::VirtualIndex | CategoryKind::VirtualDim) + ) }) .map(String::from) .collect(); @@ -594,8 +595,7 @@ mod tests { CellValue::Number(50.0), ); // Records mode setup: _Measure should not filter - m.active_view_mut() - .set_axis("_Measure", Axis::None); + m.active_view_mut().set_axis("_Measure", Axis::None); m } @@ -730,6 +730,79 @@ mod tests { ); } + /// On initial entry into records mode, rows should be in a deterministic + /// (CellKey-sorted) order regardless of insertion order. + #[test] + fn records_mode_initial_layout_is_sorted() { + let mut m = Model::new("T"); + m.add_category("Region").unwrap(); + m.category_mut("Region").unwrap().add_item("North"); + m.category_mut("Region").unwrap().add_item("East"); + // Insert East before North — opposite of alphabetical + m.set_cell( + CellKey::new(vec![("Region".into(), "East".into())]), + CellValue::Number(2.0), + ); + m.set_cell( + CellKey::new(vec![("Region".into(), "North".into())]), + CellValue::Number(1.0), + ); + // Sort the store before entering records mode, as ToggleRecordsMode would + m.data.sort_by_key(); + let v = m.active_view_mut(); + v.set_axis("_Index", Axis::Row); + v.set_axis("_Dim", Axis::Column); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!(layout.row_count(), 2); + let region_col = (0..layout.col_count()) + .find(|&c| layout.col_label(c) == "Region") + .unwrap(); + let row0_region = layout.records_display(0, region_col).unwrap(); + let row1_region = layout.records_display(1, region_col).unwrap(); + assert_eq!( + row0_region, "East", + "first row should be East (alphabetical)" + ); + assert_eq!( + row1_region, "North", + "second row should be North (alphabetical)" + ); + } + + /// New records added after initial layout should appear at the bottom, + /// not re-sorted into the middle by CellKey order. + #[test] + fn records_mode_new_record_appends_at_bottom() { + let mut m = records_model(); + let v = m.active_view_mut(); + v.set_axis("_Index", Axis::Row); + v.set_axis("_Dim", Axis::Column); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!(layout.row_count(), 2); + let first_value = layout.records_display(0, layout.col_count() - 1).unwrap(); + + // Add a record whose CellKey sorts BEFORE the existing ones + m.set_cell( + CellKey::new(vec![ + ("Region".into(), "AAA".into()), + ("_Measure".into(), "Cost".into()), + ]), + CellValue::Number(999.0), + ); + let layout2 = GridLayout::new(&m, m.active_view()); + assert_eq!(layout2.row_count(), 3); + // First row should still be the same record — new one appends at bottom + let first_value2 = layout2.records_display(0, layout2.col_count() - 1).unwrap(); + assert_eq!( + first_value, first_value2, + "first row should be unchanged after adding a new record; \ + new record should append at bottom, not re-sort" + ); + // New record should be the last row + let last_value = layout2.records_display(2, layout2.col_count() - 1).unwrap(); + assert_eq!(last_value, "999", "new record should be the last row"); + } + fn coord(pairs: &[(&str, &str)]) -> CellKey { CellKey::new( pairs