diff --git a/src/command/cmd/commit.rs b/src/command/cmd/commit.rs index 69ad10a..82a88db 100644 --- a/src/command/cmd/commit.rs +++ b/src/command/cmd/commit.rs @@ -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> { let mut effects: Vec> = 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 { diff --git a/src/command/cmd/grid.rs b/src/command/cmd/grid.rs index d0e2b8e..4a67530 100644 --- a/src/command/cmd/grid.rs +++ b/src/command/cmd/grid.rs @@ -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> = Vec::new(); diff --git a/src/command/cmd/navigation.rs b/src/command/cmd/navigation.rs index d0187f7..a522c9f 100644 --- a/src/command/cmd/navigation.rs +++ b/src/command/cmd/navigation.rs @@ -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, ®); + 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();