diff --git a/ROADMAP.md b/ROADMAP.md index 172a920d2..01f9362d8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** minor (XLSX dynamic-array formulas keep their `cm` metadata on save) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (undo steps share picture and macro bytes; floating pictures' textures bounded and kept per workbook) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,8 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-16 | Memory: undo steps share a sheet's picture bytes and the workbook's VBA project instead of copying them: 50 edits on a sheet with a 20 MB picture keep 22 MB instead of 992 MB | +| 2026-10-16 | Memory: floating pictures are decoded at about the size they are drawn, with limits, into a cache bounded by bytes and dropped with their workbook, instead of at full size into one that never let go; two open workbooks no longer show each other's picture. 12 large pictures scrolled through keep 40 MB of textures instead of 258 MB, and none once the workbook closes | | 2026-10-15 | Function help follows the interface language: argument hints, autocomplete, Insert Function (search and readable categories) and the Formulas-tab menus read per-language tables of descriptions and argument names; Spanish keeps its 10 functions in the new format | | 2026-10-14 | Protect Sheet on a protected sheet: Home › Format, its keytips (HOP, RPS) and the tab menu now read Unprotect Sheet… and unprotect (straight away without a password); a protected sheet that forbids selecting locked (or unlocked) cells is enforced for clicks, arrows and Go To, and the Protect Sheet dialog has the select options (#1346) | | 2026-10-14 | XLSX: formulas opened with dynamic-array metadata (`cm`) keep it on save, with `xl/metadata.xml`. Before, a formula returning one value lost it (11 of 40 corpus files that have the part lost it entirely), and Excel recalculated some of them with implicit intersection (`SUMPRODUCT(IFERROR(...))` changed value) | @@ -140,6 +142,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | 2026-10-13 | Colour menus: Fill Color, Font Color and the Format Cells pickers end with Recent Colors (the last ten custom colours) and More Colors… (hue/saturation area, RGB, hex box, preview, OK/Cancel) (#928) | | 2026-10-13 | Multi-level category axes: a category range two or more columns wide beside a column of values (year beside month, Excel's `multiLvlStrRef`) draws its innermost labels across the plot and the outer levels in rows under them with group separators (columns beside the labels in bar charts), instead of flattening the two columns into twice as many categories; saving writes the reference back as `multiLvlStrRef` with Excel's level caches, so the axis survives a round trip | | 2026-10-13 | Charts: a huge trendline forecast no longer collapses the chart: on a category axis it adds at most four times the data in empty slots (at least 32), the line is sampled over the plot only, and forecasts read from XLSX or set by command are clamped, NaN and infinity becoming 0 (#1148) +| 2026-10-13 | XLSX export writes a picture used on several sheets once, as one `xl/media` part every drawing points to (it wrote a copy per drawing: a 16-sheet workbook with one 112 KB logo grew from 0.4 to 1.9 MB) (#985) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | @@ -201,9 +204,12 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Progress log: undo steps share picture and macro bytes | +| 2026-10-16 | minor | Progress log: floating pictures' textures bounded and kept per workbook | | 2026-10-14 | minor | XLSX dynamic-array `cm` metadata kept on save; progress log row | | 2026-10-13 | trivial | Progress log: More Colors… and Recent Colors in the colour menus (#928) | | 2026-10-13 | minor | Multi-level category axes (read, drawn, written); progress log row | +| 2026-10-13 | trivial | Progress log: shared pictures written once on XLSX export (#985) | | 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) | | 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 523aa7e0f..812765249 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -1373,7 +1373,16 @@ impl<'h> Evaluator<'h> { })) } "width" => Ev::V(Value::number(sheet.map_or(8.43, |s| gridcraft_model::points_to_chars(s.col_width(c.col) as f64)).round())), - "filename" => Ev::V(Value::text(String::new())), + // Excel's `/[]`, which the classic sheet-name formula splits on + // the `]`. Empty until the workbook has a file of its own, as in Excel (#1489). + "filename" => Ev::V(Value::text(match &wb.source_path { + Some(file) => { + let name = file.file_name().unwrap_or_default().to_string_lossy(); + let on = sheet.map_or("", |s| s.name.as_str()); + file.parent().unwrap_or(std::path::Path::new("")).join(format!("[{name}]{on}")).to_string_lossy().into_owned() + } + None => String::new(), + })), "sheetname" => Ev::V(Value::text(sheet.map(|s| s.name.clone()).unwrap_or_default())), "protect" => Ev::V(Value::number(if sheet.is_some_and(|s| wb.styles.get(s.style_id(c)).protection.locked) { 1.0 } else { 0.0 })), "format" => { diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 89e5761a8..6dd57e334 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -1312,3 +1312,40 @@ fn median_large_values_recalculate_dependents() { assert!((t.num(cell) + 1.3).abs() < 1e-14); } } + +#[test] +fn cell_filename_names_the_workbook_file() { + // Excel returns `/[]`, and the classic sheet-name formula splits it on + // the `]`; an empty string broke that formula in every German workbook that used it (#1489). + let dir = std::path::Path::new("books"); + let mut t = T::new(); + t.wb.sheet_mut(0).unwrap().name = "Info".into(); + + // No file yet: empty, as in a workbook Excel has never saved. + t.set("A1", "1"); + t.set("B1", "=CELL(\"filename\",A1)"); + assert_eq!(t.get("B1"), Value::from("")); + + t.wb.source_path = Some(dir.join("gc_cell_info_de.xlsx")); + t.calc = Calc::new(); + let expect = dir.join("[gc_cell_info_de.xlsx]Info").to_string_lossy().into_owned(); + + t.set("B2", "=CELL(\"filename\",A1)"); + assert_eq!(t.get("B2"), Value::from(expect.as_str())); + + // The two formulas from the report: the sheet name, and the test that the `]` is there. + t.set("B3", "=MID(CELL(\"filename\",A1),FIND(\"]\",CELL(\"filename\",A1))+1,31)"); + assert_eq!(t.get("B3"), Value::from("Info"), "the sheet-name formula"); + t.set("B4", "=ISNUMBER(FIND(\"]\",CELL(\"filename\",A1)))"); + assert_eq!(t.get("B4"), Value::Bool(true)); + // The sheet is the one the reference is on, not the active one. + t.wb.sheets.push(std::sync::Arc::new(gridcraft_model::Sheet::new("Second"))); + t.set("B6", "=CELL(\"filename\",Second!A1)"); + assert_eq!(t.get("B6"), Value::from(dir.join("[gc_cell_info_de.xlsx]Second").to_string_lossy().into_owned().as_str())); + + // A workbook that arrived as bytes knows its name but no folder: still splittable. + t.wb.source_path = Some(std::path::PathBuf::from("Book.xlsx")); + t.calc = Calc::new(); + t.set("B7", "=CELL(\"filename\",A1)"); + assert_eq!(t.get("B7"), Value::from("[Book.xlsx]Info")); +} diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 24bd043f7..6043ed2f8 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -80,13 +80,13 @@ pub fn specs() -> Vec { "Group", ["Data", "Outline"], Some("Alt+Shift+Right"), - "{rows?: \"2:5\" | cols?: \"B:C\" | by?: rows|cols (the selection's rows or columns; a block of cells groups rows by default)}", + "{rows?: \"2:5\" | cols?: \"B:C\" | by?: rows|cols (the selection's rows or columns; a block of cells groups rows by default), sheet?}", has_doc, |s, p| group(s, p, true) ), - cmd!("data.ungroup", "Ungroup", ["Data", "Outline"], Some("Alt+Shift+Left"), "{rows? | cols? | by?: rows|cols}", has_doc, |s, p| group( - s, p, false - )), + cmd!("data.ungroup", "Ungroup", ["Data", "Outline"], Some("Alt+Shift+Left"), "{rows? | cols? | by?: rows|cols, sheet?}", has_doc, |s, p| { + group(s, p, false) + }), cmd!( "data.hideDetail", "Hide Detail", @@ -147,6 +147,23 @@ fn data_range(s: &Session, p: &Json) -> Result { Ok(current_region(sh, d.selection.active)) } +/// The sheet and range a data command works on: the sheet `sheet` or a `Sheet!` prefix on `range` +/// names, else the active one. The selection belongs to the active sheet, so another sheet needs `range`. +fn data_target(s: &Session, p: &Json, cmd: &str) -> Result<(usize, RangeRef)> { + let sheet = target_sheet(s, p)?; + let d = s.doc()?; + if let Some(t) = str_param(p, "range") + && let (Some(name), _) = split_sheet(t) + && d.wb.sheet_index(&name) != Some(sheet) + { + return Err(bad(cmd, format!("`{t}` names a sheet other than `sheet`"))); + } + if sheet != d.wb.active_sheet && p.get("range").is_none() { + return Err(bad(cmd, "missing `range`: the selection is on the active sheet")); + } + Ok((sheet, data_range(s, p)?)) +} + /// Excel's header guess: the first row is text over a column of numbers, or is bold. fn guess_header(wb: &Workbook, sh: &Sheet, r: RangeRef) -> bool { if let Some(t) = sh.table_at(r.start) { @@ -182,19 +199,36 @@ struct SortKey { list: Option>, } +/// The sheet and range a sort works on: the sheet `sheet` or a `Sheet!` prefix on `range` names, +/// else the active one. The selection belongs to the active sheet, so another sheet needs `range`. +fn sort_target(s: &Session, p: &Json) -> Result<(usize, RangeRef)> { + let sheet = target_sheet(s, p)?; + let d = s.doc()?; + if let Some(t) = str_param(p, "range") + && let (Some(name), _) = split_sheet(t) + && d.wb.sheet_index(&name) != Some(sheet) + { + return Err(bad("data.sort", format!("`{t}` names a sheet other than `sheet`"))); + } + if sheet != d.wb.active_sheet && p.get("range").is_none() { + return Err(bad("data.sort", "missing `range`: the selection is on the active sheet")); + } + Ok((sheet, data_range(s, p)?)) +} + fn quick_sort(s: &mut Session, p: &Json, asc: bool) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = sort_target(s, p)?; let col = col_param(p.get("column"), r.start.col).unwrap_or(s.doc()?.selection.active.col).clamp(r.start.col, r.end.col); let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; let header = guess_header(&d.wb, sh, r); - do_sort(s, r, header, vec![SortKey { col, desc: !asc, by: "values".into(), list: None }], false, false) + do_sort(s, sheet, r, header, vec![SortKey { col, desc: !asc, by: "values".into(), list: None }], false, false) } fn sort(s: &mut Session, p: &Json) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = sort_target(s, p)?; let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; let header = bool_param(p, "header").unwrap_or_else(|| guess_header(&d.wb, sh, r)); let by_cols = str_param(p, "orientation") == Some("columns"); let case = bool_param(p, "matchCase").unwrap_or(false); @@ -228,10 +262,12 @@ fn sort(s: &mut Session, p: &Json) -> Result { if keys.len() > 64 { return Err(bad("data.sort", "too many sort levels (64 max)")); } - do_sort(s, r, header, keys, by_cols, case) + do_sort(s, sheet, r, header, keys, by_cols, case) } -fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_cols: bool, case: bool) -> Result { +fn do_sort(s: &mut Session, sheet: usize, r: RangeRef, header: bool, keys: Vec, by_cols: bool, case: bool) -> Result { + // Protect Sheet ▸ Sort. Both Sort… and the A-to-Z buttons come through here. + super::refuse_unless_allowed(s, sheet, "Sorting", |p| p.sort)?; let body = if by_cols { if header && r.width() > 1 { RangeRef::new(CellRef::new(r.start.row, r.start.col + 1), r.end) } else { r } } else if header && r.height() > 1 { @@ -242,7 +278,6 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co if body.count() > 20_000_000 { return Err(bad("data.sort", "range too large")); } - let sheet = s.doc()?.wb.active_sheet; edit(s, |cx| { let Some(sh) = cx.wb.sheet(sheet) else { return Ok(Json::Null) }; if sh.merges.iter().any(|m| m.intersects(&body) && !m.is_single()) { @@ -437,10 +472,12 @@ fn sorted_anchor(sh: &Sheet, a: Anchor, bottom: (u32, u32, f32, f32), by_cols: b // ---------------------------------------------------------------- filter fn toggle_filter(s: &mut Session, p: &Json) -> Result { - let sheet = s.doc()?.wb.active_sheet; + let (sheet, r) = data_target(s, p, "data.filter")?; + super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?; + // The active cell belongs to the active sheet: on another sheet the table is the one at `range`. + let at = if sheet == s.doc()?.wb.active_sheet { s.doc()?.selection.active } else { r.start }; let has = s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.autofilter.is_some()); - let in_table = - s.doc()?.wb.sheet(sheet).and_then(|sh| sh.table_at(s.doc().map(|d| d.selection.active).unwrap_or_default()).map(|t| t.name.clone())); + let in_table = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.table_at(at).map(|t| t.name.clone())); if let Some(name) = in_table { return edit(s, |cx| { let sh = cx.sheet_mut(sheet)?; @@ -473,8 +510,7 @@ fn toggle_filter(s: &mut Session, p: &Json) -> Result { /// Turn a sheet AutoFilter on over `p.range`, or over the data range of the selection. fn install_filter(s: &mut Session, p: &Json) -> Result { - let sheet = s.doc()?.wb.active_sheet; - let mut r = data_range(s, p)?; + let (sheet, mut r) = data_target(s, p, "data.filter")?; // A selected header row filters the data below it (#201). if p.get("range").is_none() && r.height() == 1 { let d = s.doc()?; @@ -492,6 +528,7 @@ fn install_filter(s: &mut Session, p: &Json) -> Result { fn filter_by(s: &mut Session, p: &Json) -> Result { let sheet = s.doc()?.wb.active_sheet; + super::refuse_unless_allowed(s, sheet, "Filtering", |p| p.autofilter)?; let Some(sh) = s.doc()?.wb.sheet(sheet) else { return Err(EngineError::NoDocument) }; let af = match &sh.autofilter { Some(a) => a.clone(), @@ -583,6 +620,11 @@ fn custom_ok(v: &Value, text: &str, op: &str, crit: &str) -> bool { } } +/// The filter's "(Blanks)" group: truly empty cells and zero-length text such as a `=""` result. +fn filter_blank(v: &Value) -> bool { + v.is_empty() || matches!(v, Value::Text(t) if t.is_empty()) +} + /// Hides rows of the autofilter range that fail any criterion. pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) { let Some(sh) = wb.sheet(sheet) else { return }; @@ -616,7 +658,7 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) { let text = crate::display::cell_text(wb, sh, c); let ok = match crit { FilterCriterion::Values { values, blanks } => { - if v.is_empty() { + if filter_blank(&v) { *blanks } else { values.iter().any(|x| x.eq_ignore_ascii_case(&text) || x.eq_ignore_ascii_case(&v.display())) @@ -703,10 +745,11 @@ pub fn filter_values(wb: &Workbook, sheet: usize, col: u32) -> Vec<(String, bool for row in af.range.start.row + 1..=af.range.end.row.min(af.range.start.row + 1_000_000) { let c = CellRef::new(row, col); let v = sh.value(c); - let t = if v.is_empty() { "(Blanks)".to_string() } else { crate::display::cell_text(wb, sh, c) }; + let blank = filter_blank(&v); + let t = if blank { "(Blanks)".to_string() } else { crate::display::cell_text(wb, sh, c) }; let on = match selected { Some(FilterCriterion::Values { values, blanks }) => { - if v.is_empty() { + if blank { *blanks } else { values.iter().any(|x| x.eq_ignore_ascii_case(&t)) @@ -714,7 +757,8 @@ pub fn filter_values(wb: &Workbook, sheet: usize, col: u32) -> Vec<(String, bool } _ => true, }; - seen.entry((sort_key(&v), t)).or_insert(on); + let key = if blank { sort_key(&Value::Empty) } else { sort_key(&v) }; + seen.entry((key, t)).or_insert(on); } seen.into_iter().map(|((_, t), on)| (t, on)).collect() } @@ -738,9 +782,9 @@ fn sort_key(v: &Value) -> (u8, i64, String) { // ---------------------------------------------------------------- tools fn remove_duplicates(s: &mut Session, p: &Json) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = data_target(s, p, "data.removeDuplicates")?; let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; // Rows and columns past the data are blank: only the part with data is compared (whole // columns compared a million blank rows and reported them as removed duplicates). let r = sh.used_range().and_then(|u| r.intersection(&u)).unwrap_or(RangeRef::cell(r.start)); @@ -750,7 +794,12 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { .and_then(Json::as_array) .map(|a| a.iter().filter_map(|c| col_param(Some(c), r.start.col)).collect()) .unwrap_or_else(|| (r.start.col..=r.end.col).collect()); - let sheet = d.wb.active_sheet; + // With no column to compare, every row key is empty: the first row would survive and every + // other row would be deleted as a duplicate. A column outside the range compares data the + // user didn't select. Excel refuses both. + if cols.is_empty() || cols.iter().any(|c| !(r.start.col..=r.end.col).contains(c)) { + return Err(bad("data.removeDuplicates", "`columns` must name at least one column of the range")); + } let start = r.start.row + header as u32; let mut removed = 0; edit(s, |cx| { @@ -826,7 +875,10 @@ fn text_to_columns(s: &mut Session, p: &Json) -> Result { Some(w) => { let chars: Vec = text.chars().collect(); let mut cuts = vec![0]; - cuts.extend(w.iter().copied().filter(|x| *x < chars.len())); + let mut breaks: Vec = w.iter().copied().filter(|x| *x > 0 && *x < chars.len()).collect(); + breaks.sort_unstable(); + breaks.dedup(); + cuts.extend(breaks); cuts.push(chars.len()); cuts.windows(2) .map(|p| chars.get(p[0]..p[1]).map(|s| s.iter().collect::().trim().to_string()).unwrap_or_default()) @@ -1129,10 +1181,10 @@ fn circle_invalid(s: &mut Session, _: &Json) -> Result { // ---------------------------------------------------------------- outline fn outline_lines(s: &Session, p: &Json) -> Result<(bool, u32, u32)> { - if let Some(r) = str_param(p, "rows").and_then(RangeRef::parse) { + if let Some(r) = str_param(p, "rows").and_then(parse_lines) { return Ok((true, r.start.row, r.end.row)); } - if let Some(r) = str_param(p, "cols").and_then(RangeRef::parse) { + if let Some(r) = str_param(p, "cols").and_then(parse_lines) { return Ok((false, r.start.col, r.end.col)); } let r = s.doc()?.selection.current(); @@ -1145,9 +1197,13 @@ fn outline_lines(s: &Session, p: &Json) -> Result<(bool, u32, u32)> { } fn group(s: &mut Session, p: &Json, on: bool) -> Result { + let si = target_sheet(s, p)?; + // The selection belongs to the active sheet, so another sheet needs its lines. + if si != s.doc()?.wb.active_sheet && p.get("rows").is_none() && p.get("cols").is_none() { + return Err(bad(if on { "data.group" } else { "data.ungroup" }, "missing `rows` or `cols`: the selection is on the active sheet")); + } let (rows, a, b) = outline_lines(s, p)?; edit(s, |cx| { - let si = cx.sheet_index(); let sh = cx.sheet_mut(si)?; let map = if rows { &mut sh.rows } else { &mut sh.cols }; let b = b.min(a.saturating_add(1_048_575)); @@ -1257,7 +1313,8 @@ fn show_levels(s: &mut Session, p: &Json) -> Result { } fn subtotal(s: &mut Session, p: &Json) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = data_target(s, p, "data.subtotal")?; + let header = bool_param(p, "header").unwrap_or(true); let group_col = col_param(p.get("groupBy"), r.start.col).unwrap_or(r.start.col); let func = str_param(p, "function").unwrap_or("sum"); let code = match func { @@ -1281,13 +1338,13 @@ fn subtotal(s: &mut Session, p: &Json) -> Result { "product" => "Product", _ => "Total", }; - let sheet = s.doc()?.wb.active_sheet; // Group boundaries (header in the first row). let d = s.doc()?; let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; let mut groups: Vec<(u32, u32, String)> = Vec::new(); - let mut start = r.start.row + 1; - for row in r.start.row + 1..=r.end.row + 1 { + let first = r.start.row + u32::from(header); + let mut start = first; + for row in first..=r.end.row + 1 { let cur = sh.value(CellRef::new(row, group_col)).display(); let prev = sh.value(CellRef::new(start, group_col)).display(); if row > r.end.row || cur != prev { @@ -1298,25 +1355,28 @@ fn subtotal(s: &mut Session, p: &Json) -> Result { // Insert a total row after each group, bottom-up so indices stay valid. let mut inserted = 0; for (g0, g1, name) in groups.iter().rev() { - s.execute("home.insertRows", json!({"rows": format!("{}:{}", g1 + 2, g1 + 2)}))?; + s.execute("home.insertRows", json!({"sheet": sheet, "rows": format!("{}:{}", g1 + 2, g1 + 2)}))?; let row = g1 + 1; let lbl = CellRef::new(row, group_col).a1(); - s.execute("cell.set", json!({"cell": lbl, "input": format!("{name} {label}")}))?; + s.execute("cell.set", json!({"sheet": sheet, "cell": lbl, "input": format!("{name} {label}")}))?; for c in &cols { let range = RangeRef::new(CellRef::new(*g0, *c), CellRef::new(*g1, *c)).a1(); - s.execute("cell.set", json!({"cell": CellRef::new(row, *c).a1(), "input": format!("=SUBTOTAL({code},{range})")}))?; + s.execute("cell.set", json!({"sheet": sheet, "cell": CellRef::new(row, *c).a1(), "input": format!("=SUBTOTAL({code},{range})")}))?; } - s.execute("home.bold", json!({"range": RangeRef::new(CellRef::new(row, r.start.col), CellRef::new(row, r.end.col)).a1(), "on": true}))?; - s.execute("data.group", json!({"rows": format!("{}:{}", g0 + 1, g1 + 1)}))?; + s.execute( + "home.bold", + json!({"sheet": sheet, "range": RangeRef::new(CellRef::new(row, r.start.col), CellRef::new(row, r.end.col)).a1(), "on": true}), + )?; + s.execute("data.group", json!({"sheet": sheet, "rows": format!("{}:{}", g0 + 1, g1 + 1)}))?; inserted += 1; } let last = r.end.row + inserted + 1; let lbl = CellRef::new(last, group_col).a1(); - s.execute("home.insertRows", json!({"rows": format!("{}:{}", last + 1, last + 1)}))?; - s.execute("cell.set", json!({"cell": lbl, "input": format!("Grand {label}")}))?; + s.execute("home.insertRows", json!({"sheet": sheet, "rows": format!("{}:{}", last + 1, last + 1)}))?; + s.execute("cell.set", json!({"sheet": sheet, "cell": lbl, "input": format!("Grand {label}")}))?; for c in &cols { - let range = RangeRef::new(CellRef::new(r.start.row + 1, *c), CellRef::new(last - 1, *c)).a1(); - s.execute("cell.set", json!({"cell": CellRef::new(last, *c).a1(), "input": format!("=SUBTOTAL({code},{range})")}))?; + let range = RangeRef::new(CellRef::new(first, *c), CellRef::new(last - 1, *c)).a1(); + s.execute("cell.set", json!({"sheet": sheet, "cell": CellRef::new(last, *c).a1(), "input": format!("=SUBTOTAL({code},{range})")}))?; } Ok(json!({"groups": inserted})) } @@ -1325,6 +1385,37 @@ fn subtotal(s: &mut Session, p: &Json) -> Result { mod tests { use super::*; + #[test] + fn filter_blanks_include_empty_text_and_formula_results() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["id", "f"], ["e", null], ["ef", "=\"\""], ["sp", " "], ["z", 0]]})).unwrap(); + s.execute("data.filter", json!({"range": "A1:B5"})).unwrap(); + let r = s.execute("data.filterBy", json!({"column": "B", "values": [], "blanks": true})).unwrap(); + assert_eq!(r["hiddenRows"], json!(2)); + let vals = filter_values(&s.doc().unwrap().wb, 0, 1); + // The `=""` result joins (Blanks) instead of showing as its own empty entry. + assert!(!vals.iter().any(|(t, _)| t.is_empty()), "{vals:?}"); + let blanks: Vec<_> = vals.into_iter().filter(|(t, _)| t == "(Blanks)").collect(); + assert_eq!(blanks, vec![("(Blanks)".to_string(), true)]); + } + + #[test] + fn subtotal_honours_header_false() { + // #131: with header:false the first data row was treated as a header and left out of its + // group's subtotal and the grand total. + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["East", 10], ["East", 20], ["West", 30], ["West", 40]]})).unwrap(); + s.execute("data.subtotal", json!({"range": "A1:B4", "groupBy": "A", "function": "sum", "columns": ["B"], "header": false})).unwrap(); + let get = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap(); + assert_eq!(get(&mut s, "A1")["value"], json!("East")); + assert_eq!(get(&mut s, "A3")["text"], json!("East Total")); + assert_eq!(get(&mut s, "B3")["value"], json!(30.0), "East's subtotal counts both East rows"); + assert_eq!(get(&mut s, "A7")["text"], json!("Grand Total")); + assert_eq!(get(&mut s, "B7")["value"], json!(100.0), "the grand total counts every data row"); + } + #[test] fn filter_by_huge_numeric_column_is_rejected_not_overflowed() { let mut s = Session::new(); @@ -1530,10 +1621,63 @@ mod tests { } } + #[test] + fn single_row_and_column_parameters() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1, 2], [3, 4]]})).unwrap(); + s.execute("home.insertRows", json!({"rows": "2"})).unwrap(); + s.execute("home.insertColumns", json!({"cols": "B"})).unwrap(); + let v = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["value"].clone(); + assert_eq!(v(&mut s, "C3"), json!(4.0)); + s.execute("home.hideRows", json!({"rows": "1"})).unwrap(); + s.execute("data.group", json!({"cols": "C"})).unwrap(); + assert!(s.execute("home.deleteRows", json!({"rows": "0"})).is_err()); + s.execute("home.deleteRows", json!({"rows": "2"})).unwrap(); + s.execute("home.deleteColumns", json!({"cols": "B"})).unwrap(); + assert_eq!(v(&mut s, "B2"), json!(4.0)); + } + + #[test] + fn t2c_fixed_widths_are_positions_in_any_order_without_duplicates() { + let cases: [(Json, [&str; 4]); 5] = [ + (json!([6, 3]), ["abc", "def", "ghij", ""]), + (json!([3, 3]), ["abc", "defghij", "", ""]), + (json!([0]), ["abcdefghij", "", "", ""]), + (json!([20]), ["abcdefghij", "", "", ""]), + (json!([7, 0, 3, 7, 10]), ["abc", "defg", "hij", ""]), + ]; + for (widths, want) in cases { + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "abcdefghij"})).unwrap(); + s.execute("data.textToColumns", json!({"range": "A1:A1", "fixedWidths": widths})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let got: Vec = (0..4).map(|c| sh.value(CellRef::new(0, c)).display()).collect(); + assert_eq!(got, want, "{widths}"); + } + } + #[test] fn split() { assert_eq!(split_delimited("a,b,,c", &[','], false, '"'), ["a", "b", "", "c"]); assert_eq!(split_delimited("a,b,,c", &[','], true, '"'), ["a", "b", "c"]); assert_eq!(split_delimited("\"x,y\",z", &[','], false, '"'), ["x,y", "z"]); } + + #[test] + fn remove_duplicates_without_a_valid_column_is_refused() { + // `columns: []` (or only unusable names) used to compare nothing and delete every row + // but the first. + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a"], ["b"], ["c"]]})).unwrap(); + for columns in [json!([]), json!(["ZZZ"])] { + let err = s.execute("data.removeDuplicates", json!({"range": "A1:A3", "columns": columns})).unwrap_err(); + assert!(err.to_string().contains("columns"), "{err}"); + } + let sh = s.doc().unwrap().wb.active().unwrap(); + assert_eq!(sh.value(CellRef::new(0, 0)), Value::text("a")); + assert_eq!(sh.value(CellRef::new(2, 0)), Value::text("c")); + } } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 9ddc05611..2a925f951 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -58,6 +58,13 @@ fn open(s: &mut Session, p: &Json) -> Result { let encrypted = gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Encrypted; let (mut wb, warnings) = crate::io::open_bytes_with(&name, &bytes, str_param(p, "password"), bool_param(p, "csvAsText").unwrap_or(false))?; wb.source_dir = str_param(p, "path").and_then(crate::io::source_dir); + // What `CELL("filename")` reports (#1489). A workbook opened from bytes knows its name but + // never a folder, so it reports `[name]Sheet` with none; an import reports the file it came + // from, as Excel does, even though Save must still ask where to put the .xlsx. + wb.source_path = match str_param(p, "path") { + Some(path) => crate::io::source_path(path), + None => Some(std::path::PathBuf::from(&name)), + }; let imported = matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb | crate::io::FileKind::Xls)) || matches!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Ods | gridcraft_xlsx::Format::Xlsb | gridcraft_xlsx::Format::Xls); @@ -122,6 +129,13 @@ fn save(s: &mut Session, p: &Json) -> Result { crate::io::write_file(&path, &bytes)?; // Only a full-fidelity format clears the dirty flag and becomes the document's file. if matches!(kind, Some(crate::io::FileKind::Xlsx | crate::io::FileKind::Json)) { + // The new file is the workbook's own now, so `CELL("filename")` reports it (#1489). Set + // before the `saved` snapshot, or the workbook would read as dirty the moment it was + // saved; and only when it changes, so an ordinary Save does not clone the workbook. + let file = crate::io::source_path(&path); + if d.wb.source_path != file { + std::sync::Arc::make_mut(&mut d.wb).source_path = file; + } d.saved = d.wb.clone(); d.path = Some(path.clone()); } diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 4712351ca..b3e4ee800 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -156,7 +156,7 @@ pub fn specs() -> Vec { "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 (pixels) | points: 15}", has_doc, row_height ), @@ -165,28 +165,34 @@ pub fn specs() -> Vec { "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 (pixels) | 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 (pixels)}", 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,29 +939,60 @@ fn cell_style(s: &mut Session, p: &Json) -> Result { // ---------------------------------------------------------------- sizes -fn line_targets(s: &Session, p: &Json, rows: bool) -> Result> { +/// 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}")))?; + let r = parse_lines(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::>>()?; + 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())) } +/// Sizes are pixels at 96 dpi, the model's unit (header drags send pixels). `points` (row height) +/// and `chars` (column width) take Excel's units, those of its Row Height and Column Width dialogs +/// and of the file (`ht` is in points). fn row_height(s: &mut Session, p: &Json) -> Result { - let h = f64_param(p, "height").ok_or_else(|| bad("home.rowHeight", "missing `height`"))?; - 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 h = match (f64_param(p, "height"), f64_param(p, "points")) { + (Some(h), _) => { + if !(0.0..=546.0).contains(&h) { + return Err(EngineError::Other("Row height must be between 0 and 546 pixels (409.5 points).".into())); + } + h + } + (None, Some(pt)) => { + if !(0.0..=409.5).contains(&pt) { + return Err(EngineError::Other("Row height must be between 0 and 409.5 points.".into())); + } + pt * 4.0 / 3.0 + } + _ => return Err(bad("home.rowHeight", "missing `height` (pixels) or `points`")), + }; + 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)) { @@ -972,16 +1009,22 @@ fn row_height(s: &mut Session, p: &Json) -> Result { fn col_width(s: &mut Session, p: &Json) -> Result { let w = match (f64_param(p, "width"), f64_param(p, "chars")) { - (Some(w), _) => w, - (None, Some(c)) => chars_to_points(c), - _ => return Err(bad("home.columnWidth", "missing `width`")), + (Some(w), _) => { + if !(0.0..=1800.0).contains(&w) { + return Err(EngineError::Other("Column width must be between 0 and 1800 pixels.".into())); + } + w + } + (None, Some(c)) => { + if !(0.0..=255.0).contains(&c) { + return Err(EngineError::Other("Column width must be between 0 and 255 characters.".into())); + } + chars_to_points(c) + } + _ => return Err(bad("home.columnWidth", "missing `width` (pixels) or `chars`")), }; - 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 { @@ -994,7 +1037,7 @@ fn col_width(s: &mut Session, p: &Json) -> Result { }) } -/// Excel column width in characters (of the default font's digit width, 7 px) → points. +/// Excel column width in characters (of the default font's digit width, 7 px) → pixels. pub fn chars_to_points(c: f64) -> f64 { if c <= 0.0 { 0.0 @@ -1008,7 +1051,7 @@ pub fn chars_to_points(c: f64) -> f64 { pub use gridcraft_model::points_to_chars; /// Approximate text width in points for autofit (no font metrics in the engine: average glyph -/// widths for the default font; the UI refines with real measurement when it autofits). +/// widths for the default font; a front end with fonts measures instead, see `Session::text_width`). pub fn approx_text_width(text: &str, size: f32, bold: bool) -> f32 { let mut w = 0.0; for c in text.chars() { @@ -1025,12 +1068,18 @@ pub fn approx_text_width(text: &str, size: f32, bold: bool) -> f32 { w * size * if bold { 1.07 } else { 1.0 } * 96.0 / 72.0 } +/// Distinct texts AutoFit measures exactly; past this many, `Session::text_width` is asked for a +/// quick measure so a huge column doesn't lay out every value. +const EXACT_TEXT_WIDTHS: usize = 2_000; + fn autofit_cols(s: &mut Session, p: &Json) -> Result { - let lines = line_targets(s, p, false)?; + let (si, lines) = line_targets(s, p, false)?; + let measure = s.text_width.clone(); 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 mut measured: std::collections::HashMap> = Default::default(); + let mut distinct = 0usize; let used = sh.used_range(); for (a, b) in lines { for c in a..=b.min(a + 16383) { @@ -1041,12 +1090,26 @@ fn autofit_cols(s: &mut Session, p: &Json) -> Result { if sh.merge_at(cell).is_some_and(|m| !m.is_single()) { continue; } - let st = cx.wb.styles.get(sh.style_id(cell)); + let id = sh.style_id(cell); + let st = cx.wb.styles.get(id); let text = crate::display::cell_text(&cx.wb, sh, cell); for line in text.lines() { - max = max.max( - approx_text_width(line, st.font.size, st.font.bold) + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell), - ); + let w = match &measure { + Some(m) => { + let known = measured.entry(id).or_default(); + match known.get(line) { + Some(w) => *w, + None => { + let w = m(line, &st.font, distinct < EXACT_TEXT_WIDTHS); + known.insert(line.to_owned(), w); + distinct += 1; + w + } + } + } + None => approx_text_width(line, st.font.size, st.font.bold), + }; + max = max.max(w + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell)); } } if max > 0.0 { @@ -1065,9 +1128,8 @@ fn autofit_cols(s: &mut Session, p: &Json) -> Result { } fn autofit_rows(s: &mut Session, p: &Json) -> Result { - 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 +1149,16 @@ fn autofit_rows(s: &mut Session, p: &Json) -> Result { fn default_width(s: &mut Session, p: &Json) -> Result { 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 { - 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 diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 9f044613d..34eee12dd 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -978,7 +978,7 @@ fn insert_picture(s: &mut Session, p: &Json) -> Result { // Excel inserts pictures as "Move but don't size with cells" (`editAs="oneCell"`). mode: anchor_mode_param(p).unwrap_or(gridcraft_model::AnchorMode::MoveOnly), }, - data, + data: data.into(), mime, alt: str_param(p, "alt").unwrap_or("").into(), }; diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index e18bae20b..d09164457 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -246,6 +246,11 @@ pub(crate) fn target_ranges(s: &Session, p: &Json) -> Result> { Ok(s.doc()?.selection.ranges.clone()) } +/// A `rows`/`cols` parameter: `3:5` and `B:C`, or a single `3` or `B`. +pub(crate) fn parse_lines(t: &str) -> Option { + RangeRef::parse(t).or_else(|| if t.contains(':') { None } else { RangeRef::parse(&format!("{t}:{t}")) }) +} + pub(crate) fn target_range(s: &Session, p: &Json) -> Result { Ok(target_ranges(s, p)?.first().copied().unwrap_or_default()) } @@ -388,6 +393,18 @@ fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String) Ok(()) } +/// Refuses an action a protected sheet does not allow, by the tick box for it in Review ▸ Protect +/// Sheet. `what` names the action in the message. An unprotected sheet allows everything. +/// +/// Cell edits are guarded separately, by whether the cell is locked (`locked_cell_error`): these +/// permissions are the ones that are not about one cell's contents. +pub(crate) fn refuse_unless_allowed(s: &Session, sheet: usize, what: &str, pick: fn(&gridcraft_model::SheetProtection) -> bool) -> Result<()> { + if s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.protection.as_ref().is_some_and(|pr| !pick(pr))) { + return Err(EngineError::Other(format!("{what} isn't allowed on this protected sheet (Review › Unprotect Sheet)."))); + } + Ok(()) +} + /// Whether a locked cell on a protected sheet stays locked: `None` when it's in an Allow Edit /// Range without a password or one unlocked in this session, else the message to show. pub(crate) fn locked_cell_error(sh: &gridcraft_model::Sheet, c: CellRef, unlocked: &[(String, String)]) -> Option { diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index c23b20dd4..58430d0dd 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -2016,7 +2016,7 @@ mod tests { wb.sheet_mut(0).unwrap().images.push(Image { id, anchor: Anchor { cell: CellRef::new(5, 5), dx: 0.0, dy: 0.0, width: 40.0, height: 40.0, mode: gridcraft_model::AnchorMode::MoveAndSize }, - data: png, + data: png.into(), mime: "image/png".into(), alt: String::new(), }); diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 14bb064a4..28f73e304 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -63,7 +63,7 @@ pub fn specs() -> Vec { fn lines(s: &Session, p: &Json, axis: Axis) -> Result<(u32, u32)> { let key = if axis == Axis::Rows { "rows" } else { "cols" }; let r = match str_param(p, key).or_else(|| str_param(p, "range")) { - Some(t) => RangeRef::parse(t).ok_or_else(|| bad(key, format!("not a range: {t}")))?, + Some(t) => parse_lines(t).ok_or_else(|| bad(key, format!("not a range: {t}")))?, None => s.doc()?.selection.current(), }; let (a, b) = if axis == Axis::Rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }; @@ -306,7 +306,7 @@ fn delete_areas(s: &Session, p: &Json, axis: Axis) -> Result> { } else if let Some(t) = str_param(p, key).or_else(|| str_param(p, "range")) { t.split(',') .map(|part| { - let r = RangeRef::parse(part).ok_or_else(|| bad(key, format!("not a range: {part}")))?; + let r = parse_lines(part).ok_or_else(|| bad(key, format!("not a range: {part}")))?; Ok(if axis == Axis::Rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }) }) .collect::>()? diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index edcacf57d..b76d8f70a 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -13,19 +13,29 @@ pub fn specs() -> Vec { "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")), + cmd!("view.showObjects", "Show Objects", [], Some("Ctrl+6"), "{on?}", has_doc, show_objects), cmd!("view.rightToLeft", "Sheet Right-to-Left", ["Page Layout", "Sheet Options"], None, "{on?}", has_doc, |s, p| sheet_flag(s, p, "rtl")), cmd!("view.zoom", "Zoom", ["View", "Zoom"], None, "{percent: 10..400}", has_doc, zoom), cmd!("view.zoom100", "100%", ["View", "Zoom"], None, "{}", has_doc, |s, _| zoom(s, &json!({"percent": 100}))), @@ -113,23 +123,28 @@ pub fn specs() -> Vec { } fn freeze_panes(s: &mut Session, p: &Json) -> Result { + 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 { +fn set_freeze(s: &mut Session, si: usize, f: Option<(u32, u32)>) -> Result { edit(s, |cx| { - let si = cx.sheet_index(); cx.sheet_mut(si)?.freeze = f; Ok(json!({"freeze": f})) }) @@ -152,6 +167,16 @@ fn sheet_flag(s: &mut Session, p: &Json, which: &str) -> Result { }) } +/// Ctrl+6: hides every object, or shows them again. Placeholders (from a file) count as shown. +fn show_objects(s: &mut Session, p: &Json) -> Result { + use gridcraft_model::ShowObjects; + edit(s, |cx| { + let on = bool_param(p, "on").unwrap_or(cx.wb.show_objects == ShowObjects::None); + cx.wb.show_objects = if on { ShowObjects::All } else { ShowObjects::None }; + Ok(json!({"on": on})) + }) +} + fn zoom(s: &mut Session, p: &Json) -> Result { let z = f64_param(p, "percent").ok_or_else(|| bad("view.zoom", "missing `percent`"))?.clamp(10.0, 400.0) as u16; let d = s.doc_mut()?; diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 14717a3df..62dddf81b 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -235,6 +235,12 @@ pub fn source_dir(path: &str) -> Option { std::path::absolute(path).ok()?.parent().map(std::path::Path::to_path_buf) } +/// The file at `path` as `CELL("filename")` reports it: absolute, so the folder is the one the +/// user sees. A workbook that arrived as bytes has no path and keeps only its name. +pub fn source_path(path: &str) -> Option { + std::path::absolute(path).ok() +} + /// Save As CSV/Text writes "the text and values as they are displayed in cells" with "all /// characters in each cell" (Microsoft Support, *Excel formatting and features that are not /// transferred to other file formats*): number formats applied, never column-width `#####`. @@ -673,7 +679,7 @@ mod tests { #[test] fn macros_are_saved_only_into_xlsm() { let mut wb = Workbook::new(); - wb.vba_project = Some(b"vba".to_vec()); + wb.vba_project = Some(b"vba".to_vec().into()); let has_macros = |path: &str| { let bytes = save_bytes(&wb, path).unwrap(); gridcraft_xlsx::read_xlsx(&bytes).unwrap().0.vba_project.is_some() diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..f52984c4c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -227,6 +227,13 @@ pub enum UiRequest { OpenUrl(String), } +/// Width in points of one line of text drawn in a cell font, measured by a front end that has +/// the font the grid draws with. With `exact` false the front end may answer with a cheaper +/// measure that is never narrower (AutoFit asks for it on columns with many distinct values). +/// Commands call it while they run, so a front end must not run commands while holding a lock +/// the measure takes (egui's: see `theme::cell_text_width` in the UI crate). +pub type TextWidth = Arc f32 + Send + Sync>; + #[derive(Default)] pub struct Session { docs: Vec, @@ -256,6 +263,9 @@ pub struct Session { /// Review › Notes › Show All Notes: every note is displayed. An application display setting /// (like Excel's comment indicator option), so it's not saved in the file. pub show_all_notes: bool, + /// Measures cell text for AutoFit Column Width. Without it (CLI, MCP) AutoFit uses + /// [`cmd::format::approx_text_width`]. + pub text_width: Option, } impl Session { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index dd3ca91ee..5e43016f1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1098,6 +1098,31 @@ fn case_sensitive_sort_puts_lowercase_first() { assert_eq!(col, ["a", "A", "aB", "Ab", "b", "B"].map(Value::from)); } +#[test] +fn sort_with_a_sheet_prefix_sorts_the_named_sheet() { + let col = |s: &Session, sheet: &str| -> Vec { + let wb = &s.doc().unwrap().wb; + let sh = wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap(); + (1..=3).map(|r| sh.value(CellRef::new(r, 0))).collect() + }; + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Qty"], ["a", 3], ["b", 1], ["c", 2]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item", "Price"], ["p1", 10], ["p2", 30], ["p3", 20]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.sort", json!({"range": "Prices!A1:B4", "header": true, "keys": [{"column": "B", "order": "desc"}]})).unwrap(); + assert_eq!(r["sorted"], "A2:B4"); + assert_eq!(col(&s, "Prices"), ["p2", "p3", "p1"].map(Value::from)); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); + s.execute("data.sortAscending", json!({"range": "Prices!A2:B4", "column": "B"})).unwrap(); + assert_eq!(col(&s, "Prices"), ["p1", "p3", "p2"].map(Value::from)); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); + // The selection belongs to the active sheet, so another sheet needs its range. + assert!(s.execute("data.sort", json!({"sheet": "Prices", "keys": [{"column": "B"}]})).is_err()); + assert!(s.execute("data.sort", json!({"range": "Nowhere!A1:B4", "keys": [{"column": "B"}]})).is_err()); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); +} + #[test] fn remove_duplicates_compares_shown_text() { let mut s = s(); @@ -1110,6 +1135,93 @@ fn remove_duplicates_compares_shown_text() { assert_eq!(r["removed"], 1); } +#[test] +fn remove_duplicates_with_a_sheet_prefix_works_on_the_named_sheet() { + let col = |s: &Session, sheet: &str| -> Vec { + let wb = &s.doc().unwrap().wb; + let sh = wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap(); + (1..=3).map(|r| sh.value(CellRef::new(r, 0))).collect() + }; + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name"], ["a"], ["a"], ["a"]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item"], ["p1"], ["p1"], ["p2"]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.removeDuplicates", json!({"range": "Prices!A1:A4", "header": true})).unwrap(); + assert_eq!(r["removed"], 1); + assert_eq!(col(&s, "Prices"), [Value::from("p1"), Value::from("p2"), Value::Empty]); + assert_eq!(col(&s, "Sheet1"), ["a", "a", "a"].map(Value::from)); + // The selection belongs to the active sheet, so another sheet needs its range. + assert!(s.execute("data.removeDuplicates", json!({"sheet": "Prices", "header": true})).is_err()); + assert!(s.execute("data.removeDuplicates", json!({"sheet": "Sheet1", "range": "Prices!A1:A4", "header": true})).is_err()); + assert!(s.execute("data.removeDuplicates", json!({"range": "Nowhere!A1:A4", "header": true})).is_err()); + assert_eq!(col(&s, "Sheet1"), ["a", "a", "a"].map(Value::from)); +} + +#[test] +fn filter_with_a_sheet_prefix_toggles_the_named_sheet() { + let sheet = |s: &Session, name: &str| -> gridcraft_model::Sheet { + let wb = &s.doc().unwrap().wb; + wb.sheet(wb.sheet_index(name).unwrap()).unwrap().clone() + }; + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Qty"], ["a", 3], ["b", 1]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item", "Price"], ["p1", 10], ["p2", 30]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.filter", json!({"range": "Prices!A1:B3"})).unwrap(); + assert_eq!(r["range"], "A1:B3"); + assert_eq!(sheet(&s, "Prices").autofilter.map(|a| a.range.a1()), Some("A1:B3".to_string())); + assert!(sheet(&s, "Sheet1").autofilter.is_none()); + s.execute("data.filter", json!({"range": "Prices!A1:B3"})).unwrap(); + assert!(sheet(&s, "Prices").autofilter.is_none()); + assert!(sheet(&s, "Sheet1").autofilter.is_none()); + assert!(s.execute("data.filter", json!({"sheet": "Prices"})).is_err()); +} + +#[test] +fn filter_with_a_sheet_prefix_toggles_the_table_at_the_range() { + let mut s = s(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item", "Price"], ["p1", 10], ["p2", 30]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "header": true})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + // The active cell is outside the table's cells, so only `range` can find the table. + s.execute("selection.set", json!({"range": "E10"})).unwrap(); + let prices = |s: &Session| { + let wb = &s.doc().unwrap().wb; + wb.sheet(wb.sheet_index("Prices").unwrap()).unwrap().clone() + }; + assert!(prices(&s).tables[0].filter_button); + s.execute("data.filter", json!({"range": "Prices!A2:B3"})).unwrap(); + assert!(!prices(&s).tables[0].filter_button); + assert!(prices(&s).autofilter.is_none()); + assert!(s.doc().unwrap().wb.active().unwrap().autofilter.is_none()); +} + +#[test] +fn subtotal_with_a_sheet_prefix_works_on_the_named_sheet() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Qty"], ["x", 1], ["x", 2], ["y", 3]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item", "Qty"], ["a", 1], ["a", 2], ["b", 4]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.subtotal", json!({"range": "Prices!A1:B4", "groupBy": "A", "columns": ["B"]})).unwrap(); + assert_eq!(r["groups"], 2); + let wb = &s.doc().unwrap().wb; + let prices = wb.sheet(wb.sheet_index("Prices").unwrap()).unwrap(); + let col = |sh: &gridcraft_model::Sheet, c: u32| -> Vec { (0..7).map(|r| sh.value(CellRef::new(r, c))).collect() }; + assert_eq!(col(prices, 0), ["Item", "a", "a", "a Total", "b", "b Total", "Grand Total"].map(Value::from)); + assert_eq!(col(prices, 1)[1..], [1.0, 2.0, 3.0, 4.0, 4.0, 7.0].map(Value::from)); + assert!(wb.styles.get(prices.style_id(CellRef::new(3, 0))).font.bold); + assert_eq!(prices.rows.get(&1).map(|l| l.outline), Some(1)); + let sheet1 = wb.sheet(wb.sheet_index("Sheet1").unwrap()).unwrap(); + assert_eq!(col(sheet1, 0)[..4], ["Name", "x", "x", "y"].map(Value::from)); + assert!(sheet1.value(CellRef::new(4, 0)).is_empty()); + assert!(sheet1.rows.values().all(|l| l.outline == 0)); + assert!(s.execute("data.group", json!({"sheet": "Prices"})).is_err()); +} + #[test] fn formatting_and_styles() { let mut s = s(); @@ -2640,7 +2752,7 @@ fn add_image(s: &mut Session, cell: &str, mode: gridcraft_model::AnchorMode) -> wb.sheet_mut(0).unwrap().images.push(gridcraft_model::Image { id, anchor: gridcraft_model::Anchor { cell: CellRef::parse(cell).unwrap(), dx: 0.0, dy: 0.0, width: 40.0, height: 40.0, mode }, - data: tiny_png(), + data: tiny_png().into(), mime: "image/png".into(), alt: String::new(), }); @@ -2651,6 +2763,35 @@ fn image_cell(s: &Session, id: u32) -> Option { s.doc().unwrap().wb.active().unwrap().images.iter().find(|i| i.id == id).map(|i| i.anchor.cell) } +/// Every undo step holds a copy of the workbook. A picture's bytes and the VBA project are shared +/// between those copies instead of copied into each: a 20 MB picture used to cost another 20 MB +/// for every cell typed on its sheet, up to 100 steps (2 GB). +#[test] +fn undo_steps_share_picture_and_macro_bytes() { + let mut s = s(); + s.execute("insert.picture", json!({"base64": crate::io::base64_encode(&tiny_png()), "at": "C3"})).unwrap(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).vba_project = Some(b"vba".to_vec().into()); + for i in 1..=3 { + s.execute("cell.set", json!({"cell": format!("A{i}"), "input": i.to_string()})).unwrap(); + } + let d = s.doc().unwrap(); + let live = &d.wb.active().unwrap().images[0].data; + let vba = d.wb.vba_project.as_ref().unwrap(); + assert_eq!(**live, tiny_png()); + assert!(d.undo.len() >= 3); + for step in d.undo.iter().filter(|h| !h.wb.active().unwrap().images.is_empty()) { + assert!(std::sync::Arc::ptr_eq(&step.wb.active().unwrap().images[0].data, live), "{}", step.label); + } + for step in d.undo.iter().rev().take(3) { + assert!(std::sync::Arc::ptr_eq(step.wb.vba_project.as_ref().unwrap(), vba), "{}", step.label); + } + // Undo and redo still give the picture back as it was. + s.execute("edit.undo", json!({"steps": 3})).unwrap(); + assert_eq!(*s.doc().unwrap().wb.active().unwrap().images[0].data, tiny_png()); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(*s.doc().unwrap().wb.active().unwrap().images[0].data, tiny_png()); +} + /// Issue #5: images anchored in cells follow their row when the sheet is sorted. #[test] fn sort_moves_anchored_images_with_their_rows() { @@ -3153,6 +3294,91 @@ 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 row_height_and_column_width_take_pixels_or_excel_units() { + // `height` and `width` are pixels at 96 dpi (the model's unit); `points` and `chars` are + // Excel's units, those of its Row Height and Column Width dialogs. A row is saved in points. + let mut s = s(); + s.execute("home.rowHeight", json!({"rows": "1:1", "points": 30})).unwrap(); + s.execute("home.rowHeight", json!({"rows": "2:2", "height": 30})).unwrap(); + s.execute("home.rowHeight", json!({"rows": "3:3", "points": 409.5})).unwrap(); + s.execute("home.columnWidth", json!({"cols": "A:A", "chars": 8.43})).unwrap(); + s.execute("home.columnWidth", json!({"cols": "B:B", "width": 100})).unwrap(); + let sheet = s.doc().unwrap().wb.active().unwrap(); + assert_eq!([sheet.row_height(0), sheet.row_height(1), sheet.row_height(2)], [40.0, 30.0, 546.0]); + assert_eq!([sheet.col_width(0), sheet.col_width(1)], [64.0, 100.0]); + let bytes = gridcraft_xlsx::write_xlsx(&s.doc().unwrap().wb).unwrap(); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut xml = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("xl/worksheets/sheet1.xml").unwrap(), &mut xml).unwrap(); + for row in [r#" = (0..5000).map(|i| json!([format!("v{}", i % 2500)])).collect(); + s.execute("range.setValues", json!({"range":"A1", "values": values})).unwrap(); + let calls = Arc::new(Mutex::new(Vec::new())); + let seen = calls.clone(); + s.text_width = Some(Arc::new(move |text: &str, _: &gridcraft_engine::model::Font, exact: bool| { + seen.lock().unwrap().push((text.to_owned(), exact)); + if text == "v2499" { 300.0 } else { 10.0 } + })); + s.execute("home.autofitColumnWidth", json!({"cols":"A:A"})).unwrap(); + let calls = calls.lock().unwrap(); + assert_eq!(calls.len(), 2500, "one measure per distinct text"); + assert_eq!(calls.iter().filter(|(_, exact)| *exact).count(), 2000); + assert!(calls.iter().skip(2000).all(|(_, exact)| !exact)); + // The quick measures still size the column. + assert_eq!(s.doc().unwrap().wb.active().unwrap().col_width(0), 306.0); +} diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index de6bbc6c5..c699ccf69 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -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"})); } diff --git a/crates/model/src/b64.rs b/crates/model/src/b64.rs index 30db4b43d..b377fe451 100644 --- a/crates/model/src/b64.rs +++ b/crates/model/src/b64.rs @@ -49,22 +49,40 @@ pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result, D::Error> decode(&s).ok_or_else(|| serde::de::Error::custom("invalid base64")) } -/// Base64 for optional byte blobs (VBA projects): `null`/missing stays `None`. +/// Base64 for byte blobs shared through `Arc` (floating pictures), so copies of a workbook, such as +/// its undo steps, share one payload. +pub mod shared { + use std::sync::Arc; + + use serde::{Deserializer, Serializer}; + + pub fn serialize(v: &Arc>, s: S) -> Result { + super::serialize(v, s) + } + + pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result>, D::Error> { + super::deserialize(d).map(Arc::new) + } +} + +/// Base64 for optional shared byte blobs (VBA projects): `null`/missing stays `None`. pub mod opt { + use std::sync::Arc; + use serde::{Deserialize, Deserializer, Serializer}; - pub fn serialize(v: &Option>, s: S) -> Result { + pub fn serialize(v: &Option>>, s: S) -> Result { match v { Some(data) => s.serialize_str(&super::encode(data)), None => s.serialize_none(), } } - pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result>, D::Error> { + pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result>>, D::Error> { let s = Option::::deserialize(d)?; match s { None => Ok(None), - Some(s) => super::decode(&s).map(Some).ok_or_else(|| serde::de::Error::custom("invalid base64")), + Some(s) => super::decode(&s).map(|v| Some(Arc::new(v))).ok_or_else(|| serde::de::Error::custom("invalid base64")), } } } diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs index 9f30ca079..f214daba3 100644 --- a/crates/model/src/features.rs +++ b/crates/model/src/features.rs @@ -520,9 +520,10 @@ pub struct Chart { pub struct Image { pub id: u32, pub anchor: Anchor, - /// Encoded bytes (PNG/JPEG), base64 in JSON. - #[serde(with = "crate::b64")] - pub data: Vec, + /// Encoded bytes (PNG/JPEG), base64 in JSON. Shared, so a copy of the sheet (an undo step) + /// doesn't copy the picture. + #[serde(with = "crate::b64::shared")] + pub data: std::sync::Arc>, pub mime: String, pub alt: String, } @@ -879,7 +880,7 @@ pub fn password_hash(pw: &str) -> u64 { /// Outline grouping info for a row or column. #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct LineInfo { - /// Size in points; `None` = default (rows auto-fit to content). + /// Size in pixels at 96 dpi; `None` = default (rows auto-fit to content). pub size: Option, pub hidden: bool, pub outline: u8, diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index a76f30fc3..071de683c 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -25,7 +25,7 @@ pub use style::{ Alignment, BorderLine, BorderStyle, Borders, Color, Fill, Font, HAlign, NumFmt, PatternType, Style, StyleId, StyleTable, Theme, Underline, VAlign, VertAlign, }; -pub use workbook::{CalcMode, CalcSettings, DefinedName, DocProps, Workbook}; +pub use workbook::{CalcMode, CalcSettings, DefinedName, DocProps, PhoneticProps, PhoneticRun, PhoneticString, ShowObjects, Workbook}; /// Font used for new workbooks. Only a name: renderers substitute an available open font /// (e.g. the metric-compatible Carlito) when it isn't installed. diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 50c71a100..1901d8048 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -64,7 +64,7 @@ pub struct Sheet { pub spill_ranges: BTreeMap, pub cols: BTreeMap, pub rows: BTreeMap, - /// Default column width and row height in points. + /// Default column width and row height in pixels at 96 dpi (a file stores row heights in points). pub default_col_width: f32, pub default_row_height: f32, pub merges: Vec, @@ -113,7 +113,7 @@ pub struct Sheet { pub preserved: Option>, } -/// Default width of a column (points) and height of a row (points) for the default font. +/// Default width of a column and height of a row (pixels at 96 dpi) for the default font. pub const DEFAULT_COL_WIDTH: f32 = 64.0; pub const DEFAULT_ROW_HEIGHT: f32 = 20.0; diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index e5f359614..3b943d295 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -18,6 +18,17 @@ pub enum CalcMode { Manual, } +/// Options › Advanced › "For objects, show" (``, ECMA-376 `ST_Objects`): +/// whether charts, pictures, shapes and slicers are drawn. Ctrl+6 switches between `All` and +/// `None`; `Placeholders` only comes from files and is kept as it is. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub enum ShowObjects { + #[default] + All, + Placeholders, + None, +} + /// File › Options › Formulas › Calculation options, stored per workbook (``). #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct CalcSettings { @@ -85,6 +96,47 @@ pub struct DocProps { pub description: String, } +/// One phonetic (furigana) run of a string item (``): the reading `text` and the range of the +/// base text it annotates. +/// +/// `start` and `end` are zero-based offsets into the base text (`sb` and `eb` in the file) with +/// `start < end`; reading keeps them within the text's length in UTF-16 code units. Behaviour +/// reference: ISO/IEC 29500-1 §18.4.6 `rPh` and the Microsoft Learn `PhoneticRun` class. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct PhoneticRun { + pub start: u32, + pub end: u32, + pub text: String, +} + +/// The `` of a string item: how its phonetic runs are displayed. `kind` (`type`) and +/// `alignment` only ever hold values ISO/IEC 29500-1 §18.18.57 `ST_PhoneticType` / §18.18.56 +/// `ST_PhoneticAlignment` define, so damaged files cannot be written back invalid. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct PhoneticProps { + /// `fontId`: a required attribute, an index into the style sheet's fonts. A file without one + /// gets 0 — the Normal style's font, which an out-of-range index falls back to anyway. + #[serde(default)] + pub font_id: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub kind: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub alignment: Option, +} + +/// The phonetic (furigana) data of one shared string: its `` runs, in document order, and its +/// `` when the file had one. +/// +/// `text` is the string's base text — the only handle a cell keeps on its shared string — and is +/// the key the writer uses to re-emit the runs next to the right ``. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct PhoneticString { + pub text: String, + pub runs: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub props: Option, +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Workbook { pub sheets: Vec>, @@ -93,6 +145,8 @@ pub struct Workbook { pub theme: Theme, pub date_system: DateSystem, pub calc: CalcSettings, + #[serde(default, skip_serializing_if = "is_all")] + pub show_objects: ShowObjects, pub active_sheet: usize, pub props: DocProps, /// Workbook structure protection. @@ -104,11 +158,23 @@ pub struct Workbook { pub custom_lists: Vec>, /// Named cell styles (Cell Styles gallery): name → style. pub cell_styles: Vec<(String, crate::style::Style)>, + /// Phonetic (furigana) data of shared strings (`xl/sharedStrings.xml`), one entry per string + /// text, so that an open + save keeps a Japanese workbook's readings. Written back next to the + /// `` holding the same text. + /// + /// Keyed by text because that is all a cell keeps of its shared string: the saved table holds + /// each text once (the format's `uniqueCount`), so two `` items that share a base text can + /// only keep one reading — the reader keeps the first and warns. Empty for workbooks without + /// phonetic data; not present in older JSON. Modelled data (unlike [`Workbook::preserved`], + /// which holds unmodelled parts), so it is saved to JSON as well as to XLSX. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub phonetics: Vec, /// VBA project (`xl/vbaProject.bin`) carried through unchanged. GridCraft does not run /// macros; the blob is preserved so that saving a macro-enabled workbook never destroys the - /// macros it arrived with. base64 in JSON. `None` = the workbook has no macros. + /// macros it arrived with. base64 in JSON. `None` = the workbook has no macros. Shared, so a + /// copy of the workbook (every edit's undo step) doesn't copy it. #[serde(default, with = "crate::b64::opt")] - pub vba_project: Option>, + pub vba_project: Option>>, /// Field separator of the CSV this workbook was read from, so saving it as CSV writes the same /// separator back (a `;` file stays a `;` file). `None` = not read from CSV: CSV saves use `,`. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -120,6 +186,17 @@ pub struct Workbook { /// `preserved` are relative to. Saving into another folder rewrites them from there. Not in JSON. #[serde(skip)] pub source_dir: Option, + /// The file this workbook reports as its own, which is what `CELL("filename")` returns. + /// `None` = no file yet, which Excel reports as an empty string until the workbook has been + /// saved once. Deliberately separate from `source_dir`: that one names the folder relative + /// links are relative to and must keep naming it after a Save As into another folder, since + /// the links in memory are still written for the old one. Not in JSON. + #[serde(skip)] + pub source_path: Option, +} + +fn is_all(s: &ShowObjects) -> bool { + *s == ShowObjects::All } impl Default for Workbook { @@ -138,16 +215,19 @@ impl Workbook { theme: Theme::default(), date_system: DateSystem::D1900, calc: CalcSettings::default(), + show_objects: ShowObjects::All, active_sheet: 0, props: DocProps::default(), protected_structure: false, structure_password: None, custom_lists: vec![], cell_styles: vec![], + phonetics: vec![], vba_project: None, csv_delimiter: None, preserved: None, source_dir: None, + source_path: None, } } pub fn sheet(&self, i: usize) -> Option<&Sheet> { @@ -296,6 +376,23 @@ fn spell_calls(e: Expr, lambdas: &[String]) -> Expr { mod tests { use super::*; + #[test] + fn phonetics_are_additive_in_json() { + let mut wb = Workbook::new(); + assert!(wb.phonetics.is_empty(), "new workbooks have no phonetic data"); + wb.phonetics.push(PhoneticString { + text: "東".into(), + runs: vec![PhoneticRun { start: 0, end: 1, text: "ヒガシ".into() }], + props: Some(PhoneticProps { font_id: 3, kind: Some("Hiragana".into()), alignment: None }), + }); + let json = serde_json::to_string(&wb).expect("serialize"); + assert_eq!(serde_json::from_str::(&json).expect("deserialize").phonetics, wb.phonetics); + // JSON written before the field existed still loads. + let mut value: serde_json::Value = serde_json::from_str(&json).expect("value"); + value.as_object_mut().expect("an object").remove("phonetics"); + assert!(serde_json::from_value::(value).expect("older JSON").phonetics.is_empty()); + } + #[test] fn sheet_names() { let wb = Workbook::new(); diff --git a/crates/ui-egui/src/chartview.rs b/crates/ui-egui/src/chartview.rs index 180f15c21..fad08ff22 100644 --- a/crates/ui-egui/src/chartview.rs +++ b/crates/ui-egui/src/chartview.rs @@ -1,9 +1,6 @@ //! Floating objects on the sheet: charts (rendered by `gridcraft-chart` into primitives that //! we paint with egui), pictures and shapes. -use std::collections::HashMap; -use std::sync::Mutex; - use egui::epaint::{PathShape, PathStroke}; use egui::{Align2, Color32, FontId, Painter, Pos2, Rect, Stroke, StrokeKind, pos2, vec2}; use gridcraft_chart::{HAlign, Prim, VAlign}; @@ -249,8 +246,6 @@ pub(crate) fn anchor_rect(geo: &Geo, sh: &Sheet, a: &Anchor) -> Rect { Rect::from_min_size(pos2(x, y), vec2(a.width * geo.z, a.height.max(1.0) * geo.z)) } -static TEXTURES: std::sync::LazyLock>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new())); - /// The shapes one floating object painted (indices into the painter's layer) and its logical /// rectangle, so a right-to-left sheet can move it as a whole instead of mirroring it. pub(crate) struct PaintedObject { @@ -258,9 +253,20 @@ pub(crate) struct PaintedObject { pub rect: Rect, } +/// Ctrl+6 (Options › Advanced › "For objects, show: Nothing"): objects are neither drawn nor +/// clickable. Placeholders, which Excel no longer offers, show the objects. +pub(crate) fn objects_hidden(wb: &Workbook) -> bool { + wb.show_objects == gridcraft_engine::model::ShowObjects::None +} + pub(crate) fn paint_objects(app: &SheetApp, p: &Painter, geo: &Geo, wb: &Workbook, si: usize, sh: &Sheet) -> Vec { let mut painted = Vec::new(); + if objects_hidden(wb) { + return painted; + } let mut done = |start: egui::layers::ShapeIdx, rect: Rect| painted.push(PaintedObject { shapes: start.0..p.add(egui::Shape::Noop).0, rect }); + // The grid paints the active workbook: its pictures are cached under it. + let doc = app.session.active().map_or(0, |d| d.uid); for sp in &sh.shapes { let r = anchor_rect(geo, sh, &sp.anchor); if !r.intersects(p.clip_rect()) { @@ -332,18 +338,7 @@ pub(crate) fn paint_objects(app: &SheetApp, p: &Painter, geo: &Geo, wb: &Workboo continue; } let start = p.add(egui::Shape::Noop); - let key = (im.id, im.data.len()); - let mut cache = TEXTURES.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if !cache.contains_key(&key) - && let Ok(img) = image::load_from_memory(&im.data) - { - let rgba = img.to_rgba8(); - let size = [rgba.width() as usize, rgba.height() as usize]; - let ci = egui::ColorImage::from_rgba_unmultiplied(size, rgba.as_raw()); - let tex = p.ctx().load_texture(format!("img{}", im.id), ci, egui::TextureOptions::LINEAR); - cache.insert(key, tex); - } - if let Some(tex) = cache.get(&key) { + if let Some(tex) = crate::floating_pictures::texture(p.ctx(), doc, im, r) { p.image(tex.id(), r, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); } else { p.rect_filled(r, 0.0, Color32::from_gray(230)); @@ -472,7 +467,7 @@ pub enum SlicerAction { /// A click on a slicer's value tile or clear button, if any. A click elsewhere in a slicer returns /// `None` so the caller can treat it as a select/drag on the object. pub fn slicer_click(geo: &Geo, sh: &Sheet, wb: &Workbook, si: usize, p: Pos2) -> Option<(u32, SlicerAction)> { - if !geo.cells.contains(p) { + if !geo.cells.contains(p) || objects_hidden(wb) { return None; } for sl in sh.slicers.iter().rev() { @@ -506,7 +501,9 @@ fn selection_frame(p: &Painter, r: Rect) { /// The topmost object under `p`: (kind, id, rect). pub fn hit(app: &SheetApp, geo: &Geo, sh: &Sheet, p: Pos2) -> Option<(&'static str, u32, Rect)> { - let _ = app; + if app.session.active().is_some_and(|d| objects_hidden(&d.wb)) { + return None; + } for sl in sh.slicers.iter().rev() { let r = anchor_rect(geo, sh, &sl.anchor); if r.contains(p) && geo.cells.contains(p) { diff --git a/crates/ui-egui/src/floating_pictures.rs b/crates/ui-egui/src/floating_pictures.rs new file mode 100644 index 000000000..68de6e203 --- /dev/null +++ b/crates/ui-egui/src/floating_pictures.rs @@ -0,0 +1,274 @@ +//! Floating pictures (Insert › Pictures) as textures: decoded once at about the size they are +//! drawn, and kept in a cache per egui context that is bounded by bytes. The cache used to be +//! process-wide, with no limit, decoding every picture at full size and keeping it until the app +//! closed, after its workbook was closed too. + +use std::collections::HashMap; +use std::io::Cursor; +use std::sync::{Arc, Mutex}; + +use egui::TextureHandle; +use gridcraft_engine::model::Image; + +/// Larger pictures are not decoded: the header is read first, so nothing is allocated for them. +const MAX_SIDE: u32 = 16_384; +const MAX_PIXELS: u64 = 64_000_000; +const MAX_DECODE_BYTES: u64 = 512 * 1024 * 1024; +/// A texture is never wider or taller than this (nor than the GPU allows). +const MAX_TEXTURE_SIDE: u32 = 4096; +/// The smallest texture side, so a picture drawn small and zoomed in a little isn't decoded again. +const MIN_TEXTURE_SIDE: u32 = 64; +/// Pictures drawn in the last frame stay whatever this says, so none turns into a placeholder; +/// the others are dropped, least recently drawn first, to keep the cache under it. +const MAX_CACHE_BYTES: usize = 128 * 1024 * 1024; +const MAX_CACHE_ITEMS: usize = 4096; + +/// A picture: its workbook, its object id there, and its size in bytes (object ids are per +/// workbook, so two open workbooks can both have a picture 1). +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] +struct Key { + doc: u64, + id: u32, + len: usize, +} + +#[derive(Default)] +struct Cache { + entries: HashMap, + bytes: usize, + /// The open workbooks when the cache was last pruned. + docs: Vec, +} + +struct Entry { + /// `None` when the picture can't be decoded: kept, so it isn't tried again every frame. + texture: Option, + bytes: usize, + /// The longest side it was decoded for; `u32::MAX` when it holds all there is (the source + /// size or the largest texture allowed), or when it can't be decoded. + side: u32, + used: u64, +} + +impl Cache { + /// Drops the pictures of workbooks that are no longer open. + fn forget_closed(&mut self, open: &[u64]) { + if self.docs == open { + return; + } + self.entries.retain(|key, entry| { + let keep = open.contains(&key.doc); + if !keep { + self.bytes = self.bytes.saturating_sub(entry.bytes); + } + keep + }); + self.docs = open.to_vec(); + } + + /// The texture for picture `key` drawn with its longer side `want` pixels long. + fn texture(&mut self, ctx: &egui::Context, key: Key, data: &[u8], want: u32) -> Option { + let frame = ctx.cumulative_frame_nr(); + let side = texture_side(want, ctx.input(|i| i.max_texture_side)); + if let Some(entry) = self.entries.get_mut(&key) { + entry.used = frame; + if entry.side >= side { + return entry.texture.clone(); + } + } + // Not cached, or drawn larger than it was decoded for: decode it again at the new size. + if let Some(old) = self.entries.remove(&key) { + self.bytes = self.bytes.saturating_sub(old.bytes); + } + let decoded = decode(data, side); + let bytes = decoded.as_ref().map_or(0, |(image, _)| image.pixels.len().saturating_mul(4)); + while self.entries.len() >= MAX_CACHE_ITEMS || self.bytes.saturating_add(bytes) > MAX_CACHE_BYTES { + let oldest = self.entries.iter().filter(|(_, e)| e.used.saturating_add(1) < frame).min_by_key(|(_, e)| e.used).map(|(key, _)| *key); + let Some(oldest) = oldest else { break }; + if let Some(entry) = self.entries.remove(&oldest) { + self.bytes = self.bytes.saturating_sub(entry.bytes); + } + } + let (texture, side) = match decoded { + Some((image, complete)) => { + (Some(ctx.load_texture(format!("img{}", key.id), image, egui::TextureOptions::LINEAR)), if complete { u32::MAX } else { side }) + } + None => (None, u32::MAX), + }; + self.bytes = self.bytes.saturating_add(bytes); + self.entries.insert(key, Entry { texture: texture.clone(), bytes, side, used: frame }); + texture + } +} + +/// The texture side for a picture drawn `want` pixels long: the next power of two or three +/// quarters of one (…, 768, 1024, 1536, 2048), so zooming in decodes it again only at every +/// half or third larger, within the smallest and largest sides allowed. +fn texture_side(want: u32, gpu_max: usize) -> u32 { + let max = MAX_TEXTURE_SIDE.min(u32::try_from(gpu_max).unwrap_or(u32::MAX)).max(1); + let want = want.max(MIN_TEXTURE_SIDE); + let Some(p) = want.checked_next_power_of_two() else { return max }; + let side = if want <= p / 4 * 3 { p / 4 * 3 } else { p }; + side.min(max) +} + +/// Decodes a picture to fit a `side` × `side` square, and says whether that is the whole picture +/// (it was no larger, or `side` is as large as a texture may be). +fn decode(data: &[u8], side: u32) -> Option<(egui::ColorImage, bool)> { + let format = image::guess_format(data).ok()?; + let (w, h) = image::ImageReader::with_format(Cursor::new(data), format).into_dimensions().ok()?; + if w == 0 || h == 0 || w > MAX_SIDE || h > MAX_SIDE || u64::from(w) * u64::from(h) > MAX_PIXELS { + return None; + } + let mut reader = image::ImageReader::with_format(Cursor::new(data), format); + let mut limits = image::Limits::default(); + limits.max_image_width = Some(MAX_SIDE); + limits.max_image_height = Some(MAX_SIDE); + limits.max_alloc = Some(MAX_DECODE_BYTES); + reader.limits(limits); + let mut picture = reader.decode().ok()?; + let complete = w.max(h) <= side || side >= MAX_TEXTURE_SIDE; + if w.max(h) > side { + picture = picture.thumbnail(side, side); + } + let rgba = picture.into_rgba8(); + Some((egui::ColorImage::from_rgba_unmultiplied([rgba.width() as usize, rgba.height() as usize], rgba.as_raw()), complete)) +} + +fn cache(ctx: &egui::Context) -> Arc> { + // Context-local, like cell pictures: texture handles never leak between offscreen, native + // and browser contexts. + ctx.data_mut(|data| data.get_temp_mut_or_default::>>(egui::Id::new("gridcraft.floating_pictures")).clone()) +} + +/// Drops the textures of workbooks that are no longer open. Called every frame the grid paints. +pub(crate) fn forget_closed(ctx: &egui::Context, open: &[u64]) { + cache(ctx).lock().unwrap_or_else(std::sync::PoisonError::into_inner).forget_closed(open); +} + +/// The texture of picture `image` of workbook `doc`, drawn in `rect` (points); `None` when it +/// can't be decoded or is too large to be. +pub(crate) fn texture(ctx: &egui::Context, doc: u64, image: &Image, rect: egui::Rect) -> Option { + let px = (rect.width().max(rect.height()) * ctx.pixels_per_point()).ceil(); + let want = if px.is_finite() && px > 0.0 { px.min(u32::MAX as f32) as u32 } else { 1 }; + let key = Key { doc, id: image.id, len: image.data.len() }; + cache(ctx).lock().unwrap_or_else(std::sync::PoisonError::into_inner).texture(ctx, key, &image.data, want) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn png(w: u32, h: u32, color: [u8; 4]) -> Vec { + let image = image::RgbaImage::from_pixel(w, h, image::Rgba(color)); + let mut bytes = Cursor::new(Vec::new()); + image.write_to(&mut bytes, image::ImageFormat::Png).unwrap(); + bytes.into_inner() + } + + fn key(doc: u64, id: u32, data: &[u8]) -> Key { + Key { doc, id, len: data.len() } + } + + /// Runs one frame, so the frame number moves on as it does when the app paints. + fn frame(ctx: &egui::Context) { + let mut out = ctx.run_ui(Default::default(), |_| {}); + out.textures_delta.clear(); // nothing consumes the uploads in a headless test + } + + #[test] + fn pictures_are_decoded_at_about_the_size_drawn() { + let ctx = egui::Context::default(); + let mut cache = Cache::default(); + let data = png(3000, 1000, [10, 120, 200, 255]); + let k = key(1, 1, &data); + let small = cache.texture(&ctx, k, &data, 300).unwrap(); + assert_eq!(small.size(), [384, 128], "drawn 300 px long: decoded for 384"); + assert_eq!(cache.texture(&ctx, k, &data, 380).unwrap().id(), small.id(), "still within its size: not decoded again"); + let larger = cache.texture(&ctx, k, &data, 900).unwrap(); + assert_eq!(larger.size(), [1024, 341], "drawn larger: decoded again for 1024"); + assert_eq!(cache.entries.len(), 1); + assert_eq!(cache.bytes, 1024 * 341 * 4, "the smaller texture's bytes are given back"); + assert_eq!(texture_side(1280, 8192), 1536); + assert_eq!(texture_side(1600, 8192), 2048); + assert_eq!(texture_side(1, 8192), MIN_TEXTURE_SIDE); + assert_eq!(texture_side(u32::MAX, 8192), MAX_TEXTURE_SIDE); + // Never past the GPU's limit (2048 in a default context) nor the source. + assert_eq!(cache.texture(&ctx, k, &data, 50_000).unwrap().size(), [2048, 683]); + let tiny = png(40, 20, [0, 0, 0, 255]); + assert_eq!(cache.texture(&ctx, key(1, 2, &tiny), &tiny, 4000).unwrap().size(), [40, 20]); + assert_eq!(cache.entries.get(&key(1, 2, &tiny)).map(|e| e.side), Some(u32::MAX), "the whole picture: never decoded again"); + } + + #[test] + fn the_cache_stays_within_its_budget_but_keeps_what_is_on_screen() { + let ctx = egui::Context::default(); + let mut cache = Cache::default(); + // 40 pictures of 2048 × 2048 drawn in turn, one frame each: 640 MB decoded in all. + let data: Vec> = (0..40u8).map(|i| png(2048, 2048, [i, 0, 0, 255])).collect(); + for (i, d) in data.iter().enumerate() { + assert!(cache.texture(&ctx, key(1, i as u32, d), d, 2048).is_some()); + frame(&ctx); + } + assert!(cache.bytes <= MAX_CACHE_BYTES, "{} bytes kept", cache.bytes); + assert!(cache.entries.len() < data.len()); + // Many on screen at once, over the budget: every one keeps its texture, frame after frame. + let shown: Vec<_> = data.iter().enumerate().take(12).map(|(i, d)| cache.texture(&ctx, key(1, i as u32, d), d, 2048).unwrap().id()).collect(); + for _ in 0..3 { + frame(&ctx); + for (i, d) in data.iter().enumerate().take(12) { + assert_eq!(cache.texture(&ctx, key(1, i as u32, d), d, 2048).map(|t| t.id()), Some(shown[i])); + } + } + } + + #[test] + fn a_closed_workbook_takes_its_pictures_with_it() { + let ctx = egui::Context::default(); + let mut cache = Cache::default(); + let data = png(64, 64, [0, 200, 0, 255]); + let first = cache.texture(&ctx, key(1, 1, &data), &data, 64).unwrap(); + // Another workbook's picture 1, of the same size: a texture of its own. + let other = png(64, 64, [200, 0, 0, 255]); + assert_eq!(other.len(), data.len()); + assert_ne!(cache.texture(&ctx, key(2, 1, &other), &other, 64).unwrap().id(), first.id()); + cache.forget_closed(&[1, 2]); + assert_eq!(cache.entries.len(), 2); + cache.forget_closed(&[2]); + assert_eq!(cache.entries.len(), 1); + assert_eq!(cache.bytes, 64 * 64 * 4); + cache.forget_closed(&[]); + assert!(cache.entries.is_empty()); + assert_eq!(cache.bytes, 0); + } + + #[test] + fn broken_and_oversized_pictures_are_refused_once() { + let ctx = egui::Context::default(); + let mut cache = Cache::default(); + let garbage = vec![0xAB; 100]; + assert!(cache.texture(&ctx, key(1, 1, &garbage), &garbage, 100).is_none()); + assert_eq!(cache.entries.get(&key(1, 1, &garbage)).map(|e| (e.side, e.bytes)), Some((u32::MAX, 0)), "remembered, not retried"); + // A PNG whose header says 20,000 × 20,000 is refused from its header: nothing is decoded. + let mut huge = png(1, 1, [0, 0, 0, 255]); + let ihdr = huge.windows(4).position(|w| w == b"IHDR").unwrap(); + huge[ihdr + 4..ihdr + 8].copy_from_slice(&20_000u32.to_be_bytes()); + huge[ihdr + 8..ihdr + 12].copy_from_slice(&20_000u32.to_be_bytes()); + let crc = crc32(&huge[ihdr..ihdr + 17]); + huge[ihdr + 17..ihdr + 21].copy_from_slice(&crc.to_be_bytes()); + assert_eq!(image::ImageReader::with_format(Cursor::new(&huge), image::ImageFormat::Png).into_dimensions().unwrap(), (20_000, 20_000)); + assert!(decode(&huge, 64).is_none()); + assert!(decode(&png(4, 2, [255, 0, 0, 255]), 64).is_some_and(|(i, complete)| i.size == [4, 2] && complete)); + } + + fn crc32(bytes: &[u8]) -> u32 { + let mut crc = !0u32; + for &b in bytes { + crc ^= u32::from(b); + for _ in 0..8 { + crc = if crc & 1 == 1 { (crc >> 1) ^ 0xEDB8_8320 } else { crc >> 1 }; + } + } + !crc + } +} diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 9444d369a..a09f5e4fd 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -717,6 +717,9 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { app.grid.cells_rect = Some(geo.mirror_rect(geo.cells)); // The selection outline glides to a new selection. It stays on its cells when Animations is // off, when egui's animations are (the headless harness), and while a screenshot is on its way. + // A mouse button held down (a click or a drag selecting cells) moves it at once, as in Excel: + // a gliding outline trails the pointer and reads as lag. + let pointer_selecting = ui.input(|i| i.pointer.any_down()); let outline = app.grid.glide.frame(crate::selection_glide::Frame { place, range: sel.current(), @@ -724,7 +727,7 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { view: geo.cells, now, frame_nr: ui.ctx().cumulative_frame_nr(), - animate: app.ui.animations && ui.ctx().global_style().animation_time > 0.0 && !app.shots.in_flight(), + animate: app.ui.animations && ui.ctx().global_style().animation_time > 0.0 && !app.shots.in_flight() && !pointer_selecting, }); if app.grid.glide.is_gliding() { ui.ctx().request_repaint(); @@ -740,7 +743,11 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { } else { paint_panes(app, &painter, &geo, &wb, si, sh, &sel, outline, &mut cf, &t) }; - // Objects above cells. + // Objects above cells. A hidden object can't stay selected: Delete or the Format pane would act + // on something the user can't see. + if crate::chartview::objects_hidden(&wb) { + app.selected_chart = None; + } let objects = crate::chartview::paint_objects(app, &painter.with_clip_rect(geo.cells), &geo, &wb, si, sh); paint_overlays(app, ui, &painter.with_clip_rect(geo.cells), &geo, &wb, si, sh, &sel); if sh.view == gridcraft_engine::model::SheetViewMode::PageBreakPreview { diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 032db1a2d..358db5a5f 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -92,6 +92,8 @@ pub fn lookup(key: Key, physical: Option, m: Modifiers, mac: bool) -> Optio Num3 if ctrl && !mac => run("home.italic"), Num4 if ctrl && !mac => run("home.underline"), Num5 if ctrl && !mac => run("home.strikethrough"), + // Ctrl+6 hides and shows objects (Windows, and Control+6 in Excel for Mac). + Num6 if ctrl && !s => run("view.showObjects"), // Letters that differ between Windows and Mac. L if (cmd || ctrl) && s => run("data.filter"), L if cmd && mac => run("home.alignLeft"), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ca1801be2..32960f8e6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -14,6 +14,7 @@ pub mod control; pub mod credits; pub mod dialogs; pub mod editor; +mod floating_pictures; pub mod fonts; pub mod formula_bar; mod formula_locale; @@ -1135,9 +1136,16 @@ impl SheetApp { /// Per-frame logic (control channel, screenshots). Call before `ui`. pub fn logic(&mut self, ctx: &egui::Context) { + if self.session.text_width.is_none() { + let ctx = ctx.clone(); + self.session.text_width = Some(std::sync::Arc::new(move |text, font, exact| theme::cell_text_width(&ctx, text, font, exact))); + } // A keytip control request lives one frame: the ribbon consumes it during `ui`, and // anything that never reached a control (collapsed ribbon, tab switch) is dropped. self.ui.keytip_invoke = None; + // Floating pictures' textures go with their workbook, even when no sheet is painted. + let open: Vec = self.session.documents().iter().map(|d| d.uid).collect(); + floating_pictures::forget_closed(ctx, &open); if self.fonts_han.is_some_and(|han| han != theme::HanOrder::of(self.ui.language)) { self.fonts_ready = false; self.fonts_set = false; diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 613814d03..99bbcf105 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -192,6 +192,30 @@ pub fn cell_family(name: &str, bold: bool, italic: bool) -> FontFamily { ) } +/// Width in points, at 100% zoom, of one line of cell text drawn as the grid draws it: in the +/// face [`cell_family`] picks, which on a machine without the named font is a wider or narrower +/// substitute (#1316). A family the active font set doesn't have yet (bound fonts apply from the +/// next frame) falls back to the engine's estimate rather than letting egui panic on it. +/// +/// `exact` lays the text out (a galley kept until the end of the frame); otherwise the glyph +/// advances are summed without kerning, which is cheap and never narrower. This takes the +/// Context lock (`fonts_mut`): never call it, or run a command, while an egui lock is held. +pub fn cell_text_width(ctx: &egui::Context, text: &str, font: &gridcraft_engine::model::Font, exact: bool) -> f32 { + let family = cell_family(&font.name, font.bold, font.italic); + let id = FontId::new(font.size * 96.0 / 72.0, family); + ctx.fonts_mut(|f| { + f.families().contains(&id.family).then(|| { + if exact { + f.layout_no_wrap(text.to_owned(), id, Color32::PLACEHOLDER).size().x + } else { + // Half a point covers the galley's rounding of its width up to the pixel grid. + text.chars().map(|c| f.glyph_width(&id, c)).sum::() + 0.5 + } + }) + }) + .unwrap_or_else(|| gridcraft_engine::cmd::format::approx_text_width(text, font.size, font.bold)) +} + /// Whether epaint can open face `index` of this font file and it maps characters to glyphs. /// epaint panics (in `Context::set_fonts`, i.e. on startup) on a face it can't open, so every font /// file we did not ship is checked first with the same parser and the same call epaint makes diff --git a/crates/ui-egui/tests/floating_pictures.rs b/crates/ui-egui/tests/floating_pictures.rs new file mode 100644 index 000000000..f14d40d85 --- /dev/null +++ b/crates/ui-egui/tests/floating_pictures.rs @@ -0,0 +1,68 @@ +//! Floating pictures on the sheet, with original synthetic pictures. + +use std::io::Cursor; + +use egui::{Shape, TextureId, vec2}; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +fn png(color: [u8; 4]) -> Vec { + let image = image::RgbaImage::from_pixel(120, 60, image::Rgba(color)); + let mut bytes = Cursor::new(Vec::new()); + image.write_to(&mut bytes, image::ImageFormat::Png).unwrap(); + bytes.into_inner() +} + +fn harness() -> egui_kittest::Harness<'static, SheetApp> { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn picture_textures(h: &egui_kittest::Harness<'static, SheetApp>) -> Vec { + h.output() + .shapes + .iter() + .filter_map(|shape| match &shape.shape { + Shape::Mesh(mesh) if mesh.texture_id != TextureId::default() => Some(mesh.texture_id), + _ => None, + }) + .collect() +} + +/// Picture ids are per workbook, so two open workbooks both have a picture 1. Each must show its +/// own picture, even when the two are the same number of bytes: the texture cache used to tell +/// pictures apart by id and size alone, for the whole app. +#[test] +fn each_workbook_shows_its_own_picture() { + let (green, red) = (png([24, 150, 110, 255]), png([200, 40, 40, 255])); + assert_eq!(green.len(), red.len()); + let mut h = harness(); + h.state_mut().run("insert.picture", json!({"base64": gridcraft_engine::io::base64_encode(&green), "at": "B2"})).unwrap(); + h.run_steps(4); + let first = picture_textures(&h); + assert_eq!(first.len(), 1, "the picture is painted"); + + h.state_mut().run("file.new", json!({})).unwrap(); + h.state_mut().run("insert.picture", json!({"base64": gridcraft_engine::io::base64_encode(&red), "at": "B2"})).unwrap(); + h.run_steps(4); + let doc = h.state().session.active().unwrap(); + assert_eq!(doc.wb.active().unwrap().images[0].id, h.state().session.documents()[0].wb.active().unwrap().images[0].id); + let second = picture_textures(&h); + assert_eq!(second.len(), 1); + assert_ne!(second, first, "another workbook's picture 1 has a texture of its own"); + + // Closing the workbook and painting again leaves the first one's picture as it was. + h.state_mut().run("file.close", json!({"force": true})).unwrap(); + h.run_steps(4); + assert_eq!(picture_textures(&h), first); +} diff --git a/crates/ui-egui/tests/header_autofit.rs b/crates/ui-egui/tests/header_autofit.rs index 5e123809f..e5cf0cc21 100644 --- a/crates/ui-egui/tests/header_autofit.rs +++ b/crates/ui-egui/tests/header_autofit.rs @@ -191,3 +191,51 @@ fn table_filter_autofit_works_through_command_and_header_double_click() { } } } + +#[test] +fn header_double_click_fits_the_text_as_the_grid_draws_it() { + // Issue #1316: `@` and `%` are far wider in every real font than the engine's average-glyph + // estimate, so AutoFit has to measure with the font the grid draws the cell in. + let text = "sarmstrong@example.com %%% @@@"; + let mut h = harness("A1"); + h.state_mut().session.execute("range.setValues", json!({"range":"A1", "values":[[text]]})).unwrap(); + // Let the installed-font scan finish and bind its faces first, so the cell's font can't + // change between the AutoFit and the measurement below. + gridcraft_ui_egui::fonts::catalog(); + h.run_steps(6); + let (pos, _) = edge(&h, false, 0); + button(&mut h, pos, true); + button(&mut h, pos, false); + h.step(); + button(&mut h, pos, true); + button(&mut h, pos, false); + h.run_steps(2); + + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let font = &app.session.active().unwrap().wb.styles.get(sheet.style_id(gridcraft_engine::core::CellRef::new(0, 0))).font; + let id = egui::FontId::new(font.size * 96.0 / 72.0, gridcraft_ui_egui::theme::cell_family(&font.name, font.bold, font.italic)); + let drawn = h.ctx.fonts_mut(|f| f.layout_no_wrap(text.to_string(), id, egui::Color32::BLACK)).size().x; + let estimate = gridcraft_engine::cmd::format::approx_text_width(text, font.size, font.bold); + assert!(drawn > estimate + 6.0, "fixture: the drawn text ({drawn}) must be wider than the estimate ({estimate}) plus padding"); + // Two 2.5 px paddings and the gridline, as the grid lays out the cell. + assert!(sheet.col_width(0) >= drawn + 6.0, "column {} is narrower than the drawn text {drawn} + padding", sheet.col_width(0)); +} + +#[test] +fn the_quick_measure_is_never_narrower_than_the_drawn_text() { + // Columns with many distinct values are measured by summing glyph advances; that must cover + // the laid-out text, or AutoFit would clip again on large columns. + let h = harness("A1"); + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let mut font = app.session.active().unwrap().wb.styles.get(sheet.style_id(gridcraft_engine::core::CellRef::new(0, 0))).font.clone(); + for bold in [false, true] { + font.bold = bold; + for text in ["sarmstrong@example.com", "Quarterly revenue summary", "WWW MMM Wide Headers", "AVATAR To Ty", "fi ffl 1,234.56"] { + let quick = gridcraft_ui_egui::theme::cell_text_width(&h.ctx, text, &font, false); + let exact = gridcraft_ui_egui::theme::cell_text_width(&h.ctx, text, &font, true); + assert!(exact > 0.0 && quick >= exact, "{text:?} bold={bold}: quick {quick} < drawn {exact}"); + } + } +} diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index db957608b..a08d37969 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -810,7 +810,7 @@ fn column_autocomplete_can_be_turned_off() { /// An .xlsm carrying a VBA project, saved to bytes. fn macro_workbook_bytes() -> Vec { let mut wb = gridcraft_engine::model::Workbook::new(); - wb.vba_project = Some(b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1 fake OLE2 vbaProject".to_vec()); + wb.vba_project = Some(b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1 fake OLE2 vbaProject".to_vec().into()); gridcraft_engine::io::save_bytes(&wb, "macros.xlsm").unwrap() } @@ -858,7 +858,7 @@ fn plain_open_warning_becomes_an_info_notice() { fn macros_dialog_lists_vba_and_scripts() { let mut s = blank(); let mut wb = gridcraft_engine::model::Workbook::new(); - wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]); + wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1].into()); s.active_mut().unwrap().wb = std::sync::Arc::new(wb); let mut h = harness(s); h.state_mut().open_dialog("macros", json!({})); @@ -874,7 +874,7 @@ fn macros_dialog_lists_vba_and_scripts() { fn status_bar_shows_macro_badge() { let mut s = blank(); let mut wb = gridcraft_engine::model::Workbook::new(); - wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]); + wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1].into()); s.active_mut().unwrap().wb = std::sync::Arc::new(wb); // Rendering the status bar must not panic with a VBA project present. let mut h = harness(s); diff --git a/crates/ui-egui/tests/keyboard_shortcuts.rs b/crates/ui-egui/tests/keyboard_shortcuts.rs index ea593611d..b57a4bec5 100644 --- a/crates/ui-egui/tests/keyboard_shortcuts.rs +++ b/crates/ui-egui/tests/keyboard_shortcuts.rs @@ -248,3 +248,17 @@ fn ctrl_q_does_not_close_the_window_outside_macos() { h.run_steps(2); assert!(h.ctx.options(|o| !o.quit_shortcuts.is_empty()), "Cmd+Q is still egui's quit shortcut on macOS"); } + +#[test] +fn ctrl_6_is_show_objects_and_nothing_else() { + let show = Some(Shortcut::Run("view.showObjects", json!({}))); + let mac_control = Modifiers::CTRL; + assert_eq!(lookup(Key::Num6, Some(Key::Num6), CTRL, false), show); + assert_eq!(lookup(Key::Num6, Some(Key::Num6), mac_control, true), show); + // Its neighbours keep theirs: Ctrl+Shift+6 (Ctrl+^) is Scientific, Cmd+6 on a Mac is free. + assert_eq!(lookup(Key::Num6, Some(Key::Num6), CTRL_SHIFT, false), Some(Shortcut::Run("home.numberFormat", json!({"code": "0.00E+00"})))); + assert_eq!(lookup(Key::Num6, Some(Key::Num6), Modifiers::MAC_CMD, true), None); + // No other command claims Ctrl+6. + let claims: Vec<&str> = Session::new().commands().iter().filter(|c| c.shortcut == Some("Ctrl+6")).map(|c| c.id).collect(); + assert_eq!(claims, ["view.showObjects"]); +} diff --git a/crates/ui-egui/tests/selection_glide.rs b/crates/ui-egui/tests/selection_glide.rs index fd8bf4e73..c8c253ac8 100644 --- a/crates/ui-egui/tests/selection_glide.rs +++ b/crates/ui-egui/tests/selection_glide.rs @@ -201,3 +201,29 @@ fn agents_find_run_and_read_the_setting_over_the_control_channel() { assert_eq!(set["result"]["animations"], json!(true)); assert!(a.ui.animations); } + +#[test] +fn a_mouse_drag_keeps_the_outline_on_its_cells() { + // A gliding outline trails the pointer while selecting with the mouse and reads as lag; Excel + // draws it on the selection at once. Keyboard moves still glide (above). + let mut h = animated(); + let (b2, e8) = (cell_rect(&h, "B2"), cell_rect(&h, "E8")); + let m = egui::Modifiers::NONE; + h.input_mut().events.extend([ + egui::Event::PointerMoved(b2.center()), + egui::Event::PointerButton { pos: b2.center(), button: egui::PointerButton::Primary, pressed: true, modifiers: m }, + ]); + h.step(); + for k in 1..=6 { + let p = b2.center() + (e8.center() - b2.center()) * (k as f32 / 6.0); + h.input_mut().events.push(egui::Event::PointerMoved(p)); + steps(&mut h, 2); + let sel = h.state().session.active().map(|d| d.selection.current()).unwrap(); + let shown = cell_rect(&h, &sel.start.a1()).union(cell_rect(&h, &sel.end.a1())); + assert!(!gliding(&h), "no glide while the button is held"); + assert_eq!(outline(&h), shown, "the outline is on the dragged range {}", sel.a1()); + } + h.input_mut().events.push(egui::Event::PointerButton { pos: e8.center(), button: egui::PointerButton::Primary, pressed: false, modifiers: m }); + h.step(); + assert_eq!(h.state().session.active().map(|d| d.selection.current().a1()).as_deref(), Some("B2:E8")); +} diff --git a/crates/ui-egui/tests/show_objects.rs b/crates/ui-egui/tests/show_objects.rs new file mode 100644 index 000000000..9dcac7ce8 --- /dev/null +++ b/crates/ui-egui/tests/show_objects.rs @@ -0,0 +1,96 @@ +//! Ctrl+6 hides and shows every object (Options › Advanced › "For objects, show"), through real input. + +use egui::{Color32, Event, Key, Modifiers, PointerButton, epaint::Shape, vec2}; +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_engine::model::ShowObjects; +use gridcraft_ui_egui::{SheetApp, grid::Geo}; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +const FILL: Color32 = Color32::from_rgb(0x12, 0x34, 0x56); +/// Ctrl on Windows and Linux sets both `ctrl` and `command`. +const CTRL: Modifiers = Modifiers { alt: false, ctrl: true, shift: false, mac_cmd: false, command: true }; + +/// A rectangle at D3, 144 × 96 points, filled with a colour nothing else on screen uses. +fn fixture() -> (Harness, u32) { + let mut session = Session::new(); + session.new_workbook(); + let shape = session.execute("insert.shape", json!({"kind": "rectangle", "at": "D3", "fill": "123456"})).unwrap(); + let id = shape["shape"].as_u64().unwrap() as u32; + session.execute("selection.set", json!({"range": "A1"})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + (h, id) +} + +fn ctrl_6(h: &mut Harness) { + h.input_mut().events.push(Event::Key { key: Key::Num6, physical_key: Some(Key::Num6), pressed: true, repeat: false, modifiers: CTRL }); + h.input_mut().events.push(Event::Key { key: Key::Num6, physical_key: Some(Key::Num6), pressed: false, repeat: false, modifiers: CTRL }); + h.run_steps(2); +} + +fn show_objects(h: &Harness) -> ShowObjects { + h.state().session.active().unwrap().wb.show_objects +} + +/// Whether the last frame painted the rectangle's fill. +fn painted(h: &Harness) -> bool { + h.output().shapes.iter().any(|c| matches!(&c.shape, Shape::Rect(r) if r.fill == FILL)) +} + +/// Inside the rectangle, in the cell E4 (D3 is 64 points wide and 20 high at 100%). +fn inside(h: &Harness) -> egui::Pos2 { + let app = h.state(); + let sh = app.session.active().unwrap().wb.active().unwrap(); + let geo = Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll); + geo.cell_rect(sh, CellRef::parse("E4").unwrap()).center() +} + +fn click(h: &mut Harness, pos: egui::Pos2) { + h.input_mut().events.push(Event::PointerMoved(pos)); + h.run_steps(2); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(2); + } +} + +fn active_cell(h: &Harness) -> CellRef { + h.state().session.active().unwrap().selection.active +} + +#[test] +fn ctrl_6_hides_objects_and_a_second_ctrl_6_shows_them() { + let (mut h, _) = fixture(); + assert_eq!(show_objects(&h), ShowObjects::All); + assert!(painted(&h), "the rectangle is drawn to begin with"); + ctrl_6(&mut h); + assert_eq!(show_objects(&h), ShowObjects::None); + assert!(!painted(&h), "hidden objects are not drawn"); + ctrl_6(&mut h); + assert_eq!(show_objects(&h), ShowObjects::All); + assert!(painted(&h), "a second Ctrl+6 draws them again"); +} + +#[test] +fn hidden_objects_do_not_take_clicks() { + let (mut h, id) = fixture(); + let at = inside(&h); + click(&mut h, at); + assert_eq!(h.state().selected_chart, Some(id), "the click lands on the rectangle while it shows"); + assert_eq!(active_cell(&h), CellRef::parse("A1").unwrap()); + ctrl_6(&mut h); + assert_eq!(h.state().selected_chart, None, "hiding objects drops the selection on one"); + click(&mut h, at); + assert_eq!(h.state().selected_chart, None); + assert_eq!(active_cell(&h), CellRef::parse("E4").unwrap(), "the click selects the cell under the hidden rectangle"); +} diff --git a/crates/xlsx/src/drawing.rs b/crates/xlsx/src/drawing.rs index a6892f0b8..942cf4a53 100644 --- a/crates/xlsx/src/drawing.rs +++ b/crates/xlsx/src/drawing.rs @@ -156,7 +156,7 @@ pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<( }; let alt = obj.path(&["nvPicPr", "cNvPr"]).and_then(|c| c.attr("descr")).unwrap_or("").to_string(); let id = cx.next_object_id(); - sheet.images.push(Image { id, anchor, data, mime: mime_for(&target).to_string(), alt }); + sheet.images.push(Image { id, anchor, data: data.into(), mime: mime_for(&target).to_string(), alt }); } "graphicFrame" => { let Some(gd) = obj.path(&["graphic", "graphicData"]) else { continue }; diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index ee93c1b77..8ca486ff1 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use gridcraft_core::{DateSystem, RangeRef}; -use gridcraft_model::{CalcMode, CellPicture, DefinedName, Sheet, Style, StyleId, StyleTable, Visibility, Workbook}; +use gridcraft_model::{CalcMode, CellPicture, DefinedName, PhoneticString, Sheet, ShowObjects, Style, StyleId, StyleTable, Visibility, Workbook}; use crate::package::{Package, Rel}; use crate::styles::{StylesIn, read_styles}; @@ -15,6 +15,8 @@ pub struct Ctx<'a> { pub pkg: Package<'a>, pub report: ReadReport, pub sst: Vec>, + /// Phonetic (furigana) data of the shared strings, one entry per base text. + pub phonetics: Vec, /// Cell format index → model style. pub xf_map: Vec, pub dxfs: Vec