feat(cmd): use new effects to improve command behavior

Update various commands to utilize the new AbortChain and CleanEmptyRecords
effects.

- CommitAndAdvance now pushes a mode change effect when aborting.
- ToggleRecordsMode now cleans up empty records upon exiting.
- EnterAdvance now emits AbortChain when at the bottom-right corner.

Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
This commit is contained in:
Edward Langley
2026-04-15 23:42:44 -07:00
parent 489e2805e8
commit a900f147b5
3 changed files with 59 additions and 5 deletions

View File

@ -261,6 +261,16 @@ pub enum AdvanceDir {
Right,
}
/// Return the normal-mode counterpart of an editing mode. Used by
/// `CommitAndAdvance` to compute the mode to land in if the advance
/// aborts (commit + exit editing at boundary).
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.
/// Subsumes the old `CommitCellEdit` (Down) and `CommitAndAdvanceRight` (Right).
///
@ -286,6 +296,13 @@ 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(ctx, &self.key, &self.value, &mut effects);
// Pre-emptively drop to the normal counterpart of edit_mode. If the
// advance succeeds, the trailing `EnterEditAtCursor` below will lift
// us back into editing on the new cell. If the advance aborts
// (e.g. already at bottom-right on Enter), `EnterEditAtCursor` is
// skipped and we land in normal mode — which is the desired
// "Enter at bottom-right commits and exits" behavior.
effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
match self.advance {
AdvanceDir::Down => {
let adv = EnterAdvance {

View File

@ -132,7 +132,10 @@ mod tests {
let mut m = Workbook::new("Test");
m.add_category("Region").unwrap();
m.model.category_mut("Region").unwrap().add_item("East");
m.model.category_mut("_Measure").unwrap().add_item("Revenue");
m.model
.category_mut("_Measure")
.unwrap()
.add_item("Revenue");
m.model.category_mut("_Measure").unwrap().add_item("Cost");
m.model.set_cell(
CellKey::new(vec![
@ -148,7 +151,8 @@ mod tests {
]),
CellValue::Number(600.0),
);
m.model.add_formula(parse_formula("Profit = Revenue - Cost", "_Measure").unwrap());
m.model
.add_formula(parse_formula("Profit = Revenue - Cost", "_Measure").unwrap());
let layout = make_layout(&m);
let reg = make_registry();
@ -408,8 +412,15 @@ impl Cmd for ToggleRecordsMode {
let is_records = ctx.layout.is_records_mode();
if is_records {
// Navigate back to the previous view (restores original axes)
return vec![Box::new(effect::ViewBack), effect::set_status("Pivot mode")];
// Leaving records mode: clean up any records with empty CellKeys
// (produced by AddRecordRow when no page filters are set) before
// restoring the previous view. This is the inverse of `SortData`
// that runs on entry.
return vec![
Box::new(effect::CleanEmptyRecords),
Box::new(effect::ViewBack),
effect::set_status("Pivot mode"),
];
}
let mut effects: Vec<Box<dyn Effect>> = Vec::new();

View File

@ -154,7 +154,10 @@ impl Cmd for EnterAdvance {
} else if c < col_max {
(0, c + 1)
} else {
(r, c) // already at bottom-right; stay
// Already at bottom-right — the advance premise no longer holds.
// Abort the rest of the batch so the caller's trailing effects
// (e.g. `CommitAndAdvance`'s `EnterEditAtCursor`) are skipped.
return vec![Box::new(effect::AbortChain)];
};
viewport_effects(
nr,
@ -331,6 +334,29 @@ mod tests {
);
}
/// At bottom-right `EnterAdvance` has no place to go, so it emits a
/// single `AbortChain` effect. Trailing effects in a `CommitAndAdvance`
/// batch (e.g. `EnterEditAtCursor`) are then skipped, which is how
/// "Enter at bottom-right commits and exits editing" is realised.
#[test]
fn enter_advance_at_bottom_right_emits_abort_chain() {
let m = two_cat_model();
let layout = make_layout(&m);
let reg = make_registry();
let ctx = make_ctx(&m, &layout, &reg);
let mut cursor = CursorState::from_ctx(&ctx);
cursor.row = cursor.row_count.saturating_sub(1);
cursor.col = cursor.col_count.saturating_sub(1);
let cmd = EnterAdvance { cursor };
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 1, "should emit exactly AbortChain");
let dbg = format!("{:?}", effects[0]);
assert!(
dbg.contains("AbortChain"),
"Expected AbortChain, got: {dbg}"
);
}
#[test]
fn law_move_to_start_idempotent() {
let m = two_cat_model();