shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #232 from webdevtodayjason/fix/selection-move-overflow 

selection.move: clamp extreme dr/dc to the sheet edge instead of overflowing
This commit is contained in:
Brandon Thomas 2026-10-11 11:23:28 -04:00
parent 9f896c1249 fae90c8e4a
commit f7c40e3d7e
3 changed files with 62 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
crates/core/src/addr.rs
View File

@ -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<CellRef> {
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);

6
crates/engine/src/cmd/edit.rs
View File

@ -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<Json> {
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

40
crates/engine/tests/extreme_selection_moves.rs Normal file
View File

@ -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);
}
}