diff --git a/src/command/cmd/commit.rs b/src/command/cmd/commit.rs index fa78d4c..b969be8 100644 --- a/src/command/cmd/commit.rs +++ b/src/command/cmd/commit.rs @@ -137,14 +137,70 @@ mod tests { // ── Commit commands (mode-specific buffer consumers) ──────────────────────── /// Commit a cell value: for synthetic records keys, stage in drill pending edits -/// or apply directly; for real keys, write to the model. -fn commit_cell_value(key: &CellKey, value: &str, effects: &mut Vec>) { +/// in drill mode, or apply directly in plain records mode; for real keys, write +/// to the model. +fn commit_cell_value( + ctx: &CmdContext, + key: &CellKey, + value: &str, + effects: &mut Vec>, +) { if let Some((record_idx, col_name)) = crate::view::synthetic_record_info(key) { - effects.push(Box::new(effect::SetDrillPendingEdit { - record_idx, - col_name, - new_value: value.to_string(), + if ctx.has_drill_state { + effects.push(Box::new(effect::SetDrillPendingEdit { + record_idx, + col_name, + new_value: value.to_string(), + })); + return; + } + + let Some((orig_key, _)) = ctx + .layout + .records + .as_ref() + .and_then(|records| records.get(record_idx)) + else { + return; + }; + + if col_name == "Value" { + if value.is_empty() { + effects.push(Box::new(effect::ClearCell(orig_key.clone()))); + } else if let Ok(n) = value.parse::() { + effects.push(Box::new(effect::SetCell( + orig_key.clone(), + CellValue::Number(n), + ))); + } else { + effects.push(Box::new(effect::SetCell( + orig_key.clone(), + CellValue::Text(value.to_string()), + ))); + } + effects.push(effect::mark_dirty()); + return; + } + + if value.is_empty() { + effects.push(effect::set_status("Record coordinates cannot be empty")); + return; + } + + let Some(existing_value) = ctx.model.get_cell(orig_key).cloned() else { + return; + }; + effects.push(Box::new(effect::ClearCell(orig_key.clone()))); + effects.push(Box::new(effect::AddItem { + category: col_name.clone(), + item: value.to_string(), })); + effects.push(Box::new(effect::SetCell( + orig_key.clone().with(col_name, value), + existing_value, + ))); + effects.push(effect::mark_dirty()); + return; } else if value.is_empty() { effects.push(Box::new(effect::ClearCell(key.clone()))); effects.push(effect::mark_dirty()); @@ -187,7 +243,7 @@ impl Cmd for CommitAndAdvance { } fn execute(&self, ctx: &CmdContext) -> Vec> { let mut effects: Vec> = Vec::new(); - commit_cell_value(&self.key, &self.value, &mut effects); + commit_cell_value(ctx, &self.key, &self.value, &mut effects); match self.advance { AdvanceDir::Down => { let adv = EnterAdvance { diff --git a/src/command/cmd/core.rs b/src/command/cmd/core.rs index fa868c0..d9d2742 100644 --- a/src/command/cmd/core.rs +++ b/src/command/cmd/core.rs @@ -36,6 +36,8 @@ pub struct CmdContext<'a> { /// View navigation stacks (for drill back/forward) pub view_back_stack: &'a [String], pub view_forward_stack: &'a [String], + /// Whether the app currently has an active drill snapshot. + pub has_drill_state: bool, /// Display value at the cursor — works uniformly for pivot and records mode. pub display_value: String, /// How many data rows/cols fit on screen (for viewport scrolling). diff --git a/src/command/cmd/mod.rs b/src/command/cmd/mod.rs index aa0daf3..79191c0 100644 --- a/src/command/cmd/mod.rs +++ b/src/command/cmd/mod.rs @@ -69,6 +69,7 @@ pub(super) mod test_helpers { buffers: &EMPTY_BUFFERS, view_back_stack: &[], view_forward_stack: &[], + has_drill_state: false, display_value: { let key = layout.cell_key(sr, sc); key.as_ref() diff --git a/src/ui/app.rs b/src/ui/app.rs index 1fec708..cd9f411 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -288,6 +288,7 @@ impl App { tile_cat_idx: self.tile_cat_idx, view_back_stack: &self.view_back_stack, view_forward_stack: &self.view_forward_stack, + has_drill_state: self.drill_state.is_some(), display_value: { let key = layout.cell_key(sel_row, sel_col); if let Some(k) = &key { @@ -741,6 +742,126 @@ mod tests { ); } + /// Regression: editing the first row in a blank model's records view + /// should persist the typed value even though plain records mode does not + /// use drill state. + #[test] + fn edit_record_row_in_blank_model_persists_value() { + use crate::model::cell::CellKey; + + let mut app = App::new(Model::new("T"), None); + + app.handle_key(KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Char('o'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Char('5'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) + .unwrap(); + + assert_eq!( + app.model.get_cell(&CellKey::new(vec![])), + Some(&crate::model::cell::CellValue::Number(5.0)), + "editing a synthetic row in plain records mode should write the value" + ); + } + + /// Drill-view edits should stay staged in drill state until the user + /// navigates back, at which point ApplyAndClearDrill writes them through. + #[test] + fn drill_edit_is_staged_until_view_back() { + use crate::model::cell::{CellKey, CellValue}; + + let mut m = Model::new("T"); + m.add_category("Region").unwrap(); + m.add_category("Month").unwrap(); + m.category_mut("Region").unwrap().add_item("East"); + m.category_mut("Month").unwrap().add_item("Jan"); + let record_key = CellKey::new(vec![ + ("Month".to_string(), "Jan".to_string()), + ("Region".to_string(), "East".to_string()), + ]); + m.set_cell(record_key.clone(), CellValue::Number(1.0)); + let mut app = App::new(m, None); + + app.handle_key(KeyEvent::new(KeyCode::Char('>'), KeyModifiers::NONE)) + .unwrap(); + assert!(app.drill_state.is_some(), "drill should create drill state"); + let value_col = (0..app.layout.col_count()) + .find(|&col| app.layout.col_label(col) == "Value") + .expect("drill view should include a Value column"); + app.model.active_view_mut().selected = (0, value_col); + app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Char('9'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) + .unwrap(); + + assert_eq!( + app.model.get_cell(&record_key), + Some(&CellValue::Number(1.0)), + "drill edit should remain staged until leaving the drill view" + ); + assert_eq!( + app.drill_state + .as_ref() + .and_then(|s| s.pending_edits.get(&(0, "Value".to_string()))), + Some(&"9".to_string()), + "drill edit should be recorded in pending_edits" + ); + + app.handle_key(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Char('<'), KeyModifiers::NONE)) + .unwrap(); + + assert_eq!( + app.model.get_cell(&record_key), + Some(&CellValue::Number(9.0)), + "leaving drill view should apply the staged edit" + ); + } + + /// Suspected bug: blanking a records-mode category coordinate should not + /// create an item with an empty name. + #[test] + fn blanking_records_category_does_not_create_empty_item() { + use crate::model::cell::{CellKey, CellValue}; + + let mut m = Model::new("T"); + m.add_category("Region").unwrap(); + m.category_mut("Region").unwrap().add_item("East"); + m.set_cell( + CellKey::new(vec![("Region".to_string(), "East".to_string())]), + CellValue::Number(1.0), + ); + let mut app = App::new(m, None); + + app.handle_key(KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE)) + .unwrap(); + app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)) + .unwrap(); + for _ in 0..4 { + app.handle_key(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE)) + .unwrap(); + } + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) + .unwrap(); + + assert!( + !app.model + .category("Region") + .unwrap() + .items + .contains_key(""), + "records-mode edits should not create empty category items" + ); + } + #[test] fn command_mode_buffer_cleared_on_reentry() { use crossterm::event::KeyEvent; diff --git a/src/ui/effect.rs b/src/ui/effect.rs index b00d853..7617632 100644 --- a/src/ui/effect.rs +++ b/src/ui/effect.rs @@ -458,6 +458,10 @@ impl Effect for ApplyAndClearDrill { }; app.model.set_cell(orig_key.clone(), value); } else { + if new_value.is_empty() { + app.status_msg = "Record coordinates cannot be empty".to_string(); + continue; + } // Rename a coordinate: remove old cell, insert new with updated coord let value = match app.model.get_cell(orig_key) { Some(v) => v.clone(),