Merge PR #885: Row, column and freeze commands act on the sheet they are given

This commit is contained in:
aiongg
2026-10-22 02:34:36 +00:00
4 changed files with 118 additions and 40 deletions
+47 -28
View File
@@ -156,7 +156,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Row Height…",
["Home", "Cells", "Format"],
None,
"{rows?: \"2:5\" | \"2:2,4:5\", height: 20 (points on screen)}",
"{sheet?, rows?: \"2:5\" | \"2:2,4:5\" | \"Sheet2!2:5\", height: 20 (points on screen)}",
has_doc,
row_height
),
@@ -165,28 +165,34 @@ pub fn specs() -> Vec<CommandSpec> {
"Column Width…",
["Home", "Cells", "Format"],
None,
"{cols?: \"B:D\" | \"B:B,D:F\", width: 64 (points) | chars: 8.43}",
"{sheet?, cols?: \"B:D\" | \"B:B,D:F\" | \"Sheet2!B:D\", width: 64 (points) | chars: 8.43}",
has_doc,
col_width
),
cmd!("home.autofitRowHeight", "AutoFit Row Height", ["Home", "Cells", "Format"], None, "{rows?}", has_doc, autofit_rows),
cmd!("home.autofitColumnWidth", "AutoFit Column Width", ["Home", "Cells", "Format"], None, "{cols?}", has_doc, autofit_cols),
cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{width: 64}", has_doc, default_width),
cmd!("home.hideRows", "Hide Rows", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+9"), "{rows?}", has_doc, |s, p| hide(
cmd!("home.autofitRowHeight", "AutoFit Row Height", ["Home", "Cells", "Format"], None, "{sheet?, rows?}", has_doc, autofit_rows),
cmd!("home.autofitColumnWidth", "AutoFit Column Width", ["Home", "Cells", "Format"], None, "{sheet?, cols?}", has_doc, autofit_cols),
cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{sheet?, width: 64}", has_doc, default_width),
cmd!("home.hideRows", "Hide Rows", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+9"), "{sheet?, rows?}", has_doc, |s, p| hide(
s, p, true, true
)),
cmd!("home.hideColumns", "Hide Columns", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+0"), "{cols?}", has_doc, |s, p| hide(
s, p, false, true
)),
cmd!("home.unhideRows", "Unhide Rows", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+Shift+9"), "{rows?}", has_doc, |s, p| hide(
s, p, true, false
)),
cmd!("home.hideColumns", "Hide Columns", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+0"), "{sheet?, cols?}", has_doc, |s, p| {
hide(s, p, false, true)
}),
cmd!(
"home.unhideRows",
"Unhide Rows",
["Home", "Cells", "Format", "Hide & Unhide"],
Some("Cmd+Shift+9"),
"{sheet?, rows?}",
has_doc,
|s, p| hide(s, p, true, false)
),
cmd!(
"home.unhideColumns",
"Unhide Columns",
["Home", "Cells", "Format", "Hide & Unhide"],
Some("Cmd+Shift+0"),
"{cols?}",
"{sheet?, cols?}",
has_doc,
|s, p| hide(s, p, false, false)
),
@@ -933,19 +939,37 @@ fn cell_style(s: &mut Session, p: &Json) -> Result<Json> {
// ---------------------------------------------------------------- sizes
fn line_targets(s: &Session, p: &Json, rows: bool) -> Result<Vec<(u32, u32)>> {
/// The sheet and the row (or column) spans a size or visibility command works on. The lines are
/// `rows`/`cols` (`"2:5"`, `"B:B,D:F"`, optionally `Sheet!`-prefixed), else the selection's; the
/// sheet is `sheet`, else the prefix, else the active one. The selection belongs to the active
/// sheet, so another sheet needs explicit lines.
fn line_targets(s: &Session, p: &Json, rows: bool) -> Result<(usize, Vec<(u32, u32)>)> {
let key = if rows { "rows" } else { "cols" };
let mut sheet = target_sheet(s, p)?;
if let Some(t) = str_param(p, key) {
let (prefix, body) = split_sheet(t);
if let Some(name) = prefix {
let named = s.doc()?.wb.sheet_index(&name).ok_or_else(|| bad(key, format!("no sheet named `{name}`")))?;
if p.get("sheet").is_some_and(|v| !v.is_null()) && named != sheet {
return Err(bad(key, format!("`{t}` names a sheet other than `sheet`")));
}
sheet = named;
}
// A comma-separated list ("B:B,D:F") targets several areas, like a multi-area selection.
return t
let lines = body
.split(',')
.map(|part| {
let r = RangeRef::parse(part).ok_or_else(|| bad(key, format!("not a range: {part}")))?;
Ok(if rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) })
})
.collect();
.collect::<Result<Vec<_>>>()?;
return Ok((sheet, lines));
}
Ok(s.doc()?.selection.ranges.iter().map(|r| if rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }).collect())
let d = s.doc()?;
if sheet != d.wb.active_sheet {
return Err(bad(key, format!("missing `{key}`: the selection is on the active sheet")));
}
Ok((sheet, d.selection.ranges.iter().map(|r| if rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }).collect()))
}
fn row_height(s: &mut Session, p: &Json) -> Result<Json> {
@@ -953,9 +977,8 @@ fn row_height(s: &mut Session, p: &Json) -> Result<Json> {
if !(0.0..=546.0).contains(&h) {
return Err(EngineError::Other("Row height must be between 0 and 409 points.".into()));
}
let lines = line_targets(s, p, true)?;
let (si, lines) = line_targets(s, p, true)?;
edit(s, |cx| {
let si = cx.sheet_index();
let sh = cx.sheet_mut(si)?;
for (a, b) in lines {
for r in a..=b.min(a.saturating_add(1_048_575)) {
@@ -979,9 +1002,8 @@ fn col_width(s: &mut Session, p: &Json) -> Result<Json> {
if !(0.0..=1800.0).contains(&w) {
return Err(EngineError::Other("Column width must be between 0 and 255 characters.".into()));
}
let lines = line_targets(s, p, false)?;
let (si, lines) = line_targets(s, p, false)?;
edit(s, |cx| {
let si = cx.sheet_index();
let sh = cx.sheet_mut(si)?;
for (a, b) in lines {
for c in a..=b {
@@ -1026,9 +1048,8 @@ pub fn approx_text_width(text: &str, size: f32, bold: bool) -> f32 {
}
fn autofit_cols(s: &mut Session, p: &Json) -> Result<Json> {
let lines = line_targets(s, p, false)?;
let (si, lines) = line_targets(s, p, false)?;
edit(s, |cx| {
let si = cx.sheet_index();
let Some(sh) = cx.wb.sheet(si) else { return Ok(Json::Null) };
let mut widths = Vec::new();
let used = sh.used_range();
@@ -1065,9 +1086,8 @@ fn autofit_cols(s: &mut Session, p: &Json) -> Result<Json> {
}
fn autofit_rows(s: &mut Session, p: &Json) -> Result<Json> {
let lines = line_targets(s, p, true)?;
let (si, lines) = line_targets(s, p, true)?;
edit(s, |cx| {
let si = cx.sheet_index();
for (a, b) in lines {
let b = b.min(a + 100_000);
if let Some(sh) = cx.wb.sheet_mut(si) {
@@ -1087,17 +1107,16 @@ fn autofit_rows(s: &mut Session, p: &Json) -> Result<Json> {
fn default_width(s: &mut Session, p: &Json) -> Result<Json> {
let w = f64_param(p, "width").ok_or_else(|| bad("home.defaultWidth", "missing `width`"))?;
let si = target_sheet(s, p)?;
edit(s, |cx| {
let si = cx.sheet_index();
cx.sheet_mut(si)?.default_col_width = w.clamp(0.0, 1800.0) as f32;
Ok(Json::Null)
})
}
fn hide(s: &mut Session, p: &Json, rows: bool, on: bool) -> Result<Json> {
let lines = line_targets(s, p, rows)?;
let (si, lines) = line_targets(s, p, rows)?;
edit(s, |cx| {
let si = cx.sheet_index();
let sh = cx.sheet_mut(si)?;
for (a, b) in lines {
// Unhiding a single selected line also reveals hidden neighbours (Excel needs the
+24 -10
View File
@@ -13,16 +13,25 @@ pub fn specs() -> Vec<CommandSpec> {
"Freeze Panes",
["View", "Window", "Freeze Panes"],
None,
"{cell?: \"B2\" (rows above and columns left of it freeze)}",
"{sheet?, cell?: \"B2\" (rows above and columns left of it freeze); without `cell`, unfreezes a frozen sheet}",
has_doc,
freeze_panes
),
cmd!("view.freezeTopRow", "Freeze Top Row", ["View", "Window", "Freeze Panes"], None, "{}", has_doc, |s, _| set_freeze(s, Some((1, 0)))),
cmd!("view.freezeFirstColumn", "Freeze First Column", ["View", "Window", "Freeze Panes"], None, "{}", has_doc, |s, _| set_freeze(
cmd!("view.freezeTopRow", "Freeze Top Row", ["View", "Window", "Freeze Panes"], None, "{sheet?}", has_doc, |s, p| set_freeze(
s,
target_sheet(s, p)?,
Some((1, 0))
)),
cmd!("view.freezeFirstColumn", "Freeze First Column", ["View", "Window", "Freeze Panes"], None, "{sheet?}", has_doc, |s, p| set_freeze(
s,
target_sheet(s, p)?,
Some((0, 1))
)),
cmd!("view.unfreezePanes", "Unfreeze Panes", ["View", "Window", "Freeze Panes"], None, "{}", has_doc, |s, _| set_freeze(s, None)),
cmd!("view.unfreezePanes", "Unfreeze Panes", ["View", "Window", "Freeze Panes"], None, "{sheet?}", has_doc, |s, p| set_freeze(
s,
target_sheet(s, p)?,
None
)),
cmd!("view.gridlines", "Gridlines", ["View", "Show"], None, "{on?}", has_doc, |s, p| sheet_flag(s, p, "gridlines")),
cmd!("view.headings", "Headings", ["View", "Show"], None, "{on?}", has_doc, |s, p| sheet_flag(s, p, "headings")),
cmd!("view.showZeros", "Show Zeros", [], None, "{on?}", has_doc, |s, p| sheet_flag(s, p, "zeros")),
@@ -113,23 +122,28 @@ pub fn specs() -> Vec<CommandSpec> {
}
fn freeze_panes(s: &mut Session, p: &Json) -> Result<Json> {
let si = target_sheet(s, p)?;
let d = s.doc()?;
let sh = d.wb.active().ok_or(EngineError::NoDocument)?;
let sh = d.wb.sheet(si).ok_or(EngineError::NoDocument)?;
if sh.freeze.is_some() && p.get("cell").is_none() {
return set_freeze(s, None);
return set_freeze(s, si, None);
}
let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active);
let at = match cell_param_on(s, p, "cell", si)? {
Some(c) => c,
None if si == d.wb.active_sheet => d.selection.active,
// The selection belongs to the active sheet.
None => return Err(bad("view.freezePanes", "missing `cell`: the selection is on the active sheet")),
};
let top = sh.view_top_left;
let rows = at.row.saturating_sub(top.row);
let cols = at.col.saturating_sub(top.col);
let _ = (rows, cols);
let (rows, cols) = (at.row, at.col);
set_freeze(s, if rows == 0 && cols == 0 { None } else { Some((rows, cols)) })
set_freeze(s, si, if rows == 0 && cols == 0 { None } else { Some((rows, cols)) })
}
fn set_freeze(s: &mut Session, f: Option<(u32, u32)>) -> Result<Json> {
fn set_freeze(s: &mut Session, si: usize, f: Option<(u32, u32)>) -> Result<Json> {
edit(s, |cx| {
let si = cx.sheet_index();
cx.sheet_mut(si)?.freeze = f;
Ok(json!({"freeze": f}))
})
+43
View File
@@ -3098,6 +3098,49 @@ fn row_height_and_column_width_take_a_list_of_areas() {
assert_eq!(s.doc().unwrap().wb.active().unwrap().col_width(1), 120.0, "a bad list changes nothing");
}
/// Programmatic callers lay out a sheet that isn't the active one: `sheet` (or a `Sheet!` prefix on
/// `rows`, `cols` or `cell`) picks it, instead of being ignored in favour of the active sheet.
#[test]
fn line_sizes_and_freeze_panes_take_a_sheet() {
let mut s = s();
s.execute("home.insertSheet", json!({"name": "Data"})).unwrap();
s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap();
let sheet = |s: &Session, i: usize| s.doc().unwrap().wb.sheets[i].clone();
let before = sheet(&s, 0);
s.execute("view.freezePanes", json!({"sheet": "Data", "cell": "A2"})).unwrap();
s.execute("home.columnWidth", json!({"sheet": "Data", "cols": "A:A", "width": 150})).unwrap();
s.execute("home.columnWidth", json!({"cols": "Data!B:B,D:D", "chars": 20})).unwrap();
// A null `sheet` is no `sheet`, as everywhere else: the prefix decides.
s.execute("home.columnWidth", json!({"sheet": null, "cols": "Data!C:C", "chars": 20})).unwrap();
s.execute("home.rowHeight", json!({"sheet": 1, "rows": "1:1", "height": 40})).unwrap();
s.execute("home.hideColumns", json!({"sheet": "Data", "cols": "E:E"})).unwrap();
s.execute("home.defaultWidth", json!({"sheet": "Data", "width": 80})).unwrap();
let data = sheet(&s, 1);
assert_eq!(data.freeze, Some((1, 0)));
assert_eq!((data.col_width(0), data.col_width(1), data.col_width(2), data.col_width(3)), (150.0, 145.0, 145.0, 145.0));
assert_eq!(data.row_height(0), 40.0);
assert!(data.cols.get(&4).is_some_and(|c| c.hidden));
assert_eq!(data.default_col_width, 80.0);
assert_eq!(sheet(&s, 0), before, "the active sheet is untouched");
assert_eq!(s.doc().unwrap().wb.active_sheet, 0);
s.execute("view.freezePanes", json!({"cell": "Data!B3"})).unwrap();
assert_eq!(sheet(&s, 1).freeze, Some((2, 1)));
s.execute("view.unfreezePanes", json!({"sheet": "Data"})).unwrap();
s.execute("view.freezeTopRow", json!({"sheet": "Data"})).unwrap();
assert_eq!(sheet(&s, 1).freeze, Some((1, 0)));
// The selection belongs to the active sheet: another sheet needs explicit lines or a cell, and
// a prefix can't contradict `sheet`.
assert!(s.execute("home.columnWidth", json!({"sheet": "Data", "width": 10})).is_err());
s.execute("view.unfreezePanes", json!({"sheet": "Data"})).unwrap();
assert!(s.execute("view.freezePanes", json!({"sheet": "Data"})).is_err());
assert!(s.execute("home.columnWidth", json!({"sheet": "Sheet1", "cols": "Data!B:B", "width": 10})).is_err());
assert!(s.execute("home.rowHeight", json!({"rows": "Nope!1:1", "height": 10})).is_err());
assert_eq!(sheet(&s, 0), before);
}
#[test]
fn copying_areas_that_share_rows_or_columns_pastes_them_as_one_block() {
// Issue #348: Excel slides lined-up areas together; formulas travel as their values.
+4 -2
View File
@@ -466,7 +466,6 @@ fn other_sheet_prefix_is_error() {
ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": 0}}));
fails(&mut s, "set_cell", json!({"cell": "Nope!B2", "input": "x"}));
// A command that cannot honour a sheet prefix must not drop it and use the active sheet.
cmd_fails(&mut s, "view.freezePanes", json!({"cell": "Other!B2"}));
cmd_fails(&mut s, "formulas.tracePrecedents", json!({"cell": "Other!B2"}));
ok(&mut s, "set_cell", json!({"cell": "Sheet1!B2", "input": "here"}));
assert_eq!(ok(&mut s, "get_cell", json!({"cell": "B2"}))["text"], "here");
@@ -498,7 +497,10 @@ fn cross_sheet_cell_refs_resolve() {
// The active sheet may be a quoted name with spaces.
ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": "My Sheet"}}));
ok(&mut s, "execute_command", json!({"command": "view.freezePanes", "params": {"cell": "'My Sheet'!B2"}}));
cmd_fails(&mut s, "view.freezePanes", json!({"cell": "Other!B2"}));
// Freeze Panes honours another sheet's prefix (or `sheet`) and leaves the active sheet alone.
ok(&mut s, "execute_command", json!({"command": "view.freezePanes", "params": {"cell": "Other!C4"}}));
let sheets = ok(&mut s, "inspect_workbook", json!({}))["sheets"].clone();
assert_eq!((sheets[1]["freeze"].clone(), sheets[2]["freeze"].clone()), (json!([3, 2]), json!([1, 1])), "{sheets}");
// A prefix naming a different sheet than the `sheet` argument is an error, not resolved in favour of either.
cmd_fails(&mut s, "cell.get", json!({"cell": "Other!B3", "sheet": "Sheet1"}));
}