feat(cmd): allow direct commit of synthetic records in records mode

Allow synthetic record edits to be applied directly to the model when not
in drill mode, whereas they remain staged in drill state when a drill
snapshot is active. This enables editing of records in plain records view.

Additionally, add validation to prevent creating records with empty
coordinates in both direct commits and when applying staged drill edits.
Includes regression tests for persistence in blank models, drill state
staging, and empty coordinate prevention.

Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
This commit is contained in:
Edward Langley
2026-04-14 02:32:03 -07:00
parent c79498b04b
commit 9710fb534e
5 changed files with 191 additions and 7 deletions

View File

@ -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<Box<dyn Effect>>) {
/// 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<Box<dyn Effect>>,
) {
if let Some((record_idx, col_name)) = crate::view::synthetic_record_info(key) {
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::<f64>() {
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<Box<dyn Effect>> {
let mut effects: Vec<Box<dyn Effect>> = 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 {

View File

@ -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).

View File

@ -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()

View File

@ -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;

View File

@ -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(),