Files
improvise/crates/improvise-command/src/command/cmd/commit.rs
T
2026-09-27 16:51:50 +00:00

1318 lines
49 KiB
Rust
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
use crate::formula::FormulaId;
use crate::model::category::MEASURE_CATEGORY;
use crate::model::cell::CellKey;
use crate::protocol::FormulaSlot;
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect};
use crate::view::RecordCol;
use improvise_protocol::{OpenedEditor, untouched};
use super::core::{Cmd, CmdContext, read_buffer, write_buffer};
use super::grid::AddRecordRow;
use super::navigation::{CursorState, EnterAdvance, viewport_effects};
#[cfg(test)]
mod tests {
use std::collections::HashMap;
use super::*;
use crate::command::cmd::test_helpers::*;
use crate::model::cell::CellValue;
use crate::workbook::Workbook;
fn commit(kind: CommitKind) -> CommitBuffer {
CommitBuffer { kind }
}
#[test]
fn commit_formula_with_categories_adds_formula() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("formula".to_string(), "Profit = Revenue - Cost".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = commit(CommitKind::Formula).execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("AddFormula"),
"Expected AddFormula, got: {dbg}"
);
assert!(
dbg.contains("FormulaPanel"),
"Expected return to FormulaPanel, got: {dbg}"
);
}
/// Formulas always target _Measure by default, even when no regular
/// categories exist. _Measure is a virtual category that always exists.
#[test]
fn commit_formula_without_regular_categories_targets_measure() {
let m = Workbook::new("Empty");
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("formula".to_string(), "X = Y + Z".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = commit(CommitKind::Formula).execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("AddFormula"),
"Should add formula targeting _Measure, got: {dbg}"
);
assert!(
dbg.contains("_Measure"),
"target_category should be _Measure, got: {dbg}"
);
}
/// improvise-6t3: `set_status("Formula added")` after `AddFormula` overwrote the
/// parse error, so an unparseable formula was dropped and reported a success.
#[test]
fn invalid_formula_reports_the_parse_error_not_success() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("formula".to_string(), "Profit = = ".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = commit(CommitKind::Formula).execute(&ctx);
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.apply_effects(effects);
assert!(
app.view_state.status_msg.contains("Formula error"),
"Expected the parse error to reach the status bar, got: {:?}",
app.view_state.status_msg
);
}
#[test]
fn commit_category_add_with_name_produces_add_effect() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("category".to_string(), "Region".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = commit(CommitKind::Category).execute(&ctx);
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.apply_effects(effects);
assert_eq!(
app.model_state.workbook.model.regular_category_names(),
vec!["Type", "Month", "Region"],
"the category named in the buffer must be the one added"
);
}
#[test]
fn commit_category_add_with_empty_buffer_returns_to_panel() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("category".to_string(), "".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = commit(CommitKind::Category).execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("CategoryPanel"),
"Expected return to CategoryPanel, got: {dbg}"
);
}
/// `commit-buffer item` takes its target category off `AppMode::ItemAdd`
/// at construction time, so the command itself never reads the mode.
#[test]
fn commit_item_add_files_the_item_under_the_mode_s_category() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("item".to_string(), "March".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
let item_add_mode = AppMode::item_add("Month".to_string());
ctx.mode = &item_add_mode;
ctx.buffers = &bufs;
let cmd = reg
.interactive("commit-buffer", &["item".to_string()], &ctx)
.expect("item-add mode should supply the category");
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.apply_effects(cmd.execute(&ctx));
let model = &app.model_state.workbook.model;
assert_eq!(
model.effective_item_names("Month"),
vec!["Jan", "Feb", "March"],
"the item named in the buffer must land in the MODE's category"
);
assert_eq!(model.effective_item_names("Type"), vec!["Food", "Clothing"]);
}
#[test]
fn commit_item_add_outside_item_add_mode_is_a_construction_error() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let ctx = make_ctx(&m, &layout, &reg);
assert!(
reg.interactive("commit-buffer", &["item".to_string()], &ctx)
.is_err()
);
}
/// `CommitAndAdvance` threads its `edit_mode` to the trailing
/// `EnterEditAtCursor`; the command never reads `ctx.mode`.
#[test]
fn commit_and_advance_threads_edit_mode_to_enter_edit_at_cursor() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("edit".to_string(), "42".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
// ctx.mode stays Normal — the command must not look at it.
let key = ctx.cell_key().unwrap();
let cmd = CommitAndAdvance {
key,
value: "42".to_string(),
advance: super::AdvanceDir::Down,
cursor: super::CursorState::from_ctx(&ctx),
edit_mode: AppMode::records_editing(),
opened: None,
};
let effects = cmd.execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("EnterEditAtCursor") && dbg.contains("RecordsEditing"),
"Expected trailing EnterEditAtCursor with RecordsEditing target, got: {dbg}"
);
}
#[test]
fn commit_export_produces_export_and_normal_mode() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("export".to_string(), "/tmp/test.csv".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = CommitExport.execute(&ctx);
// 3 effects: SetBuffer (clear), ChangeMode, ExportCsv (improvise-2mqz).
assert_eq!(effects.len(), 3);
let dbg = effects_debug(&effects);
assert!(dbg.contains("ExportCsv"), "Expected ExportCsv, got: {dbg}");
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
assert!(
dbg.contains("SetBuffer { name: \"export\", value: \"\" }"),
"Expected buffer clear, got: {dbg}"
);
assert!(
dbg.find("Normal").unwrap() < dbg.find("ExportCsv").unwrap(),
"the mode change must precede the write an abort can reach: {dbg}"
);
}
/// BUG (improvise-z05r): the fourth commit kind did not bail on an empty
/// path, so Enter on an empty prompt exported to "".
#[test]
fn commit_export_bails_on_an_empty_path() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
for buffered in ["", " "] {
let mut bufs = HashMap::new();
bufs.insert("export".to_string(), buffered.to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let effects = CommitExport.execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
!dbg.contains("ExportCsv"),
"an empty export path must write nothing, got: {dbg}"
);
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
}
}
/// Naming a new measure in the trailing `+` column at the bottom-right sent the
/// cursor home: `AddItem` resets, and the advance aborts before restoring it.
#[test]
fn naming_a_measure_at_the_bottom_right_leaves_the_cursor_put() {
use crate::model::cell::{CellKey, CellValue};
use crate::ui::app::App;
use crate::view::Axis;
use crate::workbook::Workbook;
let mut wb = Workbook::new("Records");
wb.add_category("Region").unwrap();
wb.add_item("Region", "East").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "East".into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(100.0),
);
{
let v = wb.active_view_mut();
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
v.set_axis("Region", Axis::None);
v.set_axis("_Measure", Axis::None);
}
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(
app.layout.is_records_mode(),
"the fixture is not records mode"
);
let bottom = app.layout.row_count() - 1;
let right = app.layout.col_count() - 1;
assert!(
matches!(app.layout.record_col(right), Some(RecordCol::NewMeasure)),
"the last column is not the new-measure column, so this proves nothing"
);
app.cursor_mut().selected = (bottom, right);
let effects = {
let reg = make_registry();
let mut ctx = make_ctx(&app.model_state.workbook, &app.layout, &reg);
ctx.selected = (bottom, right);
let key = ctx.cell_key().expect("a cell under the cursor");
CommitAndAdvance {
key,
value: "Cost".to_string(),
advance: super::AdvanceDir::Down,
cursor: super::CursorState::from_ctx(&ctx),
edit_mode: AppMode::records_editing(),
opened: None,
}
.execute(&ctx)
};
app.apply_effects(effects);
assert!(
app.model_state
.workbook
.model
.measure_item_names()
.contains(&"Cost".to_string()),
"the measure was not created, so the cursor question is moot"
);
assert_eq!(
app.cursor().selected,
(bottom, right),
"naming a measure at the bottom-right sent the cursor home"
);
}
/// improvise-lae: the save prompt commits to `SaveAs`, not `ExportCsv` (wrong
/// format) and not `Save` (there is no path yet).
#[test]
fn commit_save_writes_the_model_to_the_typed_path() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("save".to_string(), "/tmp/typed.improv".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let dbg = effects_debug(&CommitSave.execute(&ctx));
assert!(
dbg.contains("SaveAs(\"/tmp/typed.improv\")"),
"Expected SaveAs at the typed path, got: {dbg}"
);
assert!(
!dbg.contains("ExportCsv"),
"the save prompt must not export CSV, got: {dbg}"
);
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
}
/// An empty prompt must not reach `SaveAs`: the user would read an I/O error
/// about `""` instead of "you typed nothing".
#[test]
fn commit_save_on_an_empty_path_writes_nothing() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut bufs = HashMap::new();
bufs.insert("save".to_string(), " ".to_string());
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.buffers = &bufs;
let dbg = effects_debug(&CommitSave.execute(&ctx));
assert!(
!dbg.contains("SaveAs"),
"an empty path must not reach SaveAs, got: {dbg}"
);
assert!(dbg.contains("Refuse"), "Expected a refusal, got: {dbg}");
}
/// A one-record records grid with `Region` as its only category and `Revenue` as
/// its only measure: columns are `[Region, +cat, Revenue, +measure]`.
fn one_record_grid() -> crate::ui::app::App {
use crate::model::cell::{CellKey, CellValue};
use crate::ui::app::App;
use crate::view::Axis;
use crate::workbook::Workbook;
let mut wb = Workbook::new("Records");
wb.add_category("Region").unwrap();
wb.add_item("Region", "East").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "East".into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(100.0),
);
{
let v = wb.active_view_mut();
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
v.set_axis("Region", Axis::None);
v.set_axis("_Measure", Axis::None);
}
let mut app = App::new(wb, None);
app.rebuild_layout();
assert!(
app.layout.is_records_mode(),
"the fixture is not records mode"
);
app
}
/// Put the cursor on the trailing `+` column of the last row, in records-normal.
fn park_on_new_measure(app: &mut crate::ui::app::App) -> (usize, usize) {
let bottom = app.layout.row_count() - 1;
let right = app.layout.col_count() - 1;
assert!(
matches!(app.layout.record_col(right), Some(RecordCol::NewMeasure)),
"the last column is not the new-measure column, so this proves nothing"
);
app.cursor_mut().selected = (bottom, right);
app.view_state.mode = AppMode::RecordsNormal;
(bottom, right)
}
fn press(app: &mut crate::ui::app::App, code: improvise_protocol::key::KeyCode) {
use improvise_protocol::key::{KeyEvent, KeyModifiers};
app.handle_key(KeyEvent {
code,
modifiers: KeyModifiers::NONE,
})
.unwrap();
}
/// BUG (improvise-0ygd) — see `context/test-evidence/command.md`.
#[test]
fn tab_naming_a_measure_lands_on_the_column_it_just_created() {
use improvise_protocol::key::KeyCode;
let mut app = one_record_grid();
let (bottom, right) = park_on_new_measure(&mut app);
let rows_before = app.layout.row_count();
press(&mut app, KeyCode::Char('i'));
for ch in "Cost".chars() {
press(&mut app, KeyCode::Char(ch));
}
press(&mut app, KeyCode::Tab);
assert!(
app.model_state
.workbook
.model
.measure_item_names()
.contains(&"Cost".to_string()),
"the measure was not created, so the cursor question is moot"
);
assert_eq!(
app.layout.row_count(),
rows_before,
"Tab on `+measure` manufactured a record row the user never asked for"
);
assert_eq!(
app.cursor().selected,
(bottom, right),
"Tab must land on the inserted measure column, not jump home"
);
assert_eq!(
app.layout.record_col(app.cursor().selected.1),
Some(&RecordCol::Measure("Cost".to_string())),
"the cursor is not on the measure the commit just created"
);
}
/// The bulk-entry contract the fix must not eat: an EMPTY `+` commit creates no
/// column, so Tab there still starts the next record row.
#[test]
fn tab_on_an_empty_new_measure_column_still_starts_a_record_row() {
use improvise_protocol::key::KeyCode;
let mut app = one_record_grid();
park_on_new_measure(&mut app);
let rows_before = app.layout.row_count();
let measures_before = app.model_state.workbook.model.measure_item_names().len();
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Tab);
assert_eq!(
app.model_state.workbook.model.measure_item_names().len(),
measures_before,
"an empty `+` commit must create no measure"
);
assert_eq!(
app.layout.row_count(),
rows_before + 1,
"Tab at the bottom-right of a records grid must start the next record row"
);
assert_eq!(
app.cursor().selected,
(rows_before, 0),
"bulk entry lands at the start of the new row"
);
}
/// Values whose `Display` (four decimals) and value disagree, plus text.
fn values_display_rounds() -> Vec<crate::model::cell::CellValue> {
use crate::model::cell::CellValue::{Number, Text};
vec![
Number(1.23456789),
Number(0.1 + 0.2),
Number(98765.4321987),
Number(123456789012345.67),
Number(-0.00001234),
Number(-0.0),
Number(f64::NEG_INFINITY),
Text("0.1 + 0.2".into()),
]
}
/// Two regions, two measures, with `value` at (West, Revenue): never index 0.
fn grid_holding(value: crate::model::cell::CellValue, records: bool) -> crate::ui::app::App {
use crate::model::cell::{CellKey, CellValue};
use crate::ui::app::App;
use crate::view::Axis;
let mut wb = Workbook::new("Seed");
wb.add_category("Region").unwrap();
for region in ["North", "West"] {
wb.add_item("Region", region).unwrap();
}
for measure in ["Cost", "Revenue"] {
wb.add_item("_Measure", measure).unwrap();
}
let key = |r: &str, m: &str| {
CellKey::new(vec![
("Region".into(), r.into()),
("_Measure".into(), m.into()),
])
};
wb.model
.set_cell(key("North", "Cost"), CellValue::Number(1.0));
wb.model
.set_cell(key("North", "Revenue"), CellValue::Number(2.0));
wb.model
.set_cell(key("West", "Cost"), CellValue::Number(3.0));
wb.model.set_cell(key("West", "Revenue"), value);
{
let v = wb.active_view_mut();
match records {
true => {
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
v.set_axis("Region", Axis::None);
v.set_axis("_Measure", Axis::None);
}
false => {
v.set_axis("Region", Axis::Row);
v.set_axis("_Measure", Axis::Column);
}
}
}
let mut app = App::new(wb, None);
app.rebuild_layout();
assert_eq!(app.layout.is_records_mode(), records, "wrong fixture mode");
app
}
/// Open the editor on (West, Revenue) and commit it untouched; the stored value.
fn no_op_commit(records: bool, value: crate::model::cell::CellValue) {
use crate::model::cell::CellKey;
use improvise_protocol::key::KeyCode;
let mut app = grid_holding(value.clone(), records);
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let at = match records {
true => (1, 3),
false => (1, 1),
};
app.cursor_mut().selected = at;
if records {
app.view_state.mode = AppMode::RecordsNormal;
assert_eq!(
app.layout.record_col(at.1),
Some(&RecordCol::Measure("Revenue".into())),
"the cursor is not on the Revenue column"
);
}
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&value),
"the fixture does not hold the value under test"
);
press(&mut app, KeyCode::Char('i'));
assert!(
app.view_state.mode.is_editing(),
"`i` did not open an editor"
);
assert_eq!(
app.view_state.buffers.get("edit"),
Some(&value.edit_text()),
"the editor was not seeded from the stored value (improvise-umi4)"
);
press(&mut app, KeyCode::Enter);
let stored = app.model_state.workbook.model.get_cell(&target).cloned();
let same = match (&stored, &value) {
(Some(CellValue::Number(a)), CellValue::Number(b)) => a.to_bits() == b.to_bits(),
(stored, value) => stored.as_ref() == Some(value),
};
assert!(
same,
"{} mode: i then Enter on {value:?} stored {stored:?}",
if records { "records" } else { "pivot" }
);
}
/// BUG (improvise-umi4) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_pivot_edit_stores_the_identical_value() {
for value in values_display_rounds() {
no_op_commit(false, value);
}
}
/// BUG (improvise-umi4) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_records_edit_stores_the_identical_value() {
for value in values_display_rounds() {
no_op_commit(true, value);
}
}
/// Stored values whose edit text `typed()` reads back as something else.
fn values_typed_misreads() -> Vec<crate::model::cell::CellValue> {
use crate::model::cell::CellValue::{Error, Text};
vec![
Text("007".into()),
Text("1e3".into()),
Text("inf".into()),
Text("2024".into()),
Error("boom".into()),
]
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_pivot_edit_of_a_misread_value_writes_nothing() {
for value in values_typed_misreads() {
no_op_commit(false, value);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_records_edit_of_a_misread_value_writes_nothing() {
for value in values_typed_misreads() {
no_op_commit(true, value);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_edit_does_not_overwrite_a_write_made_while_it_was_open() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Text("007".into()), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
app.apply_effects(vec![Box::new(effect::SetCell(
target.clone(),
CellValue::Number(5.0),
))]);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Number(5.0)),
"an editor nobody typed in overwrote the write that landed while it was open"
);
}
/// BUG (improvise-2054) — see `context/test-evidence/command.md`.
#[test]
fn an_editor_typed_back_to_its_seed_still_writes() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Number(7.0), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
app.apply_effects(vec![Box::new(effect::SetCell(
target.clone(),
CellValue::Number(5.0),
))]);
press(&mut app, KeyCode::Backspace);
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"Backspace alone did not mark the editor touched"
);
press(&mut app, KeyCode::Char('7'));
assert_eq!(
app.view_state.buffers.get("edit").map(String::as_str),
Some("7")
);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Number(7.0)),
"a value the user typed was dropped because it spelled the seed"
);
}
/// Either user edit marks the open editor, mirroring the browser's
/// `only_an_edit_of_the_editors_buffer_marks_it_touched`.
#[test]
fn appending_or_popping_marks_the_editor_touched() {
use crate::model::cell::CellValue;
use improvise_protocol::key::KeyCode;
for key in [KeyCode::Char('x'), KeyCode::Backspace] {
let mut app = grid_holding(CellValue::Number(7.0), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(false)
);
press(&mut app, key);
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"{key:?} did not mark the editor touched"
);
}
}
/// BUG (improvise-wv3h) — see `context/test-evidence/command.md`.
#[test]
fn an_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let empty = CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Cost".into()),
]);
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Number(2.0), false);
app.model_state.workbook.model.clear_cell(&empty);
app.rebuild_layout();
app.cursor_mut().selected = (0, 0);
press(&mut app, KeyCode::Char('i'));
assert_eq!(
app.view_state
.opened_editor
.as_ref()
.map(|o| o.seed.as_str()),
Some(""),
"the first editor did not open on the empty cell"
);
press(&mut app, KeyCode::Esc);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char(':'));
for c in "enter-mode editing".chars() {
press(&mut app, KeyCode::Char(c));
}
press(&mut app, KeyCode::Enter);
assert!(
app.view_state.mode.is_editing(),
"`:enter-mode editing` did not open an editor"
);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
None,
"the empty commit was skipped against the earlier editor's seed"
);
}
/// improvise-465t: the editor an advance reopens is a new open, so it starts
/// untouched even when the commit before it was typed. See command.md.
#[test]
fn the_editor_an_advance_reopens_starts_untouched() {
use improvise_protocol::key::KeyCode;
let mut app = grid_holding(CellValue::Number(4.0), false);
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Char('9'));
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"the first editor was never touched, so this proves nothing"
);
press(&mut app, KeyCode::Enter);
assert_eq!(app.cursor().selected, (0, 1), "the advance did not run");
assert_eq!(
app.view_state.opened_editor,
Some(OpenedEditor::on("2".into())),
"the reopened editor inherited the previous editor's touched flag"
);
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_commit_still_advances_and_reopens_the_editor() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Cost".into()),
]);
let mut app = grid_holding(CellValue::Number(4.0), false);
app.model_state
.workbook
.model
.set_cell(target.clone(), CellValue::Text("007".into()));
app.rebuild_layout();
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Text("007".into())),
"the untouched commit wrote"
);
assert_eq!(app.cursor().selected, (0, 1), "the advance did not run");
assert!(
app.view_state.mode.is_editing(),
"the editor was not reopened"
);
assert_eq!(
app.view_state
.opened_editor
.as_ref()
.map(|o| o.seed.as_str()),
Some("2"),
"the reopened editor did not capture the cell it landed on"
);
}
/// Every cell of the 2×2 fixture, by its key.
fn every_cell(app: &crate::ui::app::App) -> Vec<Option<CellValue>> {
use crate::model::cell::CellKey;
let mut cells = Vec::new();
for region in ["North", "West"] {
for measure in ["Cost", "Revenue"] {
let key = CellKey::new(vec![
("Region".into(), region.into()),
("_Measure".into(), measure.into()),
]);
cells.push(app.model_state.workbook.model.get_cell(&key).cloned());
}
}
cells
}
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
#[test]
fn enter_enter_leaves_every_cell_unchanged() {
use improvise_protocol::key::KeyCode;
for (records, start) in [(false, (1, 0)), (true, (1, 2))] {
let mut app = grid_holding(CellValue::Number(4.0), records);
if records {
app.view_state.mode = AppMode::RecordsNormal;
}
app.cursor_mut().selected = start;
let before = every_cell(&app);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
let reopened = app.view_state.buffers.get("edit").cloned();
press(&mut app, KeyCode::Enter);
assert_eq!(
every_cell(&app),
before,
"records={records}: i, Enter, Enter changed a cell (the reopened editor held {reopened:?})"
);
}
}
/// The reopened editor shows the cell it landed on, so typing extends it.
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
#[test]
fn typing_after_the_reopen_still_writes() {
use crate::model::cell::CellKey;
use improvise_protocol::key::KeyCode;
let mut app = grid_holding(CellValue::Number(4.0), false);
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
assert_eq!(
app.view_state.buffers.get("edit").map(String::as_str),
Some("2"),
"the editor reopened on (North, Revenue) is not seeded with its value"
);
press(&mut app, KeyCode::Char('5'));
press(&mut app, KeyCode::Enter);
let landed = CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Revenue".into()),
]);
assert_eq!(
app.model_state.workbook.model.get_cell(&landed),
Some(&CellValue::Number(25.0))
);
}
}
// ── Commit commands (mode-specific buffer consumers) ────────────────────────
/// Commit a cell value: a synthetic records key stages into the drill or applies
/// directly in plain records mode; a real key writes to the model (gotcha 19).
fn commit_regular_cell_value(key: &CellKey, value: &str, effects: &mut Vec<Box<dyn Effect>>) {
// The classification is the protocol's, not a second copy of it: the
// browser composes `SetCell` from the same rule (improvise-j7v6).
effects.push(match improvise_protocol::typed(value) {
Some(value) => Box::new(effect::SetCell(key.clone(), value)) as Box<dyn Effect>,
None => Box::new(effect::ClearCell(key.clone())),
});
}
/// Stage a synthetic edit in drill state so it can be applied atomically on exit.
fn stage_drill_edit(record_idx: usize, col: RecordCol, value: &str) -> Box<dyn Effect> {
Box::new(effect::SetDrillPendingEdit {
record_idx,
col,
new_value: value.to_string(),
})
}
/// Apply a records-mode edit directly to the model (no drill state). A drill's staged
/// edits go through the same `RecordRewrite`, all of a row's at once (gotcha 19).
fn commit_plain_records_edit(
ctx: &CmdContext,
record_idx: usize,
col: &RecordCol,
value: &str,
effects: &mut Vec<Box<dyn Effect>>,
) {
let Some(row) = ctx.layout.record_row(record_idx) else {
return;
};
match col {
// The one door the wire's `RenameRecordCoordinate` also takes (improvise-fdk1).
RecordCol::Category(category) => effects.push(Box::new(effect::RenameRecordCoordinate {
record: row.coords.clone(),
category: category.clone(),
item: value.to_string(),
})),
RecordCol::Measure(_) => {
effects.extend(effect::RecordRewrite::of(ctx.model, row, [(col, value)]).effects());
}
RecordCol::NewMeasure => add_measure(value, effects),
RecordCol::NewCategory => add_category(value, effects),
}
}
/// The name a creating column's buffer holds, or `None` when it names nothing and so
/// creates no column. The one rule; `inserts_a_column` reads the same answer.
fn created_name(value: &str) -> Option<&str> {
Some(value.trim()).filter(|name| !name.is_empty())
}
/// Typing a name into the trailing column creates that measure. A schema change, so
/// it applies immediately even inside a drill.
fn add_measure(value: &str, effects: &mut Vec<Box<dyn Effect>>) {
if let Some(name) = created_name(value) {
effects.push(Box::new(effect::AddItem {
category: MEASURE_CATEGORY.to_string(),
item: name.to_string(),
}));
}
}
/// Typing a name into `+cat` creates the category **and its column**, and writes no
/// coordinate on the current row (improvise-2zq) — see `context/code/command.md`.
fn add_category(value: &str, effects: &mut Vec<Box<dyn Effect>>) {
if let Some(name) = created_name(value) {
effects.push(Box::new(effect::AddCategory(name.to_string())));
}
}
fn commit_cell_value(
ctx: &CmdContext,
key: &CellKey,
value: &str,
effects: &mut Vec<Box<dyn Effect>>,
) {
if let Some((record_idx, col_idx)) = crate::view::synthetic_record_info(key) {
let Some(col) = ctx.layout.record_col(col_idx).cloned() else {
return;
};
match (&col, ctx.has_drill_state) {
// A new measure or category is a schema change, never a staged
// cell edit: there is no cell to roll back with the drill.
(RecordCol::NewMeasure, _) => add_measure(value, effects),
(RecordCol::NewCategory, _) => add_category(value, effects),
(_, true) => effects.push(stage_drill_edit(record_idx, col, value)),
(_, false) => commit_plain_records_edit(ctx, record_idx, &col, value, effects),
}
return;
}
commit_regular_cell_value(key, value, effects);
}
/// Whether committing `value` at `key` inserts a column, invalidating every index to
/// its right. Asked of the *layout* — one answer to "what is this column" (gotcha 1).
fn inserts_a_column(ctx: &CmdContext, key: &CellKey, value: &str) -> bool {
created_name(value).is_some()
&& crate::view::synthetic_record_info(key)
.and_then(|(_, col_idx)| ctx.layout.record_col(col_idx))
.is_some_and(|col| matches!(col, RecordCol::NewCategory | RecordCol::NewMeasure))
}
/// Direction to advance after committing a cell edit.
#[derive(Debug, Clone, Copy)]
pub enum AdvanceDir {
/// Move down (typewriter-style, wraps to next column at bottom).
Down,
/// Move right (clamps at rightmost column).
Right,
}
/// The normal-mode counterpart of an editing mode, for the mode to land in if
/// `CommitAndAdvance`'s advance aborts.
fn exit_mode_for(edit_mode: &AppMode) -> AppMode {
match edit_mode {
AppMode::RecordsEditing { .. } => AppMode::RecordsNormal,
_ => AppMode::Normal,
}
}
/// Commit a cell edit, advance the cursor, and re-enter `edit_mode` — which the
/// keymap binding supplies. The command never inspects `ctx.mode`.
#[derive(Debug)]
pub struct CommitAndAdvance {
pub key: CellKey,
pub value: String,
pub advance: AdvanceDir,
pub cursor: CursorState,
pub edit_mode: AppMode,
/// What the editor captured as it opened; `None` for a script line, which
/// always writes (improvise-ixxx).
pub opened: Option<OpenedEditor>,
}
impl Cmd for CommitAndAdvance {
fn name(&self) -> &'static str {
match self.advance {
AdvanceDir::Down => "commit-cell-edit",
AdvanceDir::Right => "commit-and-advance-right",
}
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
// Drop to normal **before** the commit, which can abort: tidy-up goes where
// an abort cannot reach it (improvise-2zq, gotcha 21).
effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
// This command owns its buffer's clear, at the head, so the re-entry
// below re-seeds after it rather than being blanked (improvise-fy9o).
if let Some(mb) = self.edit_mode.minibuffer() {
effects.push(write_buffer(&mb.buffer_key, String::new()));
}
// An untouched editor writes nothing; everything around the write still runs.
let writes = !untouched(self.opened.as_ref(), &self.value);
if writes {
commit_cell_value(ctx, &self.key, &self.value, &mut effects);
}
// The commit may reshape the grid and so reset the cursor. Restore it before
// the advance, where an aborting advance cannot skip it (gotcha 21).
effects.push(Box::new(effect::SetSelected(
self.cursor.row,
self.cursor.col,
)));
if writes && inserts_a_column(ctx, &self.key, &self.value) {
// Staying put is what lands on the inserted column (improvise-2zq); the
// restore above is what makes "staying put" something we do, not inherit.
effects.push(Box::new(effect::RebuildLayout));
return effects;
}
match self.advance {
AdvanceDir::Down => {
let adv = EnterAdvance {
cursor: self.cursor.clone(),
};
effects.extend(adv.execute(ctx));
}
AdvanceDir::Right => {
let col_max = self.cursor.col_count.saturating_sub(1);
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,
));
}
}
}
// The commit and the advance above both changed the grid; refresh the
// layout so the re-edit reads the cell we actually landed on.
effects.push(Box::new(effect::RebuildLayout));
effects.push(Box::new(effect::EnterEditAtCursor {
target_mode: self.edit_mode.clone(),
}));
effects
}
}
/// What a minibuffer commit produces. The variants differ only in the mutation they
/// build and where they leave the user; the rest is [`CommitBuffer`]'s (gotcha 20).
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CommitKind {
Formula,
/// Edit the formula the editor was opened on. Never parsed: `execute` reads
/// it off the mode's slot, the capture made at open (improvise-km71).
ReplaceFormula {
id: FormulaId,
was: String,
},
Category,
/// Add an item to this category, resolved from [`AppMode::ItemAdd`] at
/// construction so `execute` stays pure over the kind and its buffer.
Item {
category: String,
},
}
/// Resolve a `commit-buffer` argument against the current mode. Only `Item` needs
/// the mode, so a script line can still build the other two.
pub fn parse_commit_kind(arg: &str, mode: &AppMode) -> Result<CommitKind, String> {
match (arg, mode) {
("formula", _) => Ok(CommitKind::Formula),
("category", _) => Ok(CommitKind::Category),
("item", AppMode::ItemAdd { category, .. }) => Ok(CommitKind::Item {
category: category.clone(),
}),
("item", _) => Err("commit-buffer item requires item-add mode".into()),
(other, _) => Err(format!("Unknown commit kind: {other}")),
}
}
impl CommitKind {
/// A formula commit inside an editor opened on an existing formula edits
/// that one. Read in `execute`, never at parse — `context/code/command.md`.
fn in_mode(&self, mode: &AppMode) -> CommitKind {
match (self, mode) {
(
Self::Formula,
AppMode::FormulaEdit {
slot: FormulaSlot::Existing { id, was },
..
},
) => Self::ReplaceFormula {
id: id.clone(),
was: was.clone(),
},
(kind, _) => kind.clone(),
}
}
/// The minibuffer this kind consumes.
pub fn buffer(&self) -> &'static str {
match self {
Self::Formula | Self::ReplaceFormula { .. } => "formula",
Self::Category => "category",
Self::Item { .. } => "item",
}
}
/// Where an empty buffer lands the user: back in the panel that opened the
/// minibuffer.
fn on_empty(&self) -> AppMode {
match self {
Self::Formula | Self::ReplaceFormula { .. } => AppMode::FormulaPanel,
Self::Category | Self::Item { .. } => AppMode::CategoryPanel,
}
}
/// Where a *successful* commit lands the user. `None` keeps the minibuffer open:
/// categories and items are entered in batches, formulas are not.
fn on_commit(&self) -> Option<AppMode> {
match self {
Self::Formula | Self::ReplaceFormula { .. } => Some(AppMode::FormulaPanel),
Self::Category | Self::Item { .. } => None,
}
}
fn status(&self, name: &str) -> String {
match self {
Self::Formula => "Formula added".to_string(),
Self::ReplaceFormula { .. } => "Formula updated".to_string(),
Self::Category => format!("Added category \"{name}\""),
Self::Item { .. } => format!("Added \"{name}\""),
}
}
fn add_effect(&self, name: &str) -> Box<dyn Effect> {
match self {
// Formulas default to _Measure, the virtual measure category, which
// dynamically includes every formula target.
Self::Formula => Box::new(effect::AddFormula {
raw: name.to_string(),
target_category: "_Measure".to_string(),
}),
Self::ReplaceFormula { id, was } => Box::new(effect::ReplaceFormula {
id: id.clone(),
was: was.clone(),
raw: name.to_string(),
}),
Self::Category => Box::new(effect::AddCategory(name.to_string())),
Self::Item { category } => Box::new(effect::AddItem {
category: category.clone(),
item: name.to_string(),
}),
}
}
}
/// Commit a minibuffer as a formula, a category or an item. Buffer clearing is the
/// keymap's (Enter → `[commit, clear-buffer]`).
#[derive(Debug)]
pub struct CommitBuffer {
pub kind: CommitKind,
}
impl Cmd for CommitBuffer {
fn name(&self) -> &'static str {
"commit-buffer"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let kind = self.kind.in_mode(ctx.mode);
let raw = read_buffer(ctx, kind.buffer());
let name = raw.trim();
if name.is_empty() {
return vec![effect::change_mode(kind.on_empty())];
}
let mut effects: Vec<Box<dyn Effect>> = vec![
effect::set_status(kind.status(name)),
// The add goes last: it is the only party that knows whether it worked,
// so the optimistic status must not clobber its error (improvise-6t3).
kind.add_effect(name),
];
effects.extend(kind.on_commit().map(effect::change_mode));
effects
}
}
/// Commit a save path. The sibling of [`CommitExport`] and deliberately not the same
/// command — that one writes CSV, this writes the model (improvise-lae).
#[derive(Debug)]
pub struct CommitSave;
impl Cmd for CommitSave {
fn name(&self) -> &'static str {
"commit-save"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let path = read_buffer(ctx, "save").trim().to_string();
if path.is_empty() {
return vec![
effect::refuse("No path given — not saved"),
effect::change_mode(AppMode::Normal),
];
}
// Clear before `SaveAs`, whose failure aborts; this command owns the clear
// because its LocalThenRequest cannot live in a sequence (improvise-2mqz).
vec![
Box::new(effect::SetBuffer {
name: "save".into(),
value: String::new(),
}),
effect::change_mode(AppMode::Normal),
Box::new(effect::SaveAs(std::path::PathBuf::from(path))),
]
}
}
/// Commit an export from the export buffer.
#[derive(Debug)]
pub struct CommitExport;
impl Cmd for CommitExport {
fn name(&self) -> &'static str {
"commit-export"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let path = read_buffer(ctx, "export").trim().to_string();
if path.is_empty() {
return vec![
effect::refuse("No path given — not exported"),
effect::change_mode(AppMode::Normal),
];
}
// Clear before export, whose failure aborts; this command owns the clear
// because its LocalThenRequest cannot live in a sequence (improvise-2mqz).
vec![
Box::new(effect::SetBuffer {
name: "export".into(),
value: String::new(),
}),
effect::change_mode(AppMode::Normal),
Box::new(effect::ExportCsv(std::path::PathBuf::from(path))),
]
}
}