diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 8a64910..2bdfe79 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -30,14 +30,14 @@ impl CellRef { } /// Moved by `(dr, dc)`, `None` when the result leaves the sheet. pub fn offset(&self, dr: i64, dc: i64) -> Option { - let r = self.row as i64 + dr; - let c = self.col as i64 + dc; + let r = (self.row as i64).checked_add(dr)?; + let c = (self.col as i64).checked_add(dc)?; if (0..MAX_ROWS as i64).contains(&r) && (0..MAX_COLS as i64).contains(&c) { Some(CellRef::new(r as u32, c as u32)) } else { None } } /// Moved by `(dr, dc)` and clamped to the sheet. pub fn offset_clamped(&self, dr: i64, dc: i64) -> CellRef { - let r = (self.row as i64 + dr).clamp(0, MAX_ROWS as i64 - 1); - let c = (self.col as i64 + dc).clamp(0, MAX_COLS as i64 - 1); + let r = (self.row as i64).saturating_add(dr).clamp(0, MAX_ROWS as i64 - 1); + let c = (self.col as i64).saturating_add(dc).clamp(0, MAX_COLS as i64 - 1); CellRef::new(r as u32, c as u32) } /// Parses `A1`, `$A$1`, `xfd1048576` (case-insensitive). `None` if malformed or out of range. @@ -272,6 +272,21 @@ mod tests { assert_eq!(a.iter().count(), 9); } + #[test] + fn extreme_offsets() { + let cell = CellRef::new(1, 1); + for delta in [i64::MIN, i64::MAX] { + assert_eq!(cell.offset(delta, 0), None); + assert_eq!(cell.offset(0, delta), None); + assert_eq!(cell.offset(delta, delta), None); + } + assert_eq!(cell.offset_clamped(i64::MAX, i64::MAX), CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MIN), CellRef::new(0, 0)); + assert_eq!(cell.offset_clamped(i64::MAX, i64::MIN), CellRef::new(MAX_ROWS - 1, 0)); + assert_eq!(cell.offset_clamped(i64::MIN, i64::MAX), CellRef::new(0, MAX_COLS - 1)); + assert_eq!(cell.offset(2, -1), Some(CellRef::new(3, 0))); + } + #[test] fn offsets() { assert_eq!(CellRef::new(0, 0).offset(-1, 0), None); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bb54fe0..2dba66f 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1,6 +1,6 @@ //! Cell entry, selection, undo/redo, clipboard, clear, fill, find & replace. -use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, Value}; +use gridcraft_core::{CellRef, MAX_ROWS, RangeRef, Value}; use gridcraft_model::{Cell, Formula, StyleId}; use serde_json::{Value as Json, json}; @@ -408,9 +408,9 @@ fn selection_move(s: &mut Session, p: &Json) -> Result { if !extend && let Some(m) = sheet.merge_at(from) { // Leaving a merged cell moves from its edge. if dr > 0 { - t = CellRef::new((m.end.row + dr as u32).min(MAX_ROWS - 1), from.col); + t = CellRef::new(m.end.offset_clamped(dr, 0).row, from.col); } else if dc > 0 { - t = CellRef::new(from.row, (m.end.col + dc as u32).min(MAX_COLS - 1)); + t = CellRef::new(from.row, m.end.offset_clamped(0, dc).col); } } t diff --git a/crates/engine/tests/extreme_selection_moves.rs b/crates/engine/tests/extreme_selection_moves.rs new file mode 100644 index 0000000..f6f5737 --- /dev/null +++ b/crates/engine/tests/extreme_selection_moves.rs @@ -0,0 +1,40 @@ +use gridcraft_core::{CellRef, MAX_COLS, MAX_ROWS}; +use gridcraft_engine::Session; +use serde_json::json; + +#[test] +fn selection_move_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (i64::MAX, i64::MAX, CellRef::new(MAX_ROWS - 1, MAX_COLS - 1)), + (i64::MIN, i64::MIN, CellRef::new(0, 0)), + (i64::MAX, i64::MIN, CellRef::new(MAX_ROWS - 1, 0)), + (i64::MIN, i64::MAX, CellRef::new(0, MAX_COLS - 1)), + (2, -1, CellRef::new(3, 0)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +} + +#[test] +fn selection_move_from_a_merged_cell_clamps_extreme_displacements() { + let mut session = Session::new(); + session.new_workbook(); + session.execute("selection.set", json!({"range": "B2:C3"})).unwrap(); + session.execute("home.mergeCells", json!({})).unwrap(); + for (dr, dc, expected) in [ + (i64::MAX, 0, CellRef::new(MAX_ROWS - 1, 1)), + (0, i64::MAX, CellRef::new(1, MAX_COLS - 1)), + (4_294_967_297, 0, CellRef::new(MAX_ROWS - 1, 1)), + (1, 0, CellRef::new(3, 1)), + ] { + session.execute("selection.set", json!({"cell": "B2"})).unwrap(); + session.execute("selection.move", json!({"dr": dr, "dc": dc})).unwrap(); + assert_eq!(session.doc().unwrap().selection.active, expected); + } +}