fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Merge branch 'main' into worktree-improvise-ewi-formula-crate

Browse Source 
# Conflicts:
#	TAGS
This commit is contained in:
Edward Langley
2026-04-15 22:47:51 -07:00
parent 9efbed403a 03c7c00b25
commit 79dc54de21
9 changed files with 9970 additions and 433 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
src/command/cmd/commit.rs
View File

@ -118,6 +118,35 @@ mod tests {
assert!(effects.is_empty());
}
/// `CommitAndAdvance` must thread its `edit_mode` through to the
/// trailing `EnterEditAtCursor` effect so the post-commit re-edit lands
/// in the mode the keymap requested. 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(),
};
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();
@ -234,12 +263,18 @@ pub enum AdvanceDir {
/// Commit a cell edit, advance the cursor, and re-enter edit mode.
/// Subsumes the old `CommitCellEdit` (Down) and `CommitAndAdvanceRight` (Right).
///
/// `edit_mode` is the editing mode to re-enter after advancing. The keymap
/// binding supplies this — the editing-mode keymap passes `editing` and the
/// records-editing keymap passes `records-editing`. The command itself
/// 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,
}
impl Cmd for CommitAndAdvance {
fn name(&self) -> &'static str {
@ -287,7 +322,9 @@ impl Cmd for CommitAndAdvance {
}
}
}
effects.push(Box::new(effect::EnterEditAtCursor));
effects.push(Box::new(effect::EnterEditAtCursor {
target_mode: self.edit_mode.clone(),
}));
effects
}
}

149
src/command/cmd/mode.rs
View File

@ -10,21 +10,6 @@ mod tests {
use crate::command::cmd::test_helpers::*;
use crate::workbook::Workbook;
#[test]
fn enter_edit_mode_produces_editing_mode() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let ctx = make_ctx(&m, &layout, &reg);
let cmd = EnterEditMode {
initial_value: String::new(),
};
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 2);
let dbg = format!("{:?}", effects[1]);
assert!(dbg.contains("Editing"), "Expected Editing mode, got: {dbg}");
}
#[test]
fn enter_tile_select_with_categories() {
let m = two_cat_model();
@ -98,11 +83,80 @@ mod tests {
let layout = make_layout(&m);
let reg = make_registry();
let ctx = make_ctx(&m, &layout, &reg);
let effects = EditOrDrill.execute(&ctx);
let effects = EditOrDrill {
edit_mode: AppMode::editing(),
}
.execute(&ctx);
assert_eq!(effects.len(), 2);
let dbg = effects_debug(&effects);
assert!(dbg.contains("Editing"), "Expected Editing mode, got: {dbg}");
}
/// EditOrDrill must trust its `edit_mode` parameter rather than checking
/// `ctx.mode` — the records-normal keymap supplies `records-editing`,
/// but the command itself never inspects the runtime mode. This is the
/// parallel of the (deleted) `enter_edit_mode_produces_editing_mode`
/// test for the records branch.
#[test]
fn edit_or_drill_passes_records_editing_mode_through() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
// Note: ctx.mode is still Normal here — the command must not look at it.
let ctx = make_ctx(&m, &layout, &reg);
let effects = EditOrDrill {
edit_mode: AppMode::records_editing(),
}
.execute(&ctx);
assert_eq!(effects.len(), 2);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("RecordsEditing"),
"Expected RecordsEditing mode, got: {dbg}"
);
}
/// `EnterEditAtCursorCmd` must hand its `target_mode` straight through
/// to the `EnterEditAtCursor` effect — the keymap (records `o` sequence
/// or commit-and-advance) decides; the command never inspects ctx.
#[test]
fn enter_edit_at_cursor_cmd_passes_target_mode_to_effect() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let ctx = make_ctx(&m, &layout, &reg);
let effects = EnterEditAtCursorCmd {
target_mode: AppMode::records_editing(),
}
.execute(&ctx);
assert_eq!(effects.len(), 1);
let dbg = format!("{:?}", effects[0]);
assert!(
dbg.contains("RecordsEditing"),
"Expected RecordsEditing target_mode, got: {dbg}"
);
}
/// The edit branch pre-fills the `edit` buffer with the cell's current
/// display value so the user can modify rather than retype.
#[test]
fn edit_or_drill_pre_fills_edit_buffer_with_display_value() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let mut ctx = make_ctx(&m, &layout, &reg);
ctx.display_value = "42".to_string();
let effects = EditOrDrill {
edit_mode: AppMode::editing(),
}
.execute(&ctx);
let dbg = effects_debug(&effects);
assert!(
dbg.contains("SetBuffer") && dbg.contains("\"edit\"") && dbg.contains("\"42\""),
"Expected SetBuffer(\"edit\", \"42\"), got: {dbg}"
);
}
#[test]
fn enter_search_mode_sets_flag_and_clears_query() {
let m = two_cat_model();
@ -188,36 +242,18 @@ impl Cmd for SaveAndQuit {
// ── Editing entry ───────────────────────────────────────────────────────
/// Enter editing mode with an initial buffer value.
#[derive(Debug)]
pub struct EnterEditMode {
pub initial_value: String,
}
impl Cmd for EnterEditMode {
fn name(&self) -> &'static str {
"enter-edit-mode"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
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(edit_mode),
]
}
}
/// Smart dispatch for i/a: if the cursor is on an aggregated pivot cell
/// (categories on `Axis::None`, no records mode), drill into it instead of
/// editing. Otherwise enter edit mode with the current displayed value.
/// (categories on `Axis::None` and the cell is not a synthetic records-mode
/// row), drill into it instead of editing. Otherwise pre-fill the edit
/// buffer with the displayed cell value and enter `edit_mode`.
///
/// `edit_mode` is supplied by the keymap binding — the command itself is
/// mode-agnostic, so the records-normal keymap passes `records-editing`
/// while the normal keymap passes `editing`.
#[derive(Debug)]
pub struct EditOrDrill;
pub struct EditOrDrill {
pub edit_mode: AppMode,
}
impl Cmd for EditOrDrill {
fn name(&self) -> &'static str {
"edit-or-drill"
@ -232,7 +268,8 @@ impl Cmd for EditOrDrill {
.map(|cat| cat.kind.is_regular())
.unwrap_or(false)
});
// In records mode (synthetic key), always edit directly — no drilling.
// Synthetic records-mode cells are never aggregated — edit directly.
// (This is a layout property, not a mode flag.)
let is_synthetic = ctx.synthetic_record_at_cursor().is_some();
let is_aggregated = !is_synthetic && regular_none;
if is_aggregated {
@ -241,23 +278,31 @@ impl Cmd for EditOrDrill {
};
return DrillIntoCell { key }.execute(ctx);
}
EnterEditMode {
initial_value: ctx.display_value.clone(),
}
.execute(ctx)
vec![
Box::new(effect::SetBuffer {
name: "edit".to_string(),
value: ctx.display_value.clone(),
}),
effect::change_mode(self.edit_mode.clone()),
]
}
}
/// Thin command wrapper around the `EnterEditAtCursor` effect so it can
/// participate in `Binding::Sequence`.
/// participate in `Binding::Sequence`. `target_mode` is supplied as the
/// command argument by the keymap binding.
#[derive(Debug)]
pub struct EnterEditAtCursorCmd;
pub struct EnterEditAtCursorCmd {
pub target_mode: AppMode,
}
impl Cmd for EnterEditAtCursorCmd {
fn name(&self) -> &'static str {
"enter-edit-at-cursor"
}
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::EnterEditAtCursor)]
vec![Box::new(effect::EnterEditAtCursor {
target_mode: self.target_mode.clone(),
})]
}
}

90
src/command/cmd/registry.rs
View File

@ -2,6 +2,27 @@ use crate::model::cell::CellKey;
use crate::ui::app::AppMode;
use crate::ui::effect::Panel;
/// Decode a mode-name string (as supplied by `enter-mode`/`edit-or-drill`
/// keymap bindings) into an `AppMode`.
fn parse_mode_name(s: &str) -> Result<AppMode, String> {
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}")),
}
}
use super::cell::*;
use super::commit::*;
use super::core::*;
@ -266,22 +287,16 @@ pub fn default_registry() -> CmdRegistry {
r.register_nullary(|| Box::new(SaveAndQuit));
r.register_nullary(|| Box::new(SaveCmd));
r.register_nullary(|| Box::new(EnterSearchMode));
r.register(
&EnterEditMode {
initial_value: String::new(),
},
|args| {
let val = args.first().cloned().unwrap_or_default();
Ok(Box::new(EnterEditMode { initial_value: val }))
},
|_args, ctx| {
Ok(Box::new(EnterEditMode {
initial_value: ctx.display_value.clone(),
}))
},
);
r.register_nullary(|| Box::new(EditOrDrill));
r.register_nullary(|| Box::new(EnterEditAtCursorCmd));
r.register_pure(&NamedCmd("edit-or-drill"), |args| {
require_args("edit-or-drill", args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
Ok(Box::new(EditOrDrill { edit_mode }))
});
r.register_pure(&NamedCmd("enter-edit-at-cursor"), |args| {
require_args("enter-edit-at-cursor", args, 1)?;
let target_mode = parse_mode_name(&args[0])?;
Ok(Box::new(EnterEditAtCursorCmd { target_mode }))
});
r.register_nullary(|| Box::new(EnterExportPrompt));
r.register_nullary(|| Box::new(EnterFormulaEdit));
r.register_nullary(|| Box::new(EnterTileSelect));
@ -310,23 +325,7 @@ pub fn default_registry() -> CmdRegistry {
);
r.register_pure(&NamedCmd("enter-mode"), |args| {
require_args("enter-mode", args, 1)?;
let mode = match args[0].as_str() {
"normal" => AppMode::Normal,
"help" => AppMode::Help,
"formula-panel" => AppMode::FormulaPanel,
"category-panel" => AppMode::CategoryPanel,
"view-panel" => AppMode::ViewPanel,
"tile-select" => AppMode::TileSelect,
"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}")),
};
Ok(Box::new(EnterMode(mode)))
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
});
// ── Search ───────────────────────────────────────────────────────────
@ -522,25 +521,33 @@ pub fn default_registry() -> CmdRegistry {
r.register_nullary(|| Box::new(CommandModeBackspace));
// ── Commit ───────────────────────────────────────────────────────────
// commit-cell-edit / commit-and-advance-right take a mode-name arg
// (e.g. "editing" or "records-editing") as args[0]. The keymap supplies
// it; the command never inspects ctx.mode.
r.register(
&CommitAndAdvance {
key: CellKey::new(vec![]),
value: String::new(),
advance: AdvanceDir::Down,
cursor: CursorState::default(),
edit_mode: AppMode::editing(),
},
|args| {
if args.len() < 2 {
return Err("commit-cell-edit requires a value and coords".into());
if args.len() < 3 {
return Err("commit-cell-edit requires a mode, value, and coords".into());
}
let edit_mode = parse_mode_name(&args[0])?;
Ok(Box::new(CommitAndAdvance {
key: parse_cell_key_from_args(&args[1..]),
value: args[0].clone(),
key: parse_cell_key_from_args(&args[2..]),
value: args[1].clone(),
advance: AdvanceDir::Down,
cursor: CursorState::default(),
edit_mode,
}))
},
|_args, ctx| {
|args, ctx| {
require_args("commit-cell-edit", args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitAndAdvance {
@ -548,6 +555,7 @@ pub fn default_registry() -> CmdRegistry {
value,
advance: AdvanceDir::Down,
cursor: CursorState::from_ctx(ctx),
edit_mode,
}))
},
);
@ -557,9 +565,12 @@ pub fn default_registry() -> CmdRegistry {
value: String::new(),
advance: AdvanceDir::Right,
cursor: CursorState::default(),
edit_mode: AppMode::editing(),
},
|_| Err("commit-and-advance-right requires context".into()),
|_args, ctx| {
|args, ctx| {
require_args("commit-and-advance-right", args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
let value = read_buffer(ctx, "edit");
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
Ok(Box::new(CommitAndAdvance {
@ -567,6 +578,7 @@ pub fn default_registry() -> CmdRegistry {
value,
advance: AdvanceDir::Right,
cursor: CursorState::from_ctx(ctx),
edit_mode,
}))
},
);

60
src/command/keymap.rs
View File

@ -431,9 +431,21 @@ impl KeymapSet {
);
normal.bind(KeyCode::Tab, none, "cycle-panel-focus");
// Editing entry — i/a drill into aggregated cells, else edit
normal.bind(KeyCode::Char('i'), none, "edit-or-drill");
normal.bind(KeyCode::Char('a'), none, "edit-or-drill");
// 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(KeyCode::Char('e'), ctrl, "enter-export-prompt");
@ -488,10 +500,27 @@ impl KeymapSet {
// ── 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![])],
vec![
("add-record-row", vec![]),
("enter-edit-at-cursor", vec!["records-editing".into()]),
],
);
set.insert(ModeKey::RecordsNormal, Arc::new(rn));
@ -736,6 +765,8 @@ impl KeymapSet {
set.insert(ModeKey::TileSelect, Arc::new(ts));
// ── Editing mode ─────────────────────────────────────────────────
// commit-* takes the target edit-mode arg so the command stays
// mode-agnostic; records-editing overrides Enter/Tab below.
let mut ed = Keymap::new();
ed.bind_seq(
KeyCode::Esc,
@ -749,7 +780,7 @@ impl KeymapSet {
KeyCode::Enter,
none,
vec![
("commit-cell-edit", vec![]),
("commit-cell-edit", vec!["editing".into()]),
("clear-buffer", vec!["edit".into()]),
],
);
@ -757,7 +788,7 @@ impl KeymapSet {
KeyCode::Tab,
none,
vec![
("commit-and-advance-right", vec![]),
("commit-and-advance-right", vec!["editing".into()]),
("clear-buffer", vec!["edit".into()]),
],
);
@ -767,6 +798,7 @@ impl KeymapSet {
set.insert(ModeKey::Editing, ed.clone());
// ── Records editing mode (inherits from editing) ──────────────────
// Override Enter/Tab so the post-commit re-enter targets records-editing.
let mut re = Keymap::with_parent(ed);
re.bind_seq(
KeyCode::Esc,
@ -776,6 +808,22 @@ impl KeymapSet {
("enter-mode", vec!["records-normal".into()]),
],
);
re.bind_seq(
KeyCode::Enter,
none,
vec![
("commit-cell-edit", vec!["records-editing".into()]),
("clear-buffer", vec!["edit".into()]),
],
);
re.bind_seq(
KeyCode::Tab,
none,
vec![
("commit-and-advance-right", vec!["records-editing".into()]),
("clear-buffer", vec!["edit".into()]),
],
);
set.insert(ModeKey::RecordsEditing, Arc::new(re));
// ── Formula edit ─────────────────────────────────────────────────

52
src/ui/effect.rs
View File

@ -124,10 +124,19 @@ impl Effect for RemoveFormula {
/// Re-enter edit mode by reading the cell value at the current cursor.
/// Used after commit+advance to continue data entry.
///
/// `target_mode` is supplied by the caller (keymap binding via
/// `EnterEditAtCursorCmd`, or `CommitAndAdvance` from its own `edit_mode`
/// field). The effect itself never inspects `app.mode` — the mode is decided
/// statically by whoever invoked us.
#[derive(Debug)]
pub struct EnterEditAtCursor;
pub struct EnterEditAtCursor {
pub target_mode: AppMode,
}
impl Effect for EnterEditAtCursor {
fn apply(&self, app: &mut App) {
// Layout may be stale relative to prior effects in this batch (e.g.
// AddRecordRow added a row); rebuild before reading display_value.
app.rebuild_layout();
let ctx = app.cmd_context(
crossterm::event::KeyCode::Null,
@ -136,11 +145,7 @@ impl Effect for EnterEditAtCursor {
let value = ctx.display_value.clone();
drop(ctx);
app.buffers.insert("edit".to_string(), value);
app.mode = if app.mode.is_records() {
AppMode::records_editing()
} else {
AppMode::editing()
};
app.mode = self.target_mode.clone();
}
}
@ -1288,6 +1293,41 @@ mod tests {
assert_eq!(app.mode, AppMode::Help);
}
/// `EnterEditAtCursor` must use its `target_mode` field, *not* whatever
/// `app.mode` happens to be when applied. Previous implementation
/// branched on `app.mode.is_records()` — the parameterized version
/// trusts the caller (keymap or composing command).
#[test]
fn enter_edit_at_cursor_uses_target_mode_not_app_mode() {
let mut app = test_app();
// App starts in Normal mode — but caller has decided we want
// RecordsEditing (e.g. records-mode `o` sequence).
assert_eq!(app.mode, AppMode::Normal);
EnterEditAtCursor {
target_mode: AppMode::records_editing(),
}
.apply(&mut app);
assert!(
matches!(app.mode, AppMode::RecordsEditing { .. }),
"Expected RecordsEditing, got {:?}",
app.mode
);
// Same effect with editing target — should land in plain Editing
// even if app.mode was something else.
let mut app2 = test_app();
app2.mode = AppMode::RecordsNormal;
EnterEditAtCursor {
target_mode: AppMode::editing(),
}
.apply(&mut app2);
assert!(
matches!(app2.mode, AppMode::Editing { .. }),
"Expected Editing, got {:?}",
app2.mode
);
}
/// SetBuffer with empty value clears the buffer (used by clear-buffer command
/// in keymap sequences after commit).
#[test]