diff --git a/ROADMAP.md b/ROADMAP.md index 90aabd4..7f2b698 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,11 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 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) | +| 2026-10-12 | Security: cell hyperlinks open only http, https and mailto, after a confirmation; other schemes (file:, smb:, UNC, custom handlers) are refused (#777, part 1) | +| 2026-10-12 | Deleting several Ctrl-selected rows or columns removes them all in one undo step (#758) | +| 2026-10-12 | XLSX chart sheets are kept on open and save: the sheet, its drawing and chart (with style and colours) are written back unchanged in their place among the worksheets, and left out when the sheet they plot is deleted or renamed or a worksheet took their name (#727) | | 2026-10-12 | CSV and tab-separated export is capped at 512 MiB: a sheet whose used range would expand to more (two cells far apart, such as A1 and XFD1048576, are 16 GiB of commas) is refused with the rows, columns and limit, before anything is allocated or written (#528) | | 2026-10-12 | PDF writer hardening: text width cannot overflow, all-zero dash arrays and negative or NaN stroke widths are sanitised, non-finite sizes and positions draw nothing (#812) | | 2026-10-12 | PDF export writes the document title and author as UTF-16BE when they are not ASCII, so viewers show accents, currency signs and non-Latin text correctly (#813) | @@ -187,6 +192,8 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 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) | | 2026-10-12 | trivial | Progress log: CSV export size cap (#528) | | 2026-10-12 | minor | Progress log: margins dragged on the Page Layout rulers (#792) | | 2026-10-12 | minor | Ink preservation across XLSX save/reopen (ink portion of #551) | diff --git a/apps/gridcraft-cli/src/main.rs b/apps/gridcraft-cli/src/main.rs index d72bbd9..152ddbc 100644 --- a/apps/gridcraft-cli/src/main.rs +++ b/apps/gridcraft-cli/src/main.rs @@ -277,7 +277,7 @@ fn convert(args: &[String]) -> Result<(), String> { let mut s = open(input)?; activate_sheet(&mut s, a.opt("sheet"))?; let d = s.doc().map_err(|e| e.to_string())?; - let bytes = gridcraft_engine::io::save_bytes(&d.wb, out).map_err(|e| e.to_string())?; + let bytes = gridcraft_engine::io::save_file_bytes(&d.wb, out).map_err(|e| e.to_string())?; gridcraft_engine::io::write_file(out, &bytes).map_err(|e| e.to_string())?; eprintln!("wrote {out} ({} bytes)", bytes.len()); Ok(()) diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index 3856aaf..c0fcb22 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -170,8 +170,10 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services { } })), open_url: Some(Box::new(|url: &str| { - if let Some(w) = web_sys::window() { - let _ = w.open_with_url_and_target(url, "_blank"); + if let Some(w) = web_sys::window() + && let gridcraft_engine::links::LinkKind::External(url) = gridcraft_engine::links::classify(url) + { + let _ = w.open_with_url_and_target(&url, "_blank"); } })), inbox: Some(inbox), diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 08737a3..7c0eb04 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -201,6 +201,13 @@ fn services() -> Services { /// Opens a URL or file with the system's default application. fn open_with_system(target: &str) { + // Last line of defence behind `link.open` and the click prompt: the system only ever gets + // http, https and mailto addresses. + let gridcraft_engine::links::LinkKind::External(target) = gridcraft_engine::links::classify(target) else { + log::warn!("refused to open {target:?}: only http, https and mailto links are opened"); + return; + }; + let target = target.as_str(); let cmd = if cfg!(target_os = "macos") { "open" } else if cfg!(windows) { diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 47a5119..0f9cfb0 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -407,6 +407,7 @@ impl<'h> Evaluator<'h> { }; } let v = self.value(x); + let sys = self.host.workbook().date_system; Ev::V(map_unary( &v, &|n| match op { @@ -416,6 +417,7 @@ impl<'h> Evaluator<'h> { UnOp::At | UnOp::Spill | UnOp::Trim(_) => Value::number(n), }, op == UnOp::Plus, + sys, )) } @@ -502,7 +504,7 @@ impl<'h> Evaluator<'h> { let mut acc = self.value(left); while let Some((o, r)) = rest.pop() { let y = self.value(r); - acc = binary_values(o, &acc, &y); + acc = binary_values(o, &acc, &y, self.host.workbook().date_system); } Ev::V(acc) } @@ -1165,16 +1167,18 @@ impl<'h> Evaluator<'h> { v.scalar().to_bool().unwrap_or(true) } }; - let text = if a1 { - text + // With a1 = FALSE, text that isn't R1C1 can still be a name, but never an A1 reference. + let (text, a1_ok) = if a1 { + (text, true) } else { match r1c1_to_a1(&text, self.at) { - Ok(Some(a1_text)) => a1_text, - Ok(None) => text, + Ok(Some(a1_text)) => (a1_text, true), + Ok(None) => (text, false), Err(e) => return err(e), } }; match gridcraft_formula::parse(&text) { + Ok(Expr::Ref(_)) if !a1_ok => err(CellError::Ref), Ok(e @ (Expr::Ref(_) | Expr::Name(_) | Expr::Struct(_))) => match self.eval(&e) { r @ Ev::R(_) => r, _ => err(CellError::Ref), @@ -1593,12 +1597,12 @@ impl Acc { // ---------------------------------------------------------------- operators -fn map_unary(v: &Value, f: &dyn Fn(f64) -> Value, plus: bool) -> Value { +fn map_unary(v: &Value, f: &dyn Fn(f64) -> Value, plus: bool, sys: gridcraft_core::DateSystem) -> Value { match v { - Value::Array(a) => Value::from(Array { rows: a.rows, cols: a.cols, data: a.data.iter().map(|x| map_unary(x, f, plus)).collect() }), + Value::Array(a) => Value::from(Array { rows: a.rows, cols: a.cols, data: a.data.iter().map(|x| map_unary(x, f, plus, sys)).collect() }), Value::Error(e) => Value::Error(*e), Value::Text(_) if plus => v.clone(), - other => match other.to_number() { + other => match other.to_number_in(sys) { Ok(n) => f(n), Err(e) => Value::Error(e), }, @@ -1651,18 +1655,18 @@ fn zip3(c: &Array, t: &Value, f: &Value, g: impl Fn(&Value, &Value, &Value) -> V Value::from(Array { rows, cols, data }) } -pub fn binary_values(op: BinOp, x: &Value, y: &Value) -> Value { - zip2(x, y, |a, b| binary_scalar(op, a, b)) +pub fn binary_values(op: BinOp, x: &Value, y: &Value, sys: gridcraft_core::DateSystem) -> Value { + zip2(x, y, |a, b| binary_scalar(op, a, b, sys)) } -fn binary_scalar(op: BinOp, a: &Value, b: &Value) -> Value { +fn binary_scalar(op: BinOp, a: &Value, b: &Value, sys: gridcraft_core::DateSystem) -> Value { if let Value::Error(e) = a { return Value::Error(*e); } if let Value::Error(e) = b { return Value::Error(*e); } - let num = |f: fn(f64, f64) -> Value| match (a.to_number(), b.to_number()) { + let num = |f: fn(f64, f64) -> Value| match (a.to_number_in(sys), b.to_number_in(sys)) { (Ok(x), Ok(y)) => f(x, y), (Err(e), _) | (_, Err(e)) => Value::Error(e), }; diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index a0c9fe2..bacdb06 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -290,6 +290,26 @@ fn circular_references_without_iterative_calculation() { assert_eq!(t.get("A1"), Value::Error(CellError::Circ)); } +#[test] +fn date_text_coerces_in_the_workbook_date_system() { + for (sys, serial) in [(gridcraft_core::DateSystem::D1900, 1462.0), (gridcraft_core::DateSystem::D1904, 0.0)] { + let mut t = T::new(); + t.wb.date_system = sys; + t.set("A1", "'1/1/1904"); + t.set("B1", "=A1+0"); + t.set("C1", "=VALUE(A1)"); + t.set("D1", "=-A1"); + t.set("E1", "=DATEVALUE(A1)"); + t.set("F1", "=NUMBERVALUE(A1)"); + assert_eq!(t.get("A1"), Value::from("1/1/1904")); + assert_eq!(t.num("B1"), serial, "{sys:?}"); + assert_eq!(t.num("C1"), serial, "{sys:?}"); + assert_eq!(t.num("D1"), -serial, "{sys:?}"); + assert_eq!(t.num("E1"), serial, "{sys:?}"); + assert_eq!(t.num("F1"), serial, "{sys:?}"); + } +} + #[test] fn operators() { let mut t = T::new(); @@ -423,6 +443,22 @@ fn references_functions() { assert_eq!(t.num("B3"), 50.0); t.set("B4", "=INDIRECT(\"A\"&4)"); assert_eq!(t.num("B4"), 40.0); + // With a1 = FALSE the text must be R1C1: A1-style text is #REF!, not read as A1. + t.set("C1", "=INDIRECT(\"R3C1\",FALSE)"); + assert_eq!(t.num("C1"), 30.0); + t.set("C2", "=INDIRECT(\"A3\",FALSE)"); + assert_eq!(t.get("C2"), Value::Error(CellError::Ref)); + // Defined names don't depend on the reference style. + t.wb.names.push(gridcraft_model::DefinedName { + name: "Third".into(), + scope: None, + formula: "Sheet1!$A$3".into(), + comment: String::new(), + hidden: false, + }); + t.calc.rebuild(&t.wb); + t.set("C3", "=INDIRECT(\"Third\",FALSE)"); + assert_eq!(t.num("C3"), 30.0); t.set("B5", "=SUM(A1:INDEX(A1:A4,3))"); assert_eq!(t.num("B5"), 60.0); t.set("B6", "=SUM(A:A)"); diff --git a/crates/core/src/parse.rs b/crates/core/src/parse.rs index a943103..b465bea 100644 --- a/crates/core/src/parse.rs +++ b/crates/core/src/parse.rs @@ -41,7 +41,12 @@ pub fn parse_input(s: &str, sys: DateSystem) -> Parsed { /// Numeric text as arithmetic coerces it ("1,000", "50%", "$3", "1/2/2020", "10:00"). pub fn parse_number_text(s: &str) -> Option { - parse_numeric(s.trim(), DateSystem::D1900).map(|(n, _)| n) + parse_number_text_in(s, DateSystem::D1900) +} + +/// Like [`parse_number_text`], with date text read in the given workbook date system. +pub fn parse_number_text_in(s: &str, sys: DateSystem) -> Option { + parse_numeric(s.trim(), sys).map(|(n, _)| n) } fn parse_numeric(t: &str, sys: DateSystem) -> Option<(f64, Option<&'static str>)> { diff --git a/crates/core/src/value.rs b/crates/core/src/value.rs index 390800f..cfef7ac 100644 --- a/crates/core/src/value.rs +++ b/crates/core/src/value.rs @@ -224,13 +224,17 @@ impl Value { /// Coercion to a number as arithmetic does it: empty = 0, TRUE = 1, numeric text parses /// (including dates, times, percentages and currency), other text = `#VALUE!`. pub fn to_number(&self) -> Result { + self.to_number_in(crate::DateSystem::D1900) + } + /// [`Value::to_number`] with date text read in the given workbook date system. + pub fn to_number_in(&self, sys: crate::DateSystem) -> Result { match self { Value::Empty => Ok(0.0), Value::Number(n) => Ok(*n), Value::Bool(b) => Ok(if *b { 1.0 } else { 0.0 }), - Value::Text(t) => crate::parse::parse_number_text(t).ok_or(CellError::Value), + Value::Text(t) => crate::parse::parse_number_text_in(t, sys).ok_or(CellError::Value), Value::Error(e) => Err(*e), - Value::Array(a) => a.data.first().map(|v| v.to_number()).unwrap_or(Ok(0.0)), + Value::Array(a) => a.data.first().map(|v| v.to_number_in(sys)).unwrap_or(Ok(0.0)), } } /// Coercion to text as `&` does it. diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 311121e..383624b 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -122,7 +122,7 @@ pub fn specs() -> Vec { fn col_param(v: Option<&Json>, base: u32) -> Option { match v? { Json::String(s) => gridcraft_core::letters_to_col(s.trim_start_matches('$')), - Json::Number(n) => n.as_u64().map(|o| base + o as u32), + Json::Number(n) => n.as_u64().and_then(|o| u32::try_from(o).ok()).and_then(|o| base.checked_add(o)), _ => None, } } @@ -500,7 +500,12 @@ fn filter_by(s: &mut Session, p: &Json) -> Result { s.doc()?.wb.sheet(sheet).and_then(|sh| sh.autofilter.clone()).ok_or_else(|| EngineError::Other("no filter".into()))? } }; - let col = col_param(p.get("column"), af.range.start.col).ok_or_else(|| bad("data.filterBy", "missing `column`"))?; + let col = match col_param(p.get("column"), af.range.start.col) { + Some(c) => c, + // A number too large to be a column is outside the filter range, not a missing parameter. + None if p.get("column").is_some_and(|v| v.as_u64().is_some()) => return Err(bad("data.filterBy", "column outside the filter range")), + None => return Err(bad("data.filterBy", "missing `column`")), + }; if col < af.range.start.col || col > af.range.end.col { return Err(bad("data.filterBy", "column outside the filter range")); } @@ -1308,6 +1313,20 @@ fn subtotal(s: &mut Session, p: &Json) -> Result { mod tests { use super::*; + #[test] + fn filter_by_huge_numeric_column_is_rejected_not_overflowed() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "B1", "values": [["Name", "Qty"], ["a", 1], ["b", 2]]})).unwrap(); + s.execute("data.filter", json!({"range": "B1:C3"})).unwrap(); + for col in [json!(2), json!(u64::from(u32::MAX)), json!(u64::MAX)] { + let err = s.execute("data.filterBy", json!({"column": col, "values": ["a"]})).unwrap_err(); + assert!(err.to_string().contains("column outside the filter range"), "{col}: {err}"); + } + let ok = s.execute("data.filterBy", json!({"column": 0, "values": ["a"]})).unwrap(); + assert_eq!(ok["hiddenRows"], json!(1)); + } + #[test] fn filter_by_inside_a_table_installs_the_autofilter_and_keeps_the_button() { for sel in [json!({"cell": "A1"}), json!({"cell": "A2"}), json!({"range": "A1:A3"})] { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 0b467e0..82dbe0b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -503,7 +503,7 @@ fn select_col(s: &mut Session, _: &Json) -> Result { Ok(json!({"selection": d.selection.a1()})) } -fn go_to(s: &mut Session, p: &Json) -> Result { +pub(super) fn go_to(s: &mut Session, p: &Json) -> Result { let Some(reference) = str_param(p, "reference").or_else(|| str_param(p, "range")) else { return Err(bad("edit.goTo", "missing `reference`")) }; // A defined name or table? let d = s.doc()?; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 2a8b435..7291ee8 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -56,7 +56,8 @@ fn open(s: &mut Session, p: &Json) -> Result { return ok(); }; let encrypted = gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Encrypted; - let (wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?; + let (mut wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?; + wb.source_dir = str_param(p, "path").and_then(crate::io::source_dir); 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); @@ -110,11 +111,13 @@ fn save(s: &mut Session, p: &Json) -> Result { } let d = s.doc_mut()?; // The document's own file keeps the format its content had when opened; any other path - // goes by its extension. + // goes by its extension. Relative paths to linked workbooks are rewritten for the new folder. let kind = crate::io::FileKind::from_path(&path).or(d.content_kind.filter(|_| d.path.as_deref() == Some(path.as_str()))); + let rebased = crate::io::source_dir(&path).and_then(|f| gridcraft_xlsx::rebase_links(&d.wb, &f)); + let wb = rebased.as_ref().unwrap_or(&d.wb); let bytes = match kind { - Some(kind) => crate::io::save_bytes_as(&d.wb, &path, kind)?, - None => crate::io::save_bytes(&d.wb, &path)?, + Some(kind) => crate::io::save_bytes_as(wb, &path, kind)?, + None => crate::io::save_bytes(wb, &path)?, }; crate::io::write_file(&path, &bytes)?; // Only a full-fidelity format clears the dirty flag and becomes the document's file. diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index e589e60..3199542 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -157,6 +157,15 @@ pub fn specs() -> Vec { has_doc, insert_link ), + cmd!( + "link.open", + "Open Hyperlink", + [], + None, + "{url?: \"https://…\" | \"mailto:…\" | \"Sheet2!A1\", cell?} (default: the cell's link); only http, https and mailto are opened, any other scheme is an error; the UI asks before opening", + has_doc, + open_link + ), cmd!("review.newNote", "New Note", ["Review", "Notes"], Some("Shift+F2"), "{cell?, text, author?}", has_doc, |s, p| comment(s, p, false)), cmd!("review.newComment", "New Comment", ["Review", "Comments"], Some("Cmd+Shift+F2"), "{cell?, text, author?}", has_doc, |s, p| comment( s, p, true @@ -1184,6 +1193,31 @@ fn insert_link(s: &mut Session, p: &Json) -> Result { }) } +/// Follows a link: a reference selects it, an `http`/`https`/`mailto` address is handed to the +/// host (`UiRequest::OpenUrl`); anything else is refused ([`crate::links::classify`]). Opens no +/// dialog; the interactive click asks for confirmation first, in the UI. +fn open_link(s: &mut Session, p: &Json) -> Result { + let target = match str_param(p, "url").or_else(|| str_param(p, "target")) { + Some(t) => t.to_string(), + None => { + let sheet = target_sheet(s, p)?; + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); + let Some(h) = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.hyperlinks.get(&at)) else { + return Err(EngineError::Other("There's no hyperlink in this cell.".into())); + }; + h.target.clone() + } + }; + match crate::links::classify(&target) { + crate::links::LinkKind::Internal(r) => super::edit::go_to(s, &json!({"reference": r.trim_start_matches('#')})), + crate::links::LinkKind::External(u) => { + s.ui_requests.push(crate::UiRequest::OpenUrl(u.clone())); + Ok(json!({"opened": u})) + } + crate::links::LinkKind::Refused(why) => Err(EngineError::Other(why)), + } +} + /// Sorts a link target as the two pages of Excel's Insert Hyperlink dialog do: "Place in This /// Document" is a sheet reference, a defined name or `#…`; anything else ("Existing File or Web /// Page") is outside the workbook and saved as a relationship. diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index cc1dc86..a10f19d 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -1027,6 +1027,21 @@ fn empty_for_overflow(sh: &Sheet, c: CellRef, merges: &[RangeRef]) -> bool { sh.value_ref(c).is_none_or(|v| v.is_empty()) && !merges.iter().any(|m| m.contains(c)) } +/// Unscaled width of the empty Center Across Selection cells right of `c`, which its text is centred over too. +fn center_across_width(styles: &StyleTable, sh: &Sheet, c: CellRef, merges: &[RangeRef]) -> f32 { + let mut ext = 0.0; + let mut col = c.col + 1; + while col < MAX_COLS && ext < 20_000.0 { + let nc = CellRef::new(c.row, col); + if !empty_for_overflow(sh, nc, merges) || styles.get(sh.style_id(nc)).align.h != HAlign::CenterAcross { + break; + } + ext += sh.col_width(col); + col += 1; + } + ext +} + fn draw_text(pg: &mut Page, b: &Block, c: CellRef, l: &Look, rect: (f32, f32, f32, f32), merged: bool, merges: &[RangeRef], limits: (f32, f32)) { let (x, y, w, h) = rect; let sh = b.sh; @@ -1110,8 +1125,11 @@ fn draw_text(pg: &mut Page, b: &Block, c: CellRef, l: &Look, rect: (f32, f32, f3 VAlign::Bottom => y + h - pad - size * 0.22 - (n - 1.0) * line_h, }; // Overflow into empty neighbours (single-line text, not merged). - let (mut clip_l, mut clip_r) = (x, x + w); - if !wrap && !merged && !is_num && rot.abs() != 90 && tw + 2.0 * pad + indent > w { + // Center Across Selection spans the empty Center Across cells to its right; the text stays within that run. + let spans = st.align.h == HAlign::CenterAcross && !wrap && !merged && rot == 0; + let across = if spans { center_across_width(&wb.styles, sh, c, merges) * b.k } else { 0.0 }; + let (mut clip_l, mut clip_r) = (x, x + w + across); + if !wrap && !merged && !is_num && rot.abs() != 90 && across <= 0.0 && tw + 2.0 * pad + indent > w { let need = tw + 2.0 * pad + indent - w; let grow = |dir: i64, mut need: f32| -> f32 { let mut ext = 0.0; @@ -1183,7 +1201,7 @@ fn draw_text(pg: &mut Page, b: &Block, c: CellRef, l: &Look, rect: (f32, f32, f3 let lw = gridcraft_pdf::text_width(font, size, line); let lx = match h_align { HAlign::Right => x + w - pad - lw - indent, - HAlign::Center => x + (w - lw) / 2.0, + HAlign::Center => x + (w + across - lw) / 2.0, _ => x + pad + indent, }; let ly = first_baseline + i as f32 * line_h; @@ -2088,6 +2106,23 @@ mod tests { assert!(w.len() > 2); } + #[test] + fn center_across_spans_following_empty_center_across_cells() { + let mut styles = StyleTable::default(); + let across = styles.intern(Style { align: Alignment { h: HAlign::CenterAcross, ..Alignment::default() }, ..Style::default() }); + let mut sh = Sheet::new("S"); + for col in 0..3 { + sh.set_style(CellRef::new(0, col), across); + } + sh.set_value(CellRef::new(0, 0), Value::Text("Title".into())); + let a1 = CellRef::new(0, 0); + assert_eq!(center_across_width(&styles, &sh, a1, &[]), sh.col_width(1) + sh.col_width(2)); + sh.set_value(CellRef::new(0, 2), Value::Text("x".into())); + assert_eq!(center_across_width(&styles, &sh, a1, &[]), sh.col_width(1)); + let merged = [RangeRef::new(CellRef::new(0, 1), CellRef::new(1, 1))]; + assert_eq!(center_across_width(&styles, &sh, a1, &merged), 0.0); + } + #[test] fn rotated_wrapped_text_prints_rotated() { // Upright headers over narrow columns (mark sheets) combine rotation with Wrap Text. diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 4820905..4dac63a 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -292,30 +292,65 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { Ok(out) } +/// The areas a line delete/insert targets as `(first line, count)`, merged and sorted from the +/// last line to the first so deleting one never shifts the next. A comma-separated list +/// (`"3:3,7:8"`) or a multi-area selection yields several; `count` forces the single-area form. +fn delete_areas(s: &Session, p: &Json, axis: Axis) -> Result> { + let key = if axis == Axis::Rows { "rows" } else { "cols" }; + let spans: Vec<(u32, u32)> = if u32_param(p, "count").is_some() { + let (at, count) = lines(s, p, axis)?; + let limit = if axis == Axis::Rows { MAX_ROWS } else { MAX_COLS }; + // Deleting past the last line only removes up to the end of the worksheet. + let count = count.min(limit.saturating_sub(at)).max(1); + vec![(at, at.saturating_add(count - 1))] + } 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}")))?; + Ok(if axis == Axis::Rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }) + }) + .collect::>()? + } else { + s.doc()?.selection.ranges.iter().map(|r| if axis == Axis::Rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }).collect() + }; + let mut spans: Vec<(u32, u32)> = spans.into_iter().map(|(a, b)| (a.min(b), a.max(b))).collect(); + spans.sort_unstable(); + let mut merged: Vec<(u32, u32)> = Vec::new(); + for (a, b) in spans { + match merged.last_mut() { + Some(last) if a <= last.1.saturating_add(1) => last.1 = last.1.max(b), + _ => merged.push((a, b)), + } + } + Ok(merged.into_iter().rev().map(|(a, b)| (a, b - a + 1)).collect()) +} + fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result { - let (at, count) = lines(s, p, axis)?; - let limit = if axis == Axis::Rows { MAX_ROWS } else { MAX_COLS }; - // Deleting past the last line only removes up to the end of the worksheet. - let count = count.min(limit.saturating_sub(at)).max(1); - check_lines_protection(s, p, axis, at, count, false)?; + let areas = delete_areas(s, p, axis)?; + for &(at, count) in &areas { + check_lines_protection(s, p, axis, at, count, false)?; + } let sheet = target_sheet(s, p)?; let out = edit(s, |cx| { let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default(); cx.protection_checked = true; - { - let sh = cx.sheet_mut(sheet)?; - let block = match axis { - Axis::Rows => RangeRef::rows(at, (at + count - 1).min(MAX_ROWS - 1)), - Axis::Cols => RangeRef::cols(at, (at + count - 1).min(MAX_COLS - 1)), - }; - sh.take_cells(block); - match axis { - Axis::Rows => sh.cells.shift_rows(at + count, -(count as i64)), - Axis::Cols => sh.cells.shift_cols(at + count, -(count as i64)), + // Last area first, so the earlier ones keep their positions. + for (at, count) in areas { + { + let sh = cx.sheet_mut(sheet)?; + let block = match axis { + Axis::Rows => RangeRef::rows(at, (at + count - 1).min(MAX_ROWS - 1)), + Axis::Cols => RangeRef::cols(at, (at + count - 1).min(MAX_COLS - 1)), + }; + sh.take_cells(block); + match axis { + Axis::Rows => sh.cells.shift_rows(at + count, -(count as i64)), + Axis::Cols => sh.cells.shift_cols(at + count, -(count as i64)), + } + shift_sheet_features(sh, axis, at, count, false); } - shift_sheet_features(sh, axis, at, count, false); + rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count }); } - rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count }); cx.structural = true; Ok(Json::Null) })?; diff --git a/crates/engine/src/display.rs b/crates/engine/src/display.rs index db7e7c8..b90e427 100644 --- a/crates/engine/src/display.rs +++ b/crates/engine/src/display.rs @@ -10,11 +10,13 @@ use gridcraft_numfmt::{Formatted, NumberFormat, format_value}; /// Parsed number formats are cached by code. fn parsed(code: &str) -> NumberFormat { static CACHE: std::sync::LazyLock>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new())); - let mut m = CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if let Some(f) = m.get(code) { + let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(f) = lock().get(code) { return f.clone(); } + // Parse outside the lock, so a slow code never blocks the other threads' lookups. let f = NumberFormat::parse(code); + let mut m = lock(); if m.len() > 4096 { m.clear(); } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index e8a17b7..52aafae 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -147,7 +147,9 @@ fn rename_first_sheet(wb: &mut Workbook, path: &str) { let stem = std::path::Path::new(path).file_stem().and_then(|s| s.to_str()).unwrap_or("Sheet1"); let clean: String = stem.chars().filter(|c| !matches!(c, ':' | '\\' | '/' | '?' | '*' | '[' | ']')).take(31).collect(); let clean = clean.trim_matches('\'').to_string(); + // "History" is reserved (see `Workbook::check_sheet_name`): keep the default name instead. if !clean.is_empty() + && !clean.eq_ignore_ascii_case("History") && let Some(sh) = wb.sheet_mut(0) { sh.name = clean; @@ -185,6 +187,20 @@ pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result Result> { + match source_dir(path).and_then(|f| gridcraft_xlsx::rebase_links(wb, &f)) { + Some(rebased) => save_bytes(&rebased, path), + None => save_bytes(wb, path), + } +} + +/// The folder of the file at `path`, which relative paths to linked workbooks are relative to. +pub fn source_dir(path: &str) -> Option { + std::path::absolute(path).ok()?.parent().map(std::path::Path::to_path_buf) +} + /// 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 `#####`. @@ -487,6 +503,17 @@ mod tests { assert!(wb.vba_project.is_some(), "the open workbook keeps its macros"); } + #[test] + fn csv_named_history_does_not_get_the_reserved_sheet_name() { + for file in ["History.csv", "hIsToRy.csv", "history.tsv"] { + let (wb, _) = open_bytes(file, b"1\n").unwrap(); + let name = wb.sheets[0].name.clone(); + assert!(wb.check_sheet_name(&name, Some(0)).is_ok(), "{file} opened as {name:?}"); + } + let (wb, _) = open_bytes("Sales.csv", b"1\n").unwrap(); + assert_eq!(wb.sheets[0].name, "Sales"); + } + /// A save that can't be put in place (here a folder already has the workbook's name, as /// with a file another app holds locked) fails without leaving `.sctmp` behind. #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 5a2e3df..258a1cb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -13,6 +13,7 @@ pub mod cmd; pub mod display; pub mod fill; pub mod io; +pub mod links; pub mod outline; pub mod paper; pub mod pivot; diff --git a/crates/engine/src/links.rs b/crates/engine/src/links.rs new file mode 100644 index 0000000..04d8da6 --- /dev/null +++ b/crates/engine/src/links.rs @@ -0,0 +1,83 @@ +//! Which hyperlink targets may be opened. A cell's link comes from the document (often a file +//! somebody sent), so it is hostile input: only `http`, `https` and `mailto` ever leave the +//! workbook for the system; `file:`, `smb:`, UNC paths, custom URL handlers, `javascript:` and +//! the like are refused. Everything without a scheme stays an in-workbook reference. + +use gridcraft_core::RangeRef; + +/// What a link target is. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum LinkKind { + /// A cell, range, name or table of the workbook (`Sheet2!A1`, `Total`). + Internal(String), + /// An address to hand to the system, trimmed: `http`, `https` or `mailto` only. + External(String), + /// Refused, with the reason to show. + Refused(String), +} + +const ALLOWED: [&str; 3] = ["http", "https", "mailto"]; + +/// Classifies a link target. Never panics; the single place that decides what may be opened. +pub fn classify(target: &str) -> LinkKind { + let t = target.trim(); + if t.chars().any(char::is_control) { + return LinkKind::Refused("The link contains control characters.".into()); + } + if t.starts_with("\\\\") || t.starts_with("//") { + return LinkKind::Refused("Links to network locations aren't opened.".into()); + } + if RangeRef::parse(t).is_some() { + return LinkKind::Internal(t.to_string()); + } + let Some((scheme, rest)) = t.split_once(':') else { return LinkKind::Internal(t.to_string()) }; + let scheme_like = scheme.chars().next().is_some_and(|c| c.is_ascii_alphabetic()) + && scheme.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')); + if !scheme_like { + return LinkKind::Internal(t.to_string()); + } + let lower = scheme.to_ascii_lowercase(); + if ALLOWED.contains(&lower.as_str()) && !rest.trim().is_empty() { + LinkKind::External(t.to_string()) + } else { + LinkKind::Refused(format!("Links of type \"{lower}:\" aren't opened. Only http, https and mailto links are allowed.")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_http_https_and_mailto_leave_the_workbook() { + for ok in ["http://a.example/x", "HTTPS://A.example", " https://a.example ", "MailTo:a@b.example"] { + assert!(matches!(classify(ok), LinkKind::External(_)), "{ok}"); + } + assert_eq!(classify(" https://a.example "), LinkKind::External("https://a.example".into())); + for bad in [ + "javascript:alert(1)", + "JaVaScRiPt:alert(1)", + "file:///etc/passwd", + "smb://host/share", + "ms-excel:ofe|u|http://x", + "\\\\host\\share\\x.exe", + "//host/share", + "C:\\Windows\\calc.exe", + "\tjavascript:1", + "java\nscript:1", + "\u{1}http://a.example", + "http:", + "mailto:", + "x-custom://a", + ] { + assert!(matches!(classify(bad), LinkKind::Refused(_)), "{bad:?} -> {:?}", classify(bad)); + } + } + + #[test] + fn references_stay_internal() { + for r in ["Sheet2!A1", "'My Sheet'!B2:C3", "A1:B2", "#Sheet1!A1", "Total", "", " "] { + assert!(matches!(classify(r), LinkKind::Internal(_)), "{r:?} -> {:?}", classify(r)); + } + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0225b83..6f4b67a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -560,6 +560,28 @@ fn insert_delete_rows_adjust() { assert_eq!(v(&s, "A1"), Value::Number(10.0)); } +#[test] +fn deleting_a_multi_area_row_selection_removes_every_area_and_undoes_once() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3], [4], [5], [6]]})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": "=A6"})).unwrap(); + // Ctrl-select rows 2, 4 and 5:6 (A1 refs are 1-based). + let sel = &mut s.doc_mut().unwrap().selection; + sel.ranges = vec![gridcraft_core::RangeRef::rows(1, 1), gridcraft_core::RangeRef::rows(3, 3), gridcraft_core::RangeRef::rows(4, 5)]; + sel.active = CellRef::new(4, 0); + s.doc_mut().unwrap().undo.clear(); + s.execute("home.deleteRows", json!({})).unwrap(); + assert_eq!((v(&s, "A1"), v(&s, "A2"), v(&s, "A3")), (Value::Number(1.0), Value::Number(3.0), Value::Empty)); + assert_eq!(s.execute("cell.get", json!({"cell": "B1"})).unwrap()["formula"], "=#REF!"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!((1..=6).map(|r| v(&s, &format!("A{r}"))).collect::>(), (1..=6).map(|n| Value::Number(n as f64)).collect::>()); + assert!(s.doc().unwrap().undo.is_empty(), "one undo step restored all rows"); + // The explicit list form behaves the same, for columns too. + s.execute("range.setValues", json!({"range": "A1", "values": [[1, 2, 3, 4]]})).unwrap(); + s.execute("home.deleteColumns", json!({"cols": "A:A,C:C"})).unwrap(); + assert_eq!((v(&s, "A1"), v(&s, "B1"), v(&s, "C1")), (Value::Number(2.0), Value::Number(4.0), Value::Empty)); +} + #[test] fn move_columns_reorders_without_overwriting_and_undoes_once() { let mut s = s(); @@ -1620,6 +1642,82 @@ fn saving_an_opened_semicolon_csv_keeps_semicolons() { std::fs::remove_dir_all(dir).unwrap(); } +/// A workbook whose B1 is `=[1]Sheet1!A1`, linked to `link` (an external relationship target). +fn linked_workbook(link: &str) -> Vec { + use std::io::{Cursor, Write}; + const NS_REL: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + let types = r#""#.to_string(); + let rels = format!( + r#""# + ); + let book = format!( + r#""# + ); + let book_rels = format!( + r#""# + ); + let sheet = r#"[1]Sheet1!A110"#.to_string(); + let link_part = format!( + r#"10"# + ); + let link_rels = format!( + r#""# + ); + let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); + for (name, data) in [ + ("[Content_Types].xml", types), + ("_rels/.rels", rels), + ("xl/workbook.xml", book), + ("xl/_rels/workbook.xml.rels", book_rels), + ("xl/worksheets/sheet1.xml", sheet), + ("xl/externalLinks/externalLink1.xml", link_part), + ("xl/externalLinks/_rels/externalLink1.xml.rels", link_rels), + ] { + zip.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); + zip.write_all(data.as_bytes()).unwrap(); + } + zip.finish().unwrap().into_inner() +} + +/// The target of the linked workbook in a saved file. +fn link_target(path: &std::path::Path) -> String { + use std::io::Read; + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(std::fs::read(path).unwrap())).unwrap(); + let mut rels = String::new(); + zip.by_name("xl/externalLinks/_rels/externalLink1.xml.rels").unwrap().read_to_string(&mut rels).unwrap(); + let (_, after) = rels.split_once("Target=\"").unwrap(); + after.split_once('"').unwrap().0.to_string() +} + +/// Save As into another folder keeps the link on the same file, as Excel does: the relative path +/// is rewritten from the new folder (#801). +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn save_as_into_another_folder_keeps_linked_workbooks_on_the_same_file() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let a = std::env::temp_dir().join(format!("gridcraft-links-{}-{nonce}", std::process::id())); + let b = a.join("b"); + std::fs::create_dir_all(&b).unwrap(); + let dest = a.join("Dest.xlsx"); + std::fs::write(&dest, linked_workbook("Source.xlsx")).unwrap(); + let mut s = s(); + s.execute("file.open", json!({"path": dest})).unwrap(); + s.execute("file.saveAs", json!({"path": b.join("Dest.xlsx")})).unwrap(); + assert_eq!(link_target(&b.join("Dest.xlsx")), "../Source.xlsx"); + // Saved again into the folder it was read from, the path is the one it was read with. + s.execute("file.saveAs", json!({"path": a.join("Copy.xlsx")})).unwrap(); + assert_eq!(link_target(&a.join("Copy.xlsx")), "Source.xlsx"); + // Read from a subfolder and saved into its parent, the path goes down into the subfolder. + let deep = b.join("Deep.xlsx"); + std::fs::write(&deep, linked_workbook("Source.xlsx")).unwrap(); + let mut s = Session::new(); + s.execute("file.open", json!({"path": deep})).unwrap(); + s.execute("file.saveAs", json!({"path": a.join("Deep.xlsx")})).unwrap(); + assert_eq!(link_target(&a.join("Deep.xlsx")), "b/Source.xlsx"); + std::fs::remove_dir_all(a).unwrap(); +} + /// A typo like `report.cvs` must not quietly write an XLSX under that name and adopt it as the /// document's file. #[test] @@ -3043,6 +3141,22 @@ fn a_decrypted_workbook_never_overwrites_its_encrypted_original() { std::fs::remove_dir_all(dir).unwrap(); } +#[test] +fn link_open_refuses_every_scheme_but_http_https_and_mailto() { + let mut s = s(); + s.execute("insert.link", json!({"cell": "A1", "target": "file:///etc/passwd"})).unwrap(); + s.take_ui_requests(); + for url in ["file:///etc/passwd", "smb://h/s", "\\\\h\\s", "JAVASCRIPT:1"] { + assert!(s.execute("link.open", json!({"url": url})).is_err(), "{url}"); + } + assert!(s.execute("link.open", json!({"cell": "A1"})).is_err()); + assert!(s.take_ui_requests().is_empty()); + s.execute("link.open", json!({"url": "HTTPS://example.org/a"})).unwrap(); + assert_eq!(s.take_ui_requests(), vec![crate::UiRequest::OpenUrl("HTTPS://example.org/a".into())]); + s.execute("link.open", json!({"url": "B2"})).unwrap(); + assert!(s.take_ui_requests().is_empty()); +} + #[test] fn subtotal_109_recalculates_when_row_height_or_autofit_reveals_rows() { let mut s = s(); diff --git a/crates/functions/src/criteria.rs b/crates/functions/src/criteria.rs index 2b4c488..e326267 100644 --- a/crates/functions/src/criteria.rs +++ b/crates/functions/src/criteria.rs @@ -138,9 +138,10 @@ impl Criterion { match self.op { Op::Eq => blank, Op::Ne => !blank, - // "<" / ">" with nothing: Excel compares against "" for text cells. + // "<" / ">" with nothing: Excel compares against "" for text cells, but an + // empty string never matches ("<=" and ">=" count it in neither). _ => match &v { - Value::Text(t) => cmp_op(self.op, compare_text(t, "")), + Value::Text(t) if !t.is_empty() => cmp_op(self.op, compare_text(t, "")), _ => false, }, } @@ -233,19 +234,25 @@ fn tokenize(pattern: &str) -> Vec { while let Some(c) = it.next() { match c { '~' => match it.next() { - Some(n) => out.extend(n.to_lowercase().map(Tok::Char)), + Some(n) => out.extend(std::iter::once(lower_char(n)).map(Tok::Char)), None => out.push(Tok::Char('~')), }, '*' => out.push(Tok::Star), '?' => out.push(Tok::Any), - c => out.extend(c.to_lowercase().map(Tok::Char)), + c => out.extend(std::iter::once(lower_char(c)).map(Tok::Char)), } } out } +/// Lowercase one char to exactly one char, so positions in the lowered text match the original +/// (U+0130 `İ` folds to `i`, like `fold` in text.rs). +fn lower_char(c: char) -> char { + c.to_lowercase().next().unwrap_or(c) +} + fn lower_chars(s: &str) -> Vec { - s.chars().flat_map(char::to_lowercase).collect() + s.chars().map(lower_char).collect() } /// Whether the pattern contains an unescaped wildcard. @@ -355,6 +362,9 @@ mod tests { assert!(m("<>", Value::from("a"))); assert!(!m("<>", Value::Empty)); assert!(m("TRUE", Value::Bool(true))); + // A bare "<=" or ">=" never matches an empty string (Excel 365: COUNTIF(="", "<=") is 0). + assert!(!m("<=", Value::from(""))); + assert!(!m(">=", Value::from(""))); assert!(m(" R { if !(1900..=9999).contains(&y) { return Err(CellError::Num); } - let dd = (d.day.max(1) as i64).min(days_in_month_in(sys, y as i32, mo as u32) as i64); + // Excel keeps serial 0's day 0 and gives February 1900 its real 28 days here, though the + // sheet's calendar has a 29 February 1900 (serial 60). + let dd = (d.day as i64).min(days_in_month(y as i32, mo as u32) as i64); num_val(ymd_serial(sys, y, mo, dd)? as f64) } @@ -177,7 +179,8 @@ fn eomonth(a: &[Arg], c: &mut dyn Ctx) -> R { if !(1900..=9999).contains(&y) { return Err(CellError::Num); } - num_val(ymd_serial(sys, y, mo, days_in_month_in(sys, y as i32, mo as u32) as i64)? as f64) + // Like EDATE, February 1900 ends on the 28th (serial 59). + num_val(ymd_serial(sys, y, mo, days_in_month(y as i32, mo as u32) as i64)? as f64) } fn datedif(a: &[Arg], c: &mut dyn Ctx) -> R { @@ -680,17 +683,35 @@ mod tests { close(ev("DATEVALUE", vec![t("2/29/1900")]), 60.0); close(ev("DATEVALUE", vec![t("3/1/1900")]), 61.0); is_err(ev("DATEVALUE", vec![t("2/29/1901")]), CellError::Value); - // EDATE and EOMONTH land on it as the end of February 1900. - close(ev("EOMONTH", vec![n(d(1900.0, 1.0, 15.0)), n(1.0)]), 60.0); - close(ev("EOMONTH", vec![n(60.0), n(0.0)]), 60.0); - close(ev("EOMONTH", vec![n(d(1900.0, 3.0, 15.0)), n(-1.0)]), 60.0); + // EDATE and EOMONTH read it as 29 February, but end February 1900 on the 28th. + close(ev("EOMONTH", vec![n(d(1900.0, 1.0, 15.0)), n(1.0)]), 59.0); + close(ev("EOMONTH", vec![n(60.0), n(0.0)]), 59.0); + close(ev("EOMONTH", vec![n(d(1900.0, 3.0, 15.0)), n(-1.0)]), 59.0); close(ev("EOMONTH", vec![n(60.0), n(12.0)]), d(1901.0, 2.0, 28.0)); - close(ev("EDATE", vec![n(d(1900.0, 1.0, 31.0)), n(1.0)]), 60.0); - close(ev("EDATE", vec![n(d(1900.0, 3.0, 31.0)), n(-1.0)]), 60.0); + close(ev("EDATE", vec![n(d(1900.0, 1.0, 31.0)), n(1.0)]), 59.0); + close(ev("EDATE", vec![n(d(1900.0, 3.0, 31.0)), n(-1.0)]), 59.0); close(ev("EDATE", vec![n(60.0), n(1.0)]), d(1900.0, 3.0, 29.0)); close(ev("EDATE", vec![n(60.0), n(12.0)]), d(1901.0, 2.0, 28.0)); } + #[test] + fn edate_eomonth_early_1900() { + // Excel 365: serial 0 is 0 January 1900 and February 1900 has 28 days here. + close(ev("EDATE", vec![n(0.0), n(0.0)]), 0.0); + close(ev("EDATE", vec![n(0.0), n(1.0)]), 31.0); + close(ev("EDATE", vec![n(0.0), n(12.0)]), 366.0); + close(ev("EOMONTH", vec![n(0.0), n(1.0)]), 59.0); + close(ev("EDATE", vec![n(31.0), n(1.0)]), 59.0); + close(ev("EOMONTH", vec![n(59.0), n(0.0)]), 59.0); + // Unchanged, and the same as Excel. + close(ev("EOMONTH", vec![n(0.0), n(0.0)]), 31.0); + is_err(ev("EOMONTH", vec![n(0.0), n(-1.0)]), CellError::Num); + close(ev("EDATE", vec![n(59.0), n(1.0)]), 88.0); + close(ev("EDATE", vec![n(60.0), n(1.0)]), 89.0); + close(ev("EDATE", vec![n(61.0), n(-1.0)]), 32.0); + close(ev("EOMONTH", vec![n(31.0), n(0.0)]), 31.0); + } + #[test] fn datedif_units() { let s = n(d(2001.0, 6.0, 1.0)); diff --git a/crates/functions/src/financial.rs b/crates/functions/src/financial.rs index 1fb84f8..2758246 100644 --- a/crates/functions/src/financial.rs +++ b/crates/functions/src/financial.rs @@ -321,7 +321,8 @@ fn mirr(a: &[Arg], _c: &mut dyn Ctx) -> R { neg += x / (1.0 + fr).powi(i as i32); } } - if pos == 0.0 || neg == 0.0 { + // No positive flow is not an error in Excel: the formula below gives -1. + if neg == 0.0 { return Err(CellError::Div0); } let r = (-pos * (1.0 + rr).powf(n - 1.0) / neg).powf(1.0 / (n - 1.0)) - 1.0; @@ -1813,6 +1814,8 @@ mod tests { close_tol(ev("MIRR", vec![rf(m), n(0.1), n(0.12)]), 0.1260943, 1e-6); close_tol(ev("MIRR", vec![rf(col(&[-120000.0, 39000.0, 30000.0, 21000.0])), n(0.1), n(0.12)]), -0.048044655, 1e-7); is_err(ev("MIRR", vec![rf(col(&[1.0, 2.0])), n(0.1), n(0.1)]), CellError::Div0); + // Excel 365 gives -1 when every flow is negative: the positive flows are worth nothing. + close_tol(ev("MIRR", vec![rf(col(&[-10000.0, -3400.0, -6500.0, -1000.0])), n(0.07), n(0.01)]), -1.0, 1e-12); close(ev("FVSCHEDULE", vec![n(1.0), av(row(&[0.09, 0.11, 0.1]))]), 1.33089); close(ev("EFFECT", vec![n(0.0525), n(4.0)]), 0.0535426673707584); is_err(ev("EFFECT", vec![n(-0.1), n(4.0)]), CellError::Num); diff --git a/crates/functions/src/text.rs b/crates/functions/src/text.rs index 6270866..91962d6 100644 --- a/crates/functions/src/text.rs +++ b/crates/functions/src/text.rs @@ -1,6 +1,6 @@ //! Text functions. -use gridcraft_core::parse::parse_number_text; +use gridcraft_core::parse::parse_number_text_in; use gridcraft_core::{CellError, Value, number_to_text}; use crate::criteria::wildcard_find; @@ -295,17 +295,17 @@ fn unicode(a: &[Arg], _c: &mut dyn Ctx) -> R { /// VALUE: a blank cell reads as 0, but empty or blank text is not a number (observed: /// `VALUE("")` is `#VALUE!`). -fn value(a: &[Arg], _c: &mut dyn Ctx) -> R { +fn value(a: &[Arg], c: &mut dyn Ctx) -> R { match scalar(arg(a, 0)?) { Value::Number(n) => num_val(n), Value::Empty => num_val(0.0), Value::Error(e) => Err(e), - Value::Text(t) => parse_number_text(&t).ok_or(CellError::Value).and_then(num_val), + Value::Text(t) => parse_number_text_in(&t, c.date_system()).ok_or(CellError::Value).and_then(num_val), _ => Err(CellError::Value), } } -fn numbervalue(a: &[Arg], _c: &mut dyn Ctx) -> R { +fn numbervalue(a: &[Arg], c: &mut dyn Ctx) -> R { let s = text(a, 0)?; let dec = if a.len() > 1 { text(a, 1)?.chars().next().ok_or(CellError::Value)? } else { '.' }; let grp = if a.len() > 2 { text(a, 2)?.chars().next().ok_or(CellError::Value)? } else { ',' }; @@ -338,7 +338,7 @@ fn numbervalue(a: &[Arg], _c: &mut dyn Ctx) -> R { out.push(c); } } - let mut n = parse_number_text(&out).ok_or(CellError::Value)?; + let mut n = parse_number_text_in(&out, c.date_system()).ok_or(CellError::Value)?; for _ in 0..pct { n /= 100.0; } @@ -845,6 +845,7 @@ pub(crate) fn specs() -> Vec { fixed ), f!("DOLLAR", 1, 2, Text, S, "DOLLAR(number, [decimals])", "Formats a number as currency text.", dollar), + f!("USDOLLAR", 1, 2, Compatibility, S, "USDOLLAR(number, [decimals])", "Formats a number as currency text, like DOLLAR.", dollar), f!("BAHTTEXT", 1, 1, Text, S, "BAHTTEXT(number)", "Spells out a number in Thai words with Baht.", bahttext), f!("T", 1, 1, Text, S, "T(value)", "Returns the value if it is text, otherwise empty text.", t_fn), f!( @@ -1036,6 +1037,11 @@ mod tests { close(ev("SEARCH", vec![t("~*"), t("a*b")]), 2.0); is_err(ev("SEARCH", vec![t("z"), t("abc")]), CellError::Value); is_err(ev("SEARCH", vec![t("a"), t("abc"), n(5.0)]), CellError::Value); + close(ev("SEARCH", vec![t("b"), t("\u{130}b")]), 2.0); + close(ev("SEARCH", vec![t("B"), t("\u{130}b")]), 2.0); + close(ev("SEARCH", vec![t("b?"), t("a\u{130}bc")]), 3.0); + close(ev("SEARCH", vec![t("i"), t("a\u{130}bc")]), 2.0); + is_err(ev("SEARCH", vec![t("b"), t("a\u{130}bc"), n(4.0)]), CellError::Value); for f in ["FIND", "SEARCH"] { close(ev(f, vec![t(""), t("abc"), n(3.0)]), 3.0); is_err(ev(f, vec![t(""), t("abc"), n(4.0)]), CellError::Value); @@ -1099,6 +1105,8 @@ mod tests { is_text(ev("DOLLAR", vec![n(1234.567)]), "$1,234.57"); is_text(ev("DOLLAR", vec![n(-1234.567), n(1.0)]), "($1,234.6)"); is_text(ev("DOLLAR", vec![n(0.5), n(0.0)]), "$1"); + is_text(ev("USDOLLAR", vec![n(1.0)]), "$1.00"); + is_text(ev("USDOLLAR", vec![n(-1234.567), n(1.0)]), "($1,234.6)"); is_text(ev("T", vec![t("x")]), "x"); is_text(ev("T", vec![n(1.0)]), ""); } diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index fa64aad..a76f30f 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -18,7 +18,7 @@ pub mod workbook; pub use cell::{Cell, CellPicture, Formula}; pub use features::*; pub use password::PasswordHash; -pub use preserved::{Preserved, PreservedPart, PreservedRel, PreservedXml}; +pub use preserved::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml}; pub use sheet::{DEFAULT_COL_WIDTH, DEFAULT_ROW_HEIGHT, LineIndex, Sheet, SheetViewMode, Visibility, points_to_chars}; pub use store::CellStore; pub use style::{ diff --git a/crates/model/src/preserved.rs b/crates/model/src/preserved.rs index eb8e2bb..eab8501 100644 --- a/crates/model/src/preserved.rs +++ b/crates/model/src/preserved.rs @@ -7,6 +7,8 @@ use std::sync::Arc; +use crate::sheet::Visibility; + /// A relationship from an owning part (the package root, the workbook or a sheet, or a kept part) /// to a kept part or to an external target. #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -43,6 +45,19 @@ pub struct PreservedXml { pub xml: String, } +/// A chart sheet (a sheet that holds one chart and no cells) kept as it was in the source file. Its +/// part, drawing, chart, style and colour parts are in [`Preserved::parts`]; the workbook +/// relationship to it is in [`Preserved::rels`]. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct PreservedSheet { + pub name: String, + pub visibility: Visibility, + /// The id of the workbook relationship to the chart sheet part, in the source file. + pub rel_id: String, + /// How many worksheets come before it: its position among the model's sheets. + pub before: usize, +} + /// Unmodelled content kept for one owner (the workbook, which also holds package-root content, /// or one sheet). #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -58,11 +73,18 @@ pub struct Preserved { /// The sheet's legacy VML drawing when kept elements need its shapes (form controls, OLE /// objects). Its own relationships point into `parts`. pub vml: Option, + /// Chart sheets, in file order (workbook owner only). + pub sheets: Vec, } impl Preserved { pub fn is_empty(&self) -> bool { - self.package_rels.is_empty() && self.rels.is_empty() && self.parts.is_empty() && self.elements.is_empty() && self.vml.is_none() + self.package_rels.is_empty() + && self.rels.is_empty() + && self.parts.is_empty() + && self.elements.is_empty() + && self.vml.is_none() + && self.sheets.is_empty() } /// Total bytes held. pub fn byte_len(&self) -> usize { diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 18e1971..828ca69 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -116,6 +116,10 @@ pub struct Workbook { /// Unmodelled file content kept for saving (package root and workbook level). Not in JSON. #[serde(skip)] pub preserved: Option>, + /// The folder the workbook was read from, which relative paths to linked workbooks in + /// `preserved` are relative to. Saving into another folder rewrites them from there. Not in JSON. + #[serde(skip)] + pub source_dir: Option, } impl Default for Workbook { @@ -143,6 +147,7 @@ impl Workbook { vba_project: None, csv_delimiter: None, preserved: None, + source_dir: None, } } pub fn sheet(&self, i: usize) -> Option<&Sheet> { diff --git a/crates/numfmt/src/lib.rs b/crates/numfmt/src/lib.rs index e5c65bd..6280e1d 100644 --- a/crates/numfmt/src/lib.rs +++ b/crates/numfmt/src/lib.rs @@ -98,15 +98,38 @@ impl Default for NumberFormat { } } +/// Longest number format code, in characters (not bytes), that is parsed: Excel's own limit. +/// A longer code is invalid: [`NumberFormat::parse`] returns a General format (the cell shows +/// its value unformatted; the stored code is kept as is) and `TEXT` returns `#VALUE!`. The cap +/// also bounds every allocation that scales with the number of placeholders in a code. +pub const MAX_FORMAT_CODE_CHARS: usize = 255; + +/// True when `code` is longer than [`MAX_FORMAT_CODE_CHARS`] characters. +fn too_long(code: &str) -> bool { + // A code of more than 255 bytes can still be short in characters; count only when needed. + code.len() > MAX_FORMAT_CODE_CHARS && code.chars().count() > MAX_FORMAT_CODE_CHARS +} + impl NumberFormat { - /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text. + /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text, and + /// a code longer than [`MAX_FORMAT_CODE_CHARS`] characters degrades to General. pub fn parse(code: &str) -> NumberFormat { + Self::parse_inner(code, false) + } + + /// `parse` without the length cap, so tests can reach renderer paths a capped code cannot. + #[cfg(test)] + fn parse_uncapped(code: &str) -> NumberFormat { + Self::parse_inner(code, true) + } + + fn parse_inner(code: &str, uncapped: bool) -> NumberFormat { #[cfg(test)] parse_cache::count_parse(); let trimmed = code.trim(); // `checkbox` marks a checkbox cell (Insert › Checkbox): TRUE/FALSE draw as a box, any other // value displays as General. - if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("general") || trimmed == "checkbox" { + if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("general") || trimmed == "checkbox" || (!uncapped && too_long(code)) { return NumberFormat { inner: Arc::new(Inner { code: code.to_string(), general: true, sections: Vec::new(), num_sections: 0, text_section: None }), }; @@ -376,6 +399,9 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result return Err(e), diff --git a/crates/numfmt/src/parse.rs b/crates/numfmt/src/parse.rs index 6df22ad..63e2d30 100644 --- a/crates/numfmt/src/parse.rs +++ b/crates/numfmt/src/parse.rs @@ -125,12 +125,20 @@ pub(crate) struct Section { } /// Splits a code into sections at unquoted, unescaped, unbracketed semicolons. +/// +/// A `[` opens a bracket only when a `]` follows it; the bracket then runs to the next `]` (quotes +/// inside it are not special), exactly as `tokenize` reads it. An unclosed `[` is a literal in both +/// passes, so a later `;` still separates sections. Linear: each `[` either jumps over its bracket +/// or, with no `]` left in the code, is a literal at once. fn split_sections(code: &str) -> Vec { + let cs: Vec = code.chars().collect(); + let last_close = cs.iter().rposition(|&c| c == ']'); let mut out = vec![String::new()]; let mut quoted = false; - let mut bracket = false; let mut escape = false; - for c in code.chars() { + let mut i = 0usize; + while let Some(&c) = cs.get(i) { + i += 1; let cur = match out.last_mut() { Some(c) => c, None => break, @@ -145,19 +153,22 @@ fn split_sections(code: &str) -> Vec { quoted = !quoted; cur.push(c); } - '\\' | '_' | '*' if !quoted && !bracket => { + '\\' | '_' | '*' if !quoted => { escape = true; cur.push(c); } - '[' if !quoted => { - bracket = true; + '[' if !quoted && last_close.is_some_and(|l| l >= i) => { + // `i` is already past the `[`; the closing bracket is at or after it. cur.push(c); + while let Some(&d) = cs.get(i) { + i += 1; + cur.push(d); + if d == ']' { + break; + } + } } - ']' if !quoted => { - bracket = false; - cur.push(c); - } - ';' if !quoted && !bracket => { + ';' if !quoted => { if out.len() >= MAX_SECTIONS { break; } @@ -246,6 +257,8 @@ fn tokenize(src: &str) -> Raw { let cs: Vec = src.chars().collect(); let mut raw = Raw { toks: Vec::new(), color: None, cond: None, currency: false }; let toks = &mut raw.toks; + // With no `]` at or after a `[`, that `[` is a literal; checking this once keeps the scan linear. + let last_close = cs.iter().rposition(|&c| c == ']'); let mut i = 0usize; while let Some(&c) = cs.get(i) { match c { @@ -290,7 +303,7 @@ fn tokenize(src: &str) -> Raw { continue; } '[' => { - let close = cs.get(i + 1..).and_then(|r| r.iter().position(|&d| d == ']')); + let close = if last_close.is_some_and(|l| l > i) { cs.get(i + 1..).and_then(|r| r.iter().position(|&d| d == ']')) } else { None }; if let Some(p) = close { let content: String = cs.get(i + 1..i + 1 + p).unwrap_or(&[]).iter().collect(); bracket(&mut raw_parts(toks, &mut raw.color, &mut raw.cond, &mut raw.currency), &content); @@ -513,6 +526,19 @@ fn analyze(raw: Raw) -> Section { let mut seen_exp = false; let mut seen_digit = false; let mut fraction_slash = false; + // Precomputed so each comma and slash is O(1) (a long run of them stays linear): whether the + // first non-comma token after a position is a digit, and where the last exponent marker is. + let mut digit_after_commas = vec![false; toks.len()]; + let mut after = false; + for (j, t) in toks.iter().enumerate().rev() { + if let Some(slot) = digit_after_commas.get_mut(j) { + *slot = after; + } + if !matches!(t, Tok::Comma) { + after = matches!(t, Tok::Digit(_)); + } + } + let last_exp = toks.iter().rposition(|t| matches!(t, Tok::Exp { .. })); let scientific = toks.iter().skip_while(|t| !matches!(t, Tok::Digit(_))).any(|t| matches!(t, Tok::Exp { .. })); for i in 0..toks.len() { let Some(t) = toks.get(i).cloned() else { break }; @@ -543,8 +569,7 @@ fn analyze(raw: Raw) -> Section { } } Tok::Comma => { - let next = toks.get(i + 1..).and_then(|s| s.iter().find(|t| !matches!(t, Tok::Comma))); - let next_digit = matches!(next, Some(Tok::Digit(_))); + let next_digit = digit_after_commas.get(i).copied().unwrap_or(false); if seen_digit && next_digit && !seen_point && !seen_exp { sec.thousands = true; Some(Tok::Lit(String::new())) @@ -563,7 +588,8 @@ fn analyze(raw: Raw) -> Section { // Literal text may sit between the bar and the denominator (`#/ #`, `# ?=/=?`). let next_ok = matches!(toks.get(i + 1), Some(Tok::FixedDenom(_))) || matches!(toks.get(i + 1..).and_then(|s| s.iter().find(|t| !matches!(t, Tok::Lit(_)))), Some(Tok::Digit(_))); - let has_exp = toks.iter().any(|t| matches!(t, Tok::Exp { .. })); + // An exponent kept before this bar (`seen_exp`) or any marker still ahead of it. + let has_exp = seen_exp || last_exp.is_some_and(|l| l > i); if !fraction_slash && seen_digit && next_ok && !seen_point && !has_exp { fraction_slash = true; sec.fraction = true; diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index 64a4fca..4408d38 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -509,7 +509,9 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt // Denominator digits are zero padded to the `0` placeholders, then left aligned; `?` // pads with spaces after them. let zeros = den_idx.iter().filter(|&&i| toks.get(i) == Some(&Tok::Digit('0'))).count(); - let padded = format!("{den_s:0>zeros$}"); + // No `{:0>zeros$}`: a format width above u16::MAX panics. + let mut padded = "0".repeat(zeros.saturating_sub(den_s.chars().count())); + padded.push_str(&den_s); let dc: Vec = padded.chars().collect(); let count = den_idx.len(); for (k, &ti) in den_idx.iter().enumerate() { diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index c335ed8..e5687f5 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -777,11 +777,90 @@ fn parse_cache_is_bounded() { assert!(text_function(&Value::Number(1.0), "0.00\"0\"", S).is_ok()); assert_eq!(parse_cache::parses() - before, 1); - // A very long code is parsed on every call and not stored. - let long = format!("0\"{}\"", "x".repeat(parse_cache::MAX_CODE_LEN)); + // A code too long (in bytes) for the cache is parsed on every call and not stored. Two-byte + // characters keep it under the 255-character cap that TEXT() refuses past. + let long = format!("0\"{}\"", "é".repeat(parse_cache::MAX_CODE_LEN / 2 + 8)); let before = parse_cache::parses(); for _ in 0..3 { assert!(text_function(&Value::Number(1.0), &long, S).is_ok()); } assert_eq!(parse_cache::parses() - before, 3); } + +#[test] +fn fraction_denominator_with_65536_zeros_does_not_panic() { + // `{:0>n$}` panics for a width above u16::MAX; the cap keeps this out of reach of `parse`, + // so the renderer is driven with the uncapped parser. + let code = format!("# ?/{}", "0".repeat(65_536)); + let fmt = NumberFormat::parse_uncapped(&code); + let text = format_value(&Value::Number(0.5), &fmt, S).text; + assert!(text.starts_with(" 1/"), "{text}"); + // And a denominator with fewer digits than zeros is still zero padded. + assert_eq!(f("# ?/000", 0.5), " 1/002"); +} + +#[test] +fn format_codes_are_capped_at_255_characters() { + let ok = format!("0{}", "0".repeat(254)); + let long = format!("0{}", "0".repeat(255)); + assert_eq!(ok.chars().count(), MAX_FORMAT_CODE_CHARS); + assert!(text_function(&Value::Number(1.0), &ok, S).is_ok()); + assert_eq!(text_function(&Value::Number(1.0), &long, S), Err(CellError::Value)); + // Capped codes display as General. + assert_eq!(f(&long, 0.5), "0.5"); + // Characters are counted, not bytes: 255 two-byte characters are fine, 256 are not. + let multi_ok = "é".repeat(255); + assert!(multi_ok.len() > 255); + assert!(text_function(&Value::Number(1.0), &multi_ok, S).is_ok()); + assert_eq!(text_function(&Value::Number(1.0), &"é".repeat(256), S), Err(CellError::Value)); + // 65536 zeros through the public API: no panic, TEXT is #VALUE!. + let huge = format!("# ?/{}", "0".repeat(65_536)); + assert_eq!(text_function(&Value::Number(0.5), &huge, S), Err(CellError::Value)); + assert_eq!(f(&huge, 0.5), "0.5"); +} + +/// Parses and renders `code` on a worker thread; panics if that takes more than 20 s, so a quadratic +/// parser fails the test instead of hanging CI. Not a benchmark: at this size an O(n^2) pass takes +/// minutes and an O(n) pass milliseconds, so the margin is decades wide. +fn finishes_in_time(code: String) -> String { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + // Uncapped: `parse` turns a code past 255 characters into General before the parser sees it. + let _ = tx.send(format_value(&Value::Number(1234.5), &NumberFormat::parse_uncapped(&code), S).text); + }); + rx.recv_timeout(std::time::Duration::from_secs(20)).expect("format code did not parse and render in time") +} + +#[test] +fn hostile_codes_parse_in_linear_time() { + let n = 1_000_000; + // Unclosed brackets, a run of commas after a digit, a run of fraction bars. + assert!(finishes_in_time("[".repeat(n)).starts_with('[')); + finishes_in_time(format!("0{}", ",".repeat(n))); + finishes_in_time(format!("0{}", "/".repeat(n))); + finishes_in_time(format!("0{}0", ",".repeat(n))); + // Unclosed brackets mixed with semicolons: each is a literal and a section separator stays one. + finishes_in_time("[;".repeat(n / 2)); +} + +#[test] +fn unclosed_bracket_is_a_literal_in_both_passes() { + // Sections still split after an unclosed `[`. + assert_eq!(f("[0;0", 5.0), "[5"); + assert_eq!(f("0;[0", -5.0), "[5"); + assert_eq!(f("0;-0;[", 0.0), "["); + assert_eq!(parse::parse_sections("0;[Red-0").len(), 2); + assert_eq!(parse::parse_sections("[0;0").len(), 2); +} + +#[test] +fn closed_brackets_still_hold_their_semicolons() { + assert_eq!(f("[Red]0;[Blue]-0", -5.0), "-5"); + assert_eq!(f("[>=100]0.0;0", 150.0), "150.0"); + assert_eq!(f("[$-409]0.00", 2.5), "2.50"); + assert_eq!(f("[$€-2] #,##0", 1234.0), "€ 1,234"); + assert_eq!(parse::parse_sections("[Red]0;[Blue]0").len(), 2); + assert_eq!(parse::parse_sections("[unknown;x]0;0").len(), 2); + assert_eq!(f("0\\[0", 5.0), "0[5"); + assert_eq!(f("\"[\"0;0", 5.0), "[5"); +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index b98a4d5..fb899c7 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -349,6 +349,20 @@ impl Dialog { json!({"text": existing}), ) } + // Asks before a web or e-mail link leaves the workbook; OK runs `link.open`, which + // refuses anything but http, https and mailto. + "openLink" => { + let url = params.get("url").and_then(Json::as_str).unwrap_or_default().to_string(); + Dialog::form( + "openLink", + "Open Hyperlink", + "link.open", + vec![Note(format!( + "This workbook asks to open the address below. Links can lead to harmful sites or start other programs: open it only if you trust the workbook.\n\n{url}" + ))], + json!({"url": url}), + ) + } "insertLink" => Dialog::form( "insertLink", "Insert Hyperlink", @@ -552,47 +566,7 @@ impl Dialog { "dataValidation", "Data Validation", "data.validation", - vec![ - Choice { - key: "type", - label: "Allow", - options: vec![ - ("any", "Any value"), - ("whole", "Whole number"), - ("decimal", "Decimal"), - ("list", "List"), - ("date", "Date"), - ("time", "Time"), - ("textLength", "Text length"), - ("custom", "Custom"), - ], - }, - Choice { - key: "operator", - label: "Data", - options: vec![ - ("between", "between"), - ("notBetween", "not between"), - ("equal", "equal to"), - ("notEqual", "not equal to"), - ("greater", "greater than"), - ("less", "less than"), - ("greaterOrEqual", "greater than or equal to"), - ("lessOrEqual", "less than or equal to"), - ], - }, - Text { key: "formula1", label: "Minimum / Source / Formula:" }, - Text { key: "formula2", label: "Maximum:" }, - Check { key: "ignoreBlank", label: "Ignore blank" }, - Check { key: "dropdown", label: "In-cell dropdown" }, - Text { key: "inputMessage", label: "Input message:" }, - Choice { - key: "errorStyle", - label: "Error style", - options: vec![("stop", "Stop"), ("warning", "Warning"), ("information", "Information")], - }, - Text { key: "errorMessage", label: "Error message:" }, - ], + data_validation_fields("any", "between"), validation_defaults(app), ), "cfQuick" => { @@ -860,10 +834,132 @@ fn abs(r: &gridcraft_engine::core::RangeRef) -> String { if r.is_single() { a(r.start) } else { format!("{}:{}", a(r.start), a(r.end)) } } +/// The criterion controls in Excel's Settings tab depend on both Allow and Data. +/// `formula1` is the sole source for a list or custom rule; `formula2` is a +/// second bound only for Between and Not between. +fn data_validation_fields(kind: &str, operator: &str) -> Vec { + use Field::*; + let mut fields = vec![Choice { + key: "type", + label: "Allow", + options: vec![ + ("any", "Any value"), + ("whole", "Whole number"), + ("decimal", "Decimal"), + ("list", "List"), + ("date", "Date"), + ("time", "Time"), + ("textLength", "Text length"), + ("custom", "Custom"), + ], + }]; + let two_bounds = matches!(operator, "between" | "notBetween"); + match kind { + "list" => { + fields.push(Text { key: "formula1", label: "Source:" }); + fields.push(Check { key: "ignoreBlank", label: "Ignore blank" }); + fields.push(Check { key: "dropdown", label: "In-cell dropdown" }); + } + "custom" => { + fields.push(Text { key: "formula1", label: "Formula:" }); + fields.push(Check { key: "ignoreBlank", label: "Ignore blank" }); + } + "whole" | "decimal" | "date" | "time" | "textLength" => { + fields.push(Choice { + key: "operator", + label: "Data", + options: vec![ + ("between", "between"), + ("notBetween", "not between"), + ("equal", "equal to"), + ("notEqual", "not equal to"), + ("greater", "greater than"), + ("less", "less than"), + ("greaterOrEqual", "greater than or equal to"), + ("lessOrEqual", "less than or equal to"), + ], + }); + let lower = matches!(operator, "greater" | "greaterOrEqual"); + let (first, second) = match kind { + "date" => ( + if two_bounds || lower { + "Start date:" + } else if matches!(operator, "less" | "lessOrEqual") { + "End date:" + } else { + "Date:" + }, + "End date:", + ), + "time" => ( + if two_bounds || lower { + "Start time:" + } else if matches!(operator, "less" | "lessOrEqual") { + "End time:" + } else { + "Time:" + }, + "End time:", + ), + _ => ( + if two_bounds || lower { + "Minimum:" + } else if matches!(operator, "less" | "lessOrEqual") { + "Maximum:" + } else { + "Value:" + }, + "Maximum:", + ), + }; + fields.push(Text { key: "formula1", label: first }); + if two_bounds { + fields.push(Text { key: "formula2", label: second }); + } + fields.push(Check { key: "ignoreBlank", label: "Ignore blank" }); + } + _ => {} + } + fields.push(Text { key: "inputMessage", label: "Input message:" }); + fields.push(Choice { + key: "errorStyle", + label: "Error style", + options: vec![("stop", "Stop"), ("warning", "Warning"), ("information", "Information")], + }); + fields.push(Text { key: "errorMessage", label: "Error message:" }); + fields +} + +fn data_validation_form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) { + let kind = d.values.get("type").and_then(Json::as_str).unwrap_or("any"); + let operator = d.values.get("operator").and_then(Json::as_str).unwrap_or("between"); + d.fields = data_validation_fields(kind, operator); + form_fields(ui, d); + ok_cancel(ui, confirm, open); +} + /// Builds the command params from form values for dialogs that need translation. fn build_params(d: &Dialog) -> Json { let mut v = Json::Object(d.values.clone()); match d.name.as_str() { + "dataValidation" => { + let kind = d.values.get("type").and_then(Json::as_str).unwrap_or("any"); + let operator = d.values.get("operator").and_then(Json::as_str).unwrap_or("between"); + if let Some(m) = v.as_object_mut() { + if !matches!(operator, "between" | "notBetween") || matches!(kind, "any" | "list" | "custom") { + m.remove("formula2"); + } + if matches!(kind, "any" | "list" | "custom") { + m.remove("operator"); + } + if kind == "any" { + m.remove("formula1"); + } + if kind != "list" { + m.remove("dropdown"); + } + } + } "textToColumns" => { let mut delims = Vec::new(); for (k, c) in [("tab", "\t"), ("semicolon", ";"), ("comma", ","), ("space", " ")] { @@ -951,6 +1047,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { ui.set_min_width(width - 20.0); match d.name.as_str() { "formatCells" => format_cells(app, ui, &mut d, &mut confirm), + "dataValidation" => data_validation_form(ui, &mut d, &mut confirm, &mut open), "insertFunction" => insert_function(app, ui, &mut d, &mut confirm), "find" => find(app, ui, &mut d), "goTo" => go_to(app, ui, &mut d, &mut confirm), @@ -1338,9 +1435,20 @@ fn form_fields(ui: &mut egui::Ui, d: &mut Dialog) { ui.vertical(|ui| { ui.label(egui::RichText::new(label).strong()); let cur = d.values.get(key).and_then(Json::as_str).unwrap_or("").to_string(); - for (val, text) in options { - if ui.radio(cur == val, text).clicked() { - d.set_value(key, json!(val)); + if d.name == "dataValidation" { + let selected = options.iter().find(|(val, _)| *val == cur).map(|(_, text)| *text).unwrap_or(""); + egui::ComboBox::from_id_salt(("dataValidation", key)).selected_text(selected).width(300.0).show_ui(ui, |ui| { + for (val, text) in options { + if ui.selectable_label(cur == val, text).clicked() { + d.set_value(key, json!(val)); + } + } + }); + } else { + for (val, text) in options { + if ui.radio(cur == val, text).clicked() { + d.set_value(key, json!(val)); + } } } }); @@ -2554,6 +2662,51 @@ fn spelling(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, open: &mut bo }); } +#[cfg(test)] +mod data_validation_tests { + use super::*; + + #[test] + fn list_uses_one_source_and_does_not_submit_a_second_list() { + let fields = data_validation_fields("list", "between"); + assert!(fields.iter().any(|f| matches!(f, Field::Text { key: "formula1", label: "Source:" }))); + assert!(!fields.iter().any(|f| matches!(f, Field::Text { key: "formula2", .. } | Field::Choice { key: "operator", .. }))); + + let dialog = Dialog::form( + "dataValidation", + "Data Validation", + "data.validation", + fields, + json!({"type": "list", "operator": "between", "formula1": "ABC,DEF", "formula2": "GHI,JKL", "dropdown": true}), + ); + let params = build_params(&dialog); + assert_eq!(params["formula1"], "ABC,DEF"); + assert!(params.get("formula2").is_none()); + assert!(params.get("operator").is_none()); + } + + #[test] + fn only_between_operators_show_and_submit_two_bounds() { + let two = data_validation_fields("whole", "between"); + assert!(two.iter().any(|f| matches!(f, Field::Text { key: "formula2", label: "Maximum:" }))); + let one = data_validation_fields("date", "lessOrEqual"); + assert!(one.iter().any(|f| matches!(f, Field::Text { key: "formula1", label: "End date:" }))); + assert!(!one.iter().any(|f| matches!(f, Field::Text { key: "formula2", .. }))); + + let dialog = Dialog::form( + "dataValidation", + "Data Validation", + "data.validation", + one, + json!({"type": "date", "operator": "lessOrEqual", "formula1": "2026-10-12", "formula2": "2030-01-01"}), + ); + let params = build_params(&dialog); + assert_eq!(params["operator"], "lessOrEqual"); + assert_eq!(params["formula1"], "2026-10-12"); + assert!(params.get("formula2").is_none()); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index a9eb039..085b113 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -20,6 +20,42 @@ pub const COL_HEADER_H: f32 = 21.0; /// which matches desktop spreadsheet density (a default 20 px row is 16 pt tall). pub const DISPLAY_SCALE: f32 = 0.8; +/// The zoom percentages a Ctrl+wheel notch steps between: four per doubling (about 20% apart), +/// so 25, 50, 100, 200 and 400 are on it. Excel steps 15 points a notch instead; the status bar's +/// Zoom In / Zoom Out buttons keep their 10% steps. +pub const WHEEL_ZOOM_LADDER: [u16; 22] = [10, 12, 15, 18, 21, 25, 30, 35, 42, 50, 60, 70, 85, 100, 120, 140, 170, 200, 240, 280, 340, 400]; + +/// Seconds without Ctrl+wheel input after which part of a notch turned so far is forgotten. +const WHEEL_ZOOM_IDLE: f64 = 0.75; + +/// The zoom one Ctrl+wheel notch goes to from `zoom`: the next step on [`WHEEL_ZOOM_LADDER`] in +/// that direction, skipping a step less than 5% away (121% out goes to 100%, not 120%), but never +/// 100% or the 10% / 400% limits (99% or 101% towards 100% goes to 100%). +pub fn wheel_zoom_step(zoom: u16, zoom_in: bool) -> u16 { + let zoom = zoom.clamp(10, 400); + let z = u32::from(zoom); + let far = |l: u32| if zoom_in { l * 100 >= z * 105 } else { l * 105 <= z * 100 }; + let mut ahead = WHEEL_ZOOM_LADDER.iter().copied().filter(|&l| if zoom_in { l > zoom } else { l < zoom }).collect::>(); + if !zoom_in { + ahead.reverse(); + } + let limit = ahead.last().copied().unwrap_or(zoom); + ahead.into_iter().find(|&l| l == 100 || far(u32::from(l))).unwrap_or(limit) +} + +/// One frame of the zoom gliding from `from` to `to` percent: the share `t` (0–1) of the way, +/// measured as a ratio so zooming in and out feel alike, and at least one percent, so it arrives. +pub fn zoom_glide_step(from: u16, to: u16, t: f32) -> u16 { + let (f, g) = (f32::from(from.max(1)), f32::from(to.max(1))); + let next = (f * (g / f).powf(t.clamp(0.0, 1.0))).round(); + let next = if next.is_finite() { next.clamp(f32::from(from.min(to)), f32::from(from.max(to))) as u16 } else { to }; + match (next == from, from.cmp(&to)) { + (true, std::cmp::Ordering::Less) => from + 1, + (true, std::cmp::Ordering::Greater) => from - 1, + _ => next, + } +} + #[derive(Clone, Debug, Default, PartialEq)] pub enum Drag { #[default] @@ -109,6 +145,36 @@ pub struct GridState { pub hf_start: bool, /// The grid geometry of the last frame (hit-testing for agents and tests). pub geo: Option, + /// The horizontal scrollbar's track in the sheet tab bar (hit-testing for tests). + pub hscroll: Option, + /// Ctrl+wheel zoom in progress (see [`WheelZoom`]). + pub wheel_zoom: Option, +} + +/// Ctrl+wheel zoom for one (doc uid, sheet). egui eases a notch in over several frames and +/// fine-grained wheels send parts of one, so the turning is counted in notches, and each whole +/// notch steps [`WHEEL_ZOOM_LADDER`]. With animations on the zoom then glides to that step. +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct WheelZoom { + pub place: (u64, usize), + /// Turned so far towards the next step, in notches (+ in, − out). + pub notches: f32, + /// When `notches` last changed. + pub time: f64, + /// The step the zoom is gliding to, and the percentage set last on the way. + pub glide: Option<(u16, u16)>, +} + +impl WheelZoom { + /// The state for `place` showing `zoom` percent: fresh for another document or sheet, and + /// without its glide if the zoom was changed some other way since the glide set it. + fn at(app: &SheetApp, place: (u64, usize), zoom: u16, now: f64) -> WheelZoom { + let mut w = app.grid.wheel_zoom.filter(|w| w.place == place).unwrap_or(WheelZoom { place, notches: 0.0, time: now, glide: None }); + if w.glide.is_some_and(|(_, shown)| shown != zoom) { + w.glide = None; + } + w + } } impl GridState { @@ -235,6 +301,13 @@ impl Geo { None => (sh.row_top(row) - sh.row_top(self.fr)) as f32, } } + /// Scroll offset (in `scroll` units) at which `col` reaches the left of the scrolled area. + pub fn scroll_left_of(&self, sh: &Sheet, col: u32) -> f32 { + match &self.paper { + Some(pg) => pg.col_x(col) as f32, + None => (sh.col_left(col) - sh.col_left(self.fc)) as f32, + } + } /// Screen x for a logical x (and back): identity unless the sheet is right-to-left. pub fn mirror_x(&self, x: f32) -> f32 { if self.rtl { self.rect.left() + self.rect.right() - x } else { x } @@ -373,6 +446,15 @@ fn numfmt_color(c: gridcraft_engine::numfmt::FormatColor) -> Color32 { } } +/// Last column of a Center Across Selection run starting at `start`: it extends over following columns (up to `limit`) while `continues` holds. +fn center_across_end(start: u32, limit: u32, continues: impl Fn(u32) -> bool) -> u32 { + let mut end = start; + while end < limit && end + 1 < MAX_COLS && continues(end + 1) { + end += 1; + } + end +} + fn look(wb: &Workbook, si: usize, sh: &Sheet, c: CellRef, cf: &mut CfCache) -> (Look, Option) { let mut style = wb.styles.get(sh.style_id(c)).clone(); if let Some(t) = sh.table_at(c) { @@ -513,8 +595,34 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { let (scroll_delta, zoom_delta, modifiers, now) = ui.input(|i| (i.smooth_scroll_delta, i.zoom_delta(), i.modifiers, i.time)); if (resp.hovered() || resp.contains_pointer()) && !crate::text_box::contains_pointer(app, ui.ctx()) { if zoom_delta != 1.0 { - let z = (sh.zoom as f32 * zoom_delta).clamp(10.0, 400.0).round(); - let _ = app.session.run("view.zoom", json!({"percent": z})); + // egui's zoom factor is e^(speed × points); one notch is `line_scroll_speed` points. + let notch = ui.ctx().options(|o| o.input_options.scroll_zoom_speed * o.input_options.line_scroll_speed); + let turned = zoom_delta.ln() / notch; + let mut zooming = WheelZoom::at(app, place, sh.zoom, now); + if now - zooming.time > WHEEL_ZOOM_IDLE { + zooming.notches = 0.0; + } + if turned.is_finite() { + zooming.notches = (zooming.notches + turned).clamp(-32.0, 32.0); + } + zooming.time = now; + // A notch steps once 60% of it has turned, and the rest of it brings `notches` back to + // about 0. (Past half, so what's left after a step can't step back.) Steps count from + // where a glide is going, not from where it has got to. + let from = zooming.glide.map_or(sh.zoom, |(to, _)| to); + let mut z = from; + while zooming.notches >= 0.6 { + z = wheel_zoom_step(z, true); + zooming.notches -= 1.0; + } + while zooming.notches <= -0.6 { + z = wheel_zoom_step(z, false); + zooming.notches += 1.0; + } + if z != from { + zooming.glide = Some((z, sh.zoom)); + } + app.grid.wheel_zoom = Some(zooming); } else if scroll_delta != egui::Vec2::ZERO { let z = if paper { make_geo(app, view.scroll).z } else { sh.zoom as f32 / 100.0 * DISPLAY_SCALE }; let mut d = scroll_delta; @@ -529,6 +637,22 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { view.scroll -= d / z; } } + // The zoom glides to its step like egui eases a wheel notch (90% of the way in 0.1 s), or goes + // there at once without animations (as for the selection outline below: View › Animations + // off, egui's off in the headless harness, or a screenshot on its way). + let mut zooming = WheelZoom::at(app, place, sh.zoom, now); + if let Some((to, shown)) = zooming.glide { + let glides = app.ui.animations && ui.ctx().global_style().animation_time > 0.0 && !app.shots.in_flight(); + let share = egui::emath::exponential_smooth_factor(0.9, 0.1, ui.input(|i| i.stable_dt).min(0.1)); + let z = if glides { zoom_glide_step(shown, to, share) } else { to }; + let _ = app.session.run("view.zoom", json!({"percent": z})); + zooming.glide = (z != to).then_some((to, z)); + ui.ctx().request_repaint(); + } + // Store a glide's progress, or that it stopped. + if app.grid.wheel_zoom.is_some_and(|w| w.glide.is_some()) { + app.grid.wheel_zoom = Some(zooming); + } view.scroll = view.scroll.max(egui::Vec2::ZERO); let (mut max_x, mut max_y) = (sh.col_left(MAX_COLS - 20) as f32, sh.row_top(MAX_ROWS - 40) as f32); if paper { @@ -742,6 +866,63 @@ fn vertical_scrollbar(app: &mut SheetApp, ui: &mut egui::Ui, sh: &Sheet, geo: &G } } +/// The horizontal counterpart of `vertical_scrollbar`, drawn in the sheet tab bar as in Excel. The tab +/// bar is laid out before the grid, so this reads the last frame's geometry and leaves clamping the +/// far end to the grid, which runs later in the same frame. +pub(crate) fn horizontal_scrollbar(app: &mut SheetApp, ui: &mut egui::Ui, track: Rect, t: &Tokens) { + app.grid.hscroll = Some(track); + if track.height() <= 0.0 || track.width() <= 0.0 { + return; + } + let Some(geo) = app.grid.geo.clone() else { return }; + let Some(wb) = app.session.active().map(|d| d.wb.clone()) else { return }; + let Some(sh) = wb.active() else { return }; + let visible = ((geo.cells.width() - geo.frozen_w) / geo.z).max(0.0); + let scroll = app.view().scroll.x; + let last = sh.used_range().map(|r| r.end.col.saturating_add(10)).unwrap_or(26).clamp(26, MAX_COLS - 1); + let content = geo.scroll_left_of(sh, last); + let extent = (content - visible).max(scroll).max(0.0); + let id = ui.id().with("hscroll"); + let resp = ui.interact(track, id, Sense::click_and_drag()); + let drag_id = id.with("extent"); + if resp.drag_started() { + ui.ctx().data_mut(|d| d.insert_temp(drag_id, extent)); + } + let extent = if resp.dragged() { ui.ctx().data(|d| d.get_temp::(drag_id)).unwrap_or(extent) } else { extent }; + let thumb_w = (track.width() * visible / (extent + visible).max(1.0)).clamp(24.0_f32.min(track.width()), track.width()); + let travel = track.width() - thumb_w; + let offset = if extent > 0.0 { travel * (scroll / extent).clamp(0.0, 1.0) } else { 0.0 }; + // A right-to-left sheet scrolls towards the left, so its thumb starts at the right end. + let dir = if geo.rtl { -1.0 } else { 1.0 }; + let thumb_left = if geo.rtl { track.right() - thumb_w - offset } else { track.left() + offset }; + let thumb = Rect::from_min_size(pos2(thumb_left, track.top() + 3.0), vec2(thumb_w, (track.height() - 6.0).max(0.0))); + ui.painter().rect_filled(track, 0.0, t.tab_bar); + ui.painter().rect_filled(thumb, 4.0, if resp.hovered() || resp.dragged() { t.text_dim } else { t.text_disabled }); + let mut next = scroll; + if resp.dragged() && travel > 0.0 { + next += dir * resp.drag_delta().x / travel * extent; + } else if resp.clicked() + && let Some(p) = resp.interact_pointer_pos() + { + if p.x < thumb.left() { + next -= dir * visible; + } else if p.x > thumb.right() { + next += dir * visible; + } + } + if resp.hovered() { + let d = ui.input(|i| i.smooth_scroll_delta); + next -= (if d.x != 0.0 { dir * d.x } else { d.y }) / geo.z; + } + next = next.max(0.0); + if next != scroll { + if let Some(v) = app.view_mut() { + v.scroll.x = next; + } + ui.ctx().request_repaint(); + } +} + pub(crate) fn wrapped_cell_text_y(valign: VAlign, rect: Rect, gh: f32, z: f32) -> f32 { let pad_v = 1.5 * z; let y_top = rect.top() + pad_v; @@ -1047,17 +1228,24 @@ pub(crate) fn paint_quadrant( galley = p.layout_no_wrap(s, font.clone(), color); w = galley.size().x; } + // Center Across Selection: the text is centred over this cell and the empty cells to its right that also use it. + let across_end = if halign == HAlign::CenterAcross && !rotated && in_merge(c).is_none() { + center_across_end(c.col, c1 + 20, |col| { + let n = CellRef::new(c.row, col); + empty_ok(n) && wb.styles.get(sh.style_id(n)).align.h == HAlign::CenterAcross + }) + } else { + c.col + }; // Overflow into empty neighbours for text (not upright text, whose width is its line height). let mut clip = *rect; - if filter_space == 0.0 - && !numeric - && !matches!(v, Value::Number(_)) - && st.align.rotation.abs() != 90 - && w > rect.width() - 2.0 * pad - && in_merge(c).is_none() - { + let overflows = !numeric && !matches!(v, Value::Number(_)) && st.align.rotation.abs() != 90 && w > rect.width() - 2.0 * pad; + if filter_space == 0.0 && (overflows || across_end > c.col) && in_merge(c).is_none() { let mut right = rect.right(); let mut left = rect.left(); + if across_end > c.col { + right = geo.x(sh, across_end + 1); + } if matches!(halign, HAlign::Left | HAlign::Center | HAlign::Justify) { let mut cc = c.col + 1; while right - rect.left() < w + 2.0 * pad && cc < MAX_COLS && empty_ok(CellRef::new(c.row, cc)) && cc <= c1 + 20 { @@ -1087,7 +1275,8 @@ pub(crate) fn paint_quadrant( } } let x = match halign { - HAlign::Center | HAlign::CenterAcross | HAlign::Distributed => rect.center().x - w / 2.0, + HAlign::CenterAcross => clip.center().x - w / 2.0, + HAlign::Center | HAlign::Distributed => rect.center().x - w / 2.0, HAlign::Right => rect.right() - pad - w - indent, _ => text_left + pad + indent, }; @@ -2335,15 +2524,18 @@ fn delete_selected_object(app: &mut SheetApp, sh: &Sheet) -> bool { true } +/// A click on a hyperlink. References select; web and e-mail addresses ask first (the dialog's +/// OK runs `link.open`); every other kind is refused with a message. The decision is the +/// engine's ([`gridcraft_engine::links::classify`]), the same one `link.open` applies. pub fn follow_link(app: &mut SheetApp, target: &str) { - if target.contains("://") || target.starts_with("mailto:") { - if let Some(f) = &app.services.open_url { - f(target); + use gridcraft_engine::links::{LinkKind, classify}; + match classify(target) { + LinkKind::Internal(r) => { + let _ = app.session.run("edit.goTo", json!({"reference": r.trim_start_matches('#')})); + app.grid.ensure_visible = true; } - } else { - let t = target.trim_start_matches('#'); - let _ = app.session.run("edit.goTo", json!({"reference": t})); - app.grid.ensure_visible = true; + LinkKind::External(url) => app.open_dialog("openLink", json!({"url": url})), + LinkKind::Refused(why) => app.message = Some(("Hyperlink".into(), why)), } } @@ -3129,6 +3321,76 @@ mod selection_fill_tests { mod tests { use super::*; + #[test] + fn wheel_zoom_steps_along_the_ladder_and_stops_at_100() { + for (from, zoom_in, to) in [ + (100, true, 120), + (120, false, 100), + (100, false, 85), + // Between steps, the next one each way (Excel: 112% goes to 115% or 100%). + (112, true, 120), + (112, false, 100), + // A step less than 5% away is skipped… + (121, false, 100), + (86, false, 70), + (139, true, 170), + // …but never 100% or a limit. + (99, true, 100), + (101, false, 100), + (96, true, 100), + (385, true, 400), + (11, false, 10), + (400, true, 400), + (10, false, 10), + // Out-of-range zooms (from a file) come into range first. + (5, true, 12), + (1000, false, 340), + (0, false, 10), + (u16::MAX, true, 400), + ] { + assert_eq!(wheel_zoom_step(from, zoom_in), to, "{from}% {}", if zoom_in { "in" } else { "out" }); + } + // Every step on the ladder is 15–25% from the next, and a notch in then out returns. + // (The glide between steps is `zoom_glide_moves_by_ratio_and_always_arrives`.) + for w in WHEEL_ZOOM_LADDER.windows(2) { + let ratio = f32::from(w[1]) / f32::from(w[0]); + assert!((1.15..=1.25).contains(&ratio), "{w:?}"); + assert_eq!(wheel_zoom_step(w[0], true), w[1]); + assert_eq!(wheel_zoom_step(w[1], false), w[0]); + } + } + + #[test] + fn zoom_glide_moves_by_ratio_and_always_arrives() { + // A third of the way per frame, as at 60 frames a second. + for (from, to) in [(100, 120), (120, 100), (10, 12), (340, 400), (400, 10), (85, 100)] { + let (mut z, mut frames) = (from, 0); + while z != to { + let next = zoom_glide_step(z, to, 0.32); + assert!(next != z && next.abs_diff(to) < z.abs_diff(to), "{from} → {to}: {z} then {next}"); + z = next; + frames += 1; + assert!(frames < 40, "{from} → {to} arrives"); + } + assert!(frames >= 2, "{from} → {to} glides over more than one frame ({frames})"); + } + assert_eq!(zoom_glide_step(100, 120, 0.32), 106, "a third of the way by ratio"); + assert_eq!(zoom_glide_step(120, 100, 0.32), 113); + // No progress asked or nonsense: still a percent closer, or straight there. + assert_eq!(zoom_glide_step(100, 120, 0.0), 101); + assert_eq!(zoom_glide_step(120, 100, f32::NAN), 100); + assert_eq!(zoom_glide_step(100, 120, 5.0), 120); + assert_eq!(zoom_glide_step(120, 120, 0.5), 120); + assert_eq!(zoom_glide_step(0, 10, 0.5), 3); + } + + #[test] + fn center_across_run_stops_at_first_non_continuing_column() { + assert_eq!(center_across_end(0, 20, |c| c <= 2), 2); + assert_eq!(center_across_end(0, 20, |_| false), 0); + assert_eq!(center_across_end(0, 1, |_| true), 1); + } + #[test] fn filter_menu_uses_the_table_whose_header_was_clicked() { use gridcraft_engine::model::Table; diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 576e004..9c6d89e 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2117,6 +2117,7 @@ fn view(app: &mut SheetApp, ui: &mut Ui) { let lang = app.ui.language; let page_layout = app.session.active().and_then(|d| d.wb.active().map(|s| s.view == gridcraft_engine::model::SheetViewMode::PageLayout)).unwrap_or(false); + // Two columns of two, as in Excel's Show group: four stacked rows overflow the ribbon body. ui.vertical(|ui| { // Rulers exist only in Page Layout view. let mut ruler = !app.session.hide_ruler; @@ -2125,18 +2126,21 @@ fn view(app: &mut SheetApp, ui: &mut Ui) { if ruler_resp.changed() { act(app, "view.ruler", json!({"on": ruler})); } - let mut fb = app.session.prefs.formula_bar; - let resp = ui.checkbox(&mut fb, lang.tr("Formula Bar")); - control_keytip(app, ui, &resp, "view.formulaBar"); - if resp.changed() || invoke(app, "view.formulaBar") { - act(app, "view.formulaBar", json!({"on": checkbox_on(resp.changed(), fb)})); - } let mut g = s.0; let resp = ui.checkbox(&mut g, lang.tr("Gridlines")); control_keytip(app, ui, &resp, "view.gridlines"); if resp.changed() { act(app, "view.gridlines", json!({"on": g})); } + }); + ui.add_space(6.0); + ui.vertical(|ui| { + let mut fb = app.session.prefs.formula_bar; + let resp = ui.checkbox(&mut fb, lang.tr("Formula Bar")); + control_keytip(app, ui, &resp, "view.formulaBar"); + if resp.changed() || invoke(app, "view.formulaBar") { + act(app, "view.formulaBar", json!({"on": checkbox_on(resp.changed(), fb)})); + } let mut h = s.1; let resp = ui.checkbox(&mut h, lang.tr("Headings")); control_keytip(app, ui, &resp, "view.headings"); diff --git a/crates/ui-egui/src/tabs.rs b/crates/ui-egui/src/tabs.rs index f9669d6..b77524b 100644 --- a/crates/ui-egui/src/tabs.rs +++ b/crates/ui-egui/src/tabs.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, Ui, pos2, vec2}; +use egui::{Align2, Rect, Sense, Stroke, StrokeKind, Ui, pos2, vec2}; use gridcraft_engine::model::{Sheet, Visibility}; use serde_json::json; @@ -68,9 +68,9 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) { .filter(|(_, s)| s.visibility == Visibility::Visible) .map(|(i, s)| (i, s.name.clone(), s.tab_color.and_then(|c| c.resolve(&d.wb.theme)), i == d.wb.active_sheet)) .collect(); - // Keep worksheet tabs in their own horizontally scrollable - // viewport, leaving room for the New Sheet button and grid scrollbar. - let tabs_width = (ui.available_width() - 180.0).max(100.0); + // Keep worksheet tabs in their own horizontally scrollable viewport. As in Excel, the + // tabs get 60% of the bar and the grid's horizontal scrollbar the rest. + let tabs_width = (ui.available_width() * 0.6).max(100.0); // Reveal the selected sheet after navigation or layout changes, but // allow browsing other tabs without snapping back on every frame. let reveal_key = egui::Id::new((d.uid, d.wb.active_sheet, &sheets, tabs_width.to_bits())); @@ -215,38 +215,11 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) { if crate::widgets::icon_button(ui, Icon::Plus, t.text_dim, "New sheet (⇧F11)", vec2(26.0, 26.0)).clicked() { app.run_or_alert("home.insertSheet", json!({})); } - // Horizontal position indicator (a slim scrollbar). + // The track ends where the grid does, so it stops under the vertical scrollbar. let avail = ui.available_rect_before_wrap(); - if avail.width() > 120.0 { - let bar = Rect::from_min_max(pos2(avail.left() + 40.0, avail.center().y - 4.0), pos2(avail.right() - 16.0, avail.center().y + 4.0)); - let used_w = app - .session - .active() - .and_then(|d| d.wb.active().and_then(|s| s.used_range().map(|u| s.col_left(u.end.col + 10) as f32))) - .unwrap_or(2000.0) - .max(2000.0); - let view = app.view(); - let vis = app.grid.cells_rect.map(|r| r.width()).unwrap_or(1000.0); - let total = used_w.max(view.scroll.x + vis); - let frac0 = (view.scroll.x / total).clamp(0.0, 1.0); - let frac1 = ((view.scroll.x + vis) / total).clamp(0.0, 1.0); - let thumb = Rect::from_min_max( - pos2(bar.left() + bar.width() * frac0, bar.top()), - pos2(bar.left() + bar.width() * frac1.max(frac0 + 0.03), bar.bottom()), - ); - let resp = ui.interact(bar, ui.id().with("hscroll"), Sense::click_and_drag()); - ui.painter().rect_filled( - thumb, - 4.0, - if resp.hovered() || resp.dragged() { Color32::from_gray(150) } else { Color32::from_gray(190) }, - ); - if resp.dragged() { - let dx = resp.drag_delta().x / bar.width() * total; - if let Some(v) = app.view_mut() { - v.scroll.x = (v.scroll.x + dx).max(0.0); - } - } - } + let right = app.grid.rect.map_or(avail.right(), |r| r.right()).min(avail.right()); + let track = Rect::from_min_max(pos2(avail.left() + 8.0, avail.center().y - 7.0), pos2(right, avail.center().y + 7.0)); + crate::grid::horizontal_scrollbar(app, ui, track, &t); }); }); } diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index b7b66bd..0958c8c 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -58,6 +58,72 @@ fn vertical_scrollbar_pages_and_drags_without_changing_selection() { assert_eq!(h.state().session.active().unwrap().selection.active, CellRef::new(0, 0)); } +/// The first column painted right of the frozen ones. +fn first_scrolled_col(h: &egui_kittest::Harness<'static, SheetApp>) -> u32 { + let app = h.state(); + let sheet = app.session.active().unwrap().wb.active().unwrap(); + let geo = app.grid.geo.as_ref().unwrap(); + geo.col_at(sheet, geo.cells.left() + geo.frozen_w + 0.5) +} + +fn click(h: &mut egui_kittest::Harness<'static, SheetApp>, p: egui::Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.run_steps(2); + h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); +} + +#[test] +fn horizontal_scrollbar_pages_and_drags_across_a_wide_sheet() { + let mut wb = gridcraft_engine::model::Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + for c in 0..26 { + sh.set_value(CellRef::new(0, c), Value::text(format!("col {}", c + 1))); + } + // Column A frozen: a page is the scrolling part only. + sh.freeze = Some((0, 1)); + let mut s = Session::new(); + s.add_document(gridcraft_engine::DocState::new(wb, None, "Book1".into())); + let mut h = harness(s); + let track = h.state().grid.hscroll.expect("the tab bar has a horizontal scrollbar"); + assert!(track.width() >= 200.0, "the scrollbar gets a usable share of the tab bar: {track:?}"); + assert_eq!(first_scrolled_col(&h), 1); + + let page = { + let geo = h.state().grid.geo.as_ref().unwrap(); + (geo.cells.width() - geo.frozen_w) / geo.z + }; + click(&mut h, egui::pos2(track.right() - 6.0, track.center().y)); + let scrolled = h.state().view().scroll.x; + assert!((scrolled - page).abs() < 0.5, "clicking the track right of the thumb scrolls one page: {scrolled} vs {page}"); + assert!(first_scrolled_col(&h) > 1); + let geo = h.state().grid.geo.clone().unwrap(); + let sheet = h.state().session.active().unwrap().wb.active().unwrap().clone(); + assert_eq!(geo.x(&sheet, 0), geo.cells.left(), "the frozen column stays put"); + click(&mut h, egui::pos2(track.right() - 6.0, track.center().y)); + let further = h.state().view().scroll.x; + click(&mut h, egui::pos2(track.left() + 2.0, track.center().y)); + assert!(h.state().view().scroll.x < further, "clicking left of the thumb scrolls back"); + + h.state_mut().view_mut().unwrap().scroll.x = 0.0; + h.run_steps(2); + let start = egui::pos2(track.left() + 6.0, track.center().y); + h.input_mut().events.push(Event::PointerMoved(start)); + h.input_mut().events.push(Event::PointerButton { pos: start, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.run_steps(2); + let end = start + egui::vec2(120.0, 0.0); + h.input_mut().events.push(Event::PointerMoved(end)); + h.run_steps(2); + let dragged = h.state().view().scroll.x; + assert!(first_scrolled_col(&h) > 1, "dragging the thumb right changes the first visible column"); + h.run_steps(3); + assert_eq!(h.state().view().scroll.x, dragged, "holding the thumb still must not keep scrolling"); + h.input_mut().events.push(Event::PointerButton { pos: end, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); + assert_eq!(h.state().session.active().unwrap().selection.active, CellRef::new(0, 0)); +} + fn key(h: &mut egui_kittest::Harness<'static, SheetApp>, k: Key, m: Modifiers) { h.input_mut().events.push(Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: m }); h.input_mut().events.push(Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers: m }); diff --git a/crates/ui-egui/tests/sheet_tabs.rs b/crates/ui-egui/tests/sheet_tabs.rs index e799f7a..7b43cd1 100644 --- a/crates/ui-egui/tests/sheet_tabs.rs +++ b/crates/ui-egui/tests/sheet_tabs.rs @@ -1,5 +1,5 @@ //! Overflowing worksheet tabs, exercised through their actual painted geometry and input. -use egui::{Color32, Event, Modifiers, PointerButton, Rect, Shape, pos2, vec2}; +use egui::{Event, Modifiers, PointerButton, Rect, Shape, pos2, vec2}; use gridcraft_engine::Session; use gridcraft_ui_egui::SheetApp; use serde_json::json; @@ -153,26 +153,12 @@ fn new_sheet_button_stays_reachable_and_reveals_the_inserted_tab() { #[test] fn overflow_leaves_the_worksheet_scrollbar_reachable() { + // The tab bar's horizontal scrollbar track keeps its share of the bar however many tabs + // overflow; dragging its thumb is covered with a rendered grid in `interaction.rs`. for width in [500.0, 700.0, 1200.0] { - let mut h = harness(width, 0); - let (bar, clip) = h - .output() - .shapes - .iter() - .find_map(|s| match &s.shape { - Shape::Rect(r) if r.fill == Color32::from_gray(190) && r.rect.height() == 8.0 => Some((r.rect, s.clip_rect)), - _ => None, - }) - .expect("worksheet scrollbar thumb must be painted"); - assert!(clip.contains_rect(bar), "worksheet scrollbar is offscreen: {bar:?}, {clip:?}"); - let at = bar.center(); - h.input_mut().events.extend([ - Event::PointerMoved(at), - Event::PointerButton { pos: at, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }, - ]); - h.step(); - h.input_mut().events.push(Event::PointerMoved(at + vec2(12.0, 0.0))); - h.step(); - assert!(h.state().view().scroll.x > 0.0, "worksheet scrollbar must respond to dragging"); + let h = harness(width, 0); + let track = h.state().grid.hscroll.expect("the tab bar has a horizontal scrollbar"); + assert!(track.width() >= 100.0, "{width}: the scrollbar keeps a usable width: {track:?}"); + assert!(track.left() >= 0.0 && track.right() <= width + 0.5, "{width}: the scrollbar is offscreen: {track:?}"); } } diff --git a/crates/ui-egui/tests/wheel_zoom.rs b/crates/ui-egui/tests/wheel_zoom.rs new file mode 100644 index 0000000..397fb6b --- /dev/null +++ b/crates/ui-egui/tests/wheel_zoom.rs @@ -0,0 +1,186 @@ +//! Ctrl+wheel zoom steps along a ladder of percentages that always comes back to 100%. + +use egui::{Event, Modifiers, MouseWheelUnit, TouchPhase, vec2}; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut session = Session::new(); + session.new_workbook(); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).with_step_dt(1.0 / 60.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); + let over_grid = h.state().grid.rect.unwrap().center(); + h.input_mut().events.push(Event::PointerMoved(over_grid)); + h.run_steps(2); + h +} + +fn zoom(h: &Harness) -> u16 { + h.state().session.active().unwrap().wb.active().unwrap().zoom +} + +fn set_zoom(h: &mut Harness, percent: u16) { + h.state_mut().run("view.zoom", json!({"percent": percent})).unwrap(); + h.run_steps(2); +} + +/// Turns the wheel `turns` notches at a time with Ctrl held (up zooms in), each turn `frames` +/// frames after the last, then lets egui's easing play out. +fn wheel_turns(h: &mut Harness, turns: &[f32], frames: usize) { + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::COMMAND)); + for &t in turns { + let delta = vec2(0.0, t); + h.input_mut().events.push(Event::MouseWheel { unit: MouseWheelUnit::Line, delta, phase: TouchPhase::Move, modifiers: Modifiers::COMMAND }); + h.run_steps(frames); + } + h.run_steps(60); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); + h.run_steps(2); +} + +fn wheel(h: &mut Harness, notches: f32) { + wheel_turns(h, &[notches], 1); +} + +/// The harness turns egui's animations off (and the zoom glide follows that); this turns them on. +fn animated() -> Harness { + let mut h = harness(); + h.ctx.all_styles_mut(|s| s.animation_time = 0.1); + h.step(); + h +} + +/// Turns the wheel `notches` with Ctrl held and records the zoom over the next `frames` frames. +fn turn_and_watch(h: &mut Harness, notches: f32, frames: usize) -> Vec { + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::COMMAND)); + let delta = vec2(0.0, notches); + h.input_mut().events.push(Event::MouseWheel { unit: MouseWheelUnit::Line, delta, phase: TouchPhase::Move, modifiers: Modifiers::COMMAND }); + (0..frames) + .map(|_| { + h.step(); + zoom(h) + }) + .collect() +} + +#[test] +fn with_animations_the_zoom_glides_to_the_step() { + let mut h = animated(); + let seen = turn_and_watch(&mut h, 1.0, 40); + assert!(seen.windows(2).all(|w| w[0] <= w[1]), "only ever towards the step: {seen:?}"); + let between = seen.iter().filter(|&&z| z > 100 && z < 120).count(); + assert!(between >= 3, "on its way over several frames: {seen:?}"); + assert_eq!(seen.last(), Some(&120), "and exactly on the step: {seen:?}"); + let seen = turn_and_watch(&mut h, -1.0, 40); + assert!(seen.windows(2).all(|w| w[0] >= w[1]) && seen.iter().any(|&z| z > 100 && z < 120), "{seen:?}"); + assert_eq!(seen.last(), Some(&100), "{seen:?}"); +} + +#[test] +fn a_notch_during_a_glide_steps_on_from_where_it_is_going() { + let mut h = animated(); + let seen = turn_and_watch(&mut h, 1.0, 6); + assert!(seen.last().is_some_and(|&z| z > 100 && z < 120), "still gliding: {seen:?}"); + turn_and_watch(&mut h, 1.0, 60); + assert_eq!(zoom(&h), 140); + let seen = turn_and_watch(&mut h, -1.0, 6); + assert!(seen.last().is_some_and(|&z| z > 120 && z < 140), "{seen:?}"); + turn_and_watch(&mut h, -1.0, 60); + assert_eq!(zoom(&h), 100); +} + +#[test] +fn a_zoom_set_another_way_stops_a_glide() { + let mut h = animated(); + let seen = turn_and_watch(&mut h, 1.0, 6); + assert!(seen.last().is_some_and(|&z| z > 100 && z < 120), "{seen:?}"); + h.state_mut().run("view.zoom", json!({"percent": 150})).unwrap(); + h.run_steps(60); + assert_eq!(zoom(&h), 150); + // Setting the percentage the glide had got to doesn't start it again. + let stopped_at = seen.last().copied().unwrap(); + set_zoom(&mut h, stopped_at); + h.run_steps(60); + assert_eq!(zoom(&h), stopped_at); +} + +#[test] +fn with_view_animations_off_a_notch_steps_at_once() { + let mut h = animated(); + h.state_mut().run("view.animations", json!({"on": false})).unwrap(); + let seen = turn_and_watch(&mut h, 1.0, 40); + assert!(seen.iter().all(|&z| z == 100 || z == 120), "no percentages in between: {seen:?}"); + assert_eq!(seen.last(), Some(&120)); +} + +#[test] +fn a_notch_out_returns_to_the_zoom_before_the_notch_in() { + let mut h = harness(); + assert_eq!(zoom(&h), 100); + // Multiplying the rounded zoom by egui's eased factor every frame drifted: 100 → 122 → 101 → 123 → 102. + for _ in 0..4 { + wheel(&mut h, 1.0); + assert_eq!(zoom(&h), 120); + wheel(&mut h, -1.0); + assert_eq!(zoom(&h), 100); + } + for expect in [120, 140, 170] { + wheel(&mut h, 1.0); + assert_eq!(zoom(&h), expect); + } + wheel(&mut h, -3.0); + assert_eq!(zoom(&h), 100); + wheel(&mut h, -2.0); + assert_eq!(zoom(&h), 70); + // Notches turned quickly, each before the last one has eased in. + wheel_turns(&mut h, &[1.0, 1.0, 1.0, 1.0], 3); + assert_eq!(zoom(&h), 140); + wheel_turns(&mut h, &[-1.0, -1.0], 3); + assert_eq!(zoom(&h), 100); +} + +#[test] +fn parts_of_a_notch_add_up_to_whole_steps() { + // A fine-grained wheel (or a Windows touchpad pinch) sends a notch in pieces, not always evenly. + let mut h = harness(); + wheel_turns(&mut h, &[0.25, 0.25, 0.25], 4); + assert_eq!(zoom(&h), 120, "three quarters of a notch step"); + wheel_turns(&mut h, &[-0.3, -0.4, -0.3], 4); + assert_eq!(zoom(&h), 100); + wheel_turns(&mut h, &[0.25, 0.25, 0.25, 0.25, 0.3, 0.4, 0.3], 4); + assert_eq!(zoom(&h), 140, "one step per notch, however it's divided"); + wheel_turns(&mut h, &[-0.5, -0.25, -0.25, -0.25, -0.25, -0.5], 4); + assert_eq!(zoom(&h), 100); + wheel_turns(&mut h, &[0.5], 4); + assert_eq!(zoom(&h), 100, "half a notch doesn't step"); + // What's left of a notch is forgotten after a pause (`wheel_turns` waits a second) rather + // than adding up with the next turn. + wheel_turns(&mut h, &[0.4], 4); + assert_eq!(zoom(&h), 100); +} + +#[test] +fn from_a_zoom_set_another_way_a_notch_goes_to_the_next_step_or_100() { + let mut h = harness(); + for (from, notches, to) in + [(112, 1.0, 120), (112, -1.0, 100), (121, -1.0, 100), (99, 1.0, 100), (101, -1.0, 100), (385, 1.0, 400), (400, 1.0, 400), (11, -1.0, 10)] + { + set_zoom(&mut h, from); + wheel(&mut h, notches); + assert_eq!(zoom(&h), to, "{from}% by {notches}"); + } + // Two notches out from 121%: 100%, then 85%. + set_zoom(&mut h, 121); + wheel(&mut h, -2.0); + assert_eq!(zoom(&h), 85); +} diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 6fd80d9..3300f97 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -47,6 +47,7 @@ use gridcraft_model::Workbook; pub use crypto::{DecryptError, check_encryption, decrypt}; pub use csv::{MAX_CSV_BYTES, read_csv, write_csv, write_csv_limited}; pub use ods::read_ods; +pub use passthrough::rebase_links; pub use xls::read_xls; pub use xlsb::read_xlsb; diff --git a/crates/xlsx/src/passthrough.rs b/crates/xlsx/src/passthrough.rs index 7db21fd..8af5583 100644 --- a/crates/xlsx/src/passthrough.rs +++ b/crates/xlsx/src/passthrough.rs @@ -15,7 +15,8 @@ //! drawings, charts, tables, PivotTables, comments, slicers…) is not kept: it would point at //! content that no longer exists in the same form. //! - External relationship targets (the path of a linked workbook) are kept as text and never -//! opened, followed or refreshed. +//! opened, followed or refreshed. A relative path to a linked workbook is rewritten when the +//! workbook is saved into another folder ([`rebase_links`]), so it still names the same file. //! - Sheet content lives on the sheet: deleting the sheet drops it, Copy Sheet doesn't copy it. //! Cell references inside kept parts (a control's linked cell, a query table's range) are kept //! as they were, not adjusted to inserted or deleted rows. @@ -25,9 +26,10 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt::Write as _; +use std::path::{Component, Path}; use std::sync::Arc; -use gridcraft_model::{Preserved, PreservedPart, PreservedRel, PreservedXml, Sheet, Workbook}; +use gridcraft_model::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml, Sheet, Visibility, Workbook}; use quick_xml::events::attributes::Attribute; use quick_xml::events::{BytesStart, Event}; use quick_xml::name::QName; @@ -137,6 +139,10 @@ const REWRITTEN: &[&str] = &[ "powerpivotdata", ]; +/// Relationship kinds a chart sheet reaches that are otherwise rewritten: its drawing, chart and +/// their style, colour and shape parts are kept byte for byte (the chart sheet is not modelled). +const CHART_SHEET_GRAPH: &[&str] = &["chartsheet", "drawing", "chart", "chartex", "chartstyle", "chartcolorstyle", "chartusershapes"]; + /// Part-name prefixes (lower case) GridCraft's writer never produces: kept parts there keep their /// original names. Anything else is renamed so it can't collide with written parts. const OWN_DIRS: &[&str] = &[ @@ -201,6 +207,21 @@ fn raw_rels(cx: &mut Ctx<'_>, part: &str) -> Result, String> { // ---------------------------------------------------------------- reading +/// The `` entries of the workbook that are chart sheets, each with the number of worksheets +/// (model sheets) before it. +fn chart_sheet_entries<'a>(wb_xml: &'a xml::El, wb_rels: &[PreservedRel], file_to_model: &[Option]) -> Vec<(usize, &'a xml::El)> { + let mut out = vec![]; + let mut worksheets = 0usize; + for (i, se) in wb_xml.child("sheets").into_iter().flat_map(|s| s.kids("sheet")).enumerate() { + if file_to_model.get(i).copied().flatten().is_some() { + worksheets += 1; + } else if se.attr("id").and_then(|id| wb_rels.iter().find(|r| r.id == id)).is_some_and(|r| is_kind(r, &["chartsheet"])) { + out.push((worksheets, se)); + } + } + out +} + /// Collects kept content while a package is read. pub struct KeptReader { on: bool, @@ -257,11 +278,12 @@ impl KeptReader { /// The parts reachable from `root` that aren't in `seen` yet. Fails (and keeps nothing) when the /// graph reaches rewritten content, a missing or damaged part, or a size limit. - fn closure(&mut self, cx: &mut Ctx<'_>, root: &PreservedRel, seen: &HashSet) -> Result, String> { + fn closure(&mut self, cx: &mut Ctx<'_>, root: &PreservedRel, seen: &HashSet, through: &[&str]) -> Result, String> { if root.external { return Ok(vec![]); } - if rewritten(root) { + let blocked = |r: &PreservedRel| rewritten(r) && !through.iter().any(|k| k.eq_ignore_ascii_case(kind(&r.rel_type))); + if blocked(root) { return Err(format!("it refers to {}", kind(&root.rel_type))); } let mut parts = vec![]; @@ -291,7 +313,7 @@ impl KeptReader { } let rels = raw_rels(cx, &name)?; for r in rels.iter().filter(|r| !r.external) { - if rewritten(r) { + if blocked(r) { return Err(format!("it refers to {}", kind(&r.rel_type))); } queue.push_back((r.target.clone(), depth + 1)); @@ -305,10 +327,22 @@ impl KeptReader { /// Keeps a set of relationships (all or nothing) and the parts behind them. fn keep(&mut self, cx: &mut Ctx<'_>, p: &mut Preserved, roots: &[PreservedRel], seen: &mut HashSet) -> Result<(), String> { + self.keep_through(cx, p, roots, seen, &[]) + } + + /// [`Self::keep`] for roots whose graph may reach the rewritten kinds in `through`. + fn keep_through( + &mut self, + cx: &mut Ctx<'_>, + p: &mut Preserved, + roots: &[PreservedRel], + seen: &mut HashSet, + through: &[&str], + ) -> Result<(), String> { let mut new_parts: Vec = vec![]; let mut scratch = seen.clone(); for r in roots { - let parts = self.closure(cx, r, &scratch)?; + let parts = self.closure(cx, r, &scratch, through)?; scratch.extend(parts.iter().map(|q| normalize(&q.name))); new_parts.extend(parts); } @@ -323,8 +357,12 @@ impl KeptReader { } /// Package-root and workbook-level content. - pub fn workbook(&mut self, cx: &mut Ctx<'_>, wb_part: &str, wb_xml: &xml::El) -> Option> { + pub fn workbook(&mut self, cx: &mut Ctx<'_>, wb_part: &str, wb_xml: &xml::El, file_to_model: &[Option]) -> Option> { if !self.on { + let wb_rels = raw_rels(cx, wb_part).unwrap_or_default(); + for (_, se) in chart_sheet_entries(wb_xml, &wb_rels, file_to_model) { + cx.warn(format!("chart sheet \"{}\" was skipped and is not kept on save", se.attr("name").unwrap_or(""))); + } if self.strict { cx.warn("unmodelled content of a Strict Open XML workbook (custom XML, links, controls…) is not kept on save"); } @@ -351,6 +389,25 @@ impl KeptReader { cx.warn(format!("{} part {} was dropped: {why}", kind(&r.rel_type), r.target)); } } + // Chart sheets: the sheet entry (name, state, position) plus the chart sheet part and the + // drawing, chart, style and colour parts behind it. + for (worksheets, se) in chart_sheet_entries(wb_xml, &wb_rels, file_to_model) { + let name = se.attr("name").unwrap_or("").to_string(); + let Some(r) = se.attr("id").and_then(|id| wb_rels.iter().find(|r| r.id == id)) else { continue }; + match self.keep_through(cx, &mut p, std::slice::from_ref(r), &mut seen, CHART_SHEET_GRAPH) { + Ok(()) => p.sheets.push(PreservedSheet { + name, + visibility: match se.attr("state") { + Some("hidden") => Visibility::Hidden, + Some("veryHidden") => Visibility::VeryHidden, + _ => Visibility::Visible, + }, + rel_id: r.id.clone(), + before: worksheets, + }), + Err(why) => cx.warn(format!("chart sheet \"{name}\" was dropped: {why}")), + } + } // Links to external workbooks: `[1]` in formulas is the position in , // so they are kept all together or not at all. let ids: Vec<&str> = @@ -869,18 +926,165 @@ fn remap(x: &str, ids: &HashMap) -> Option { map_rel_ids(x, &mut |id| ids.get(id).cloned()) } -/// Writes the workbook's kept parts and relationships; returns the workbook XML elements that -/// go right after `` (``). -pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> String { - let Some(p) = wb.preserved.as_deref() else { return String::new() }; - let names = write_parts(out, &p.parts); - let ids = attach(wb_rels, &p.rels, &names); +/// A kept chart sheet being written: its `` entry's fields and its new relationship id. +pub struct KeptSheet { + before: usize, + name: String, + visibility: Visibility, + rid: String, +} + +/// The workbook's kept content as written. +#[derive(Default)] +pub struct WorkbookKept { + /// Workbook XML elements that go right after `` (``). + pub elements: String, + sheets: Vec, +} + +impl WorkbookKept { + /// Where the model's sheet `i` is in the file's sheet list: chart sheets come between worksheets. + pub fn file_index(&self, i: usize) -> usize { + i + self.sheets.iter().filter(|s| s.before <= i).count() + } + + /// The `` entries of the chart sheets that go right before the model's sheet `i` + /// (`i` = the number of sheets for those that come last); `id` numbers them from `first_id`. + pub fn sheet_entries(&self, i: usize, last: bool, mut id: usize) -> (String, usize) { + let mut x = String::new(); + for s in self.sheets.iter().filter(|s| if last { s.before >= i } else { s.before == i }) { + let state = match s.visibility { + Visibility::Visible => "", + Visibility::Hidden => " state=\"hidden\"", + Visibility::VeryHidden => " state=\"veryHidden\"", + }; + let name: String = s.name.chars().take(31).collect(); + let _ = write!(x, "", esc_attr(&name), esc_attr(&s.rid)); + id += 1; + } + (x, id) + } +} + +/// The part names reachable from `root` through kept parts' relationships (normalized). +fn reachable(p: &Preserved, root: &PreservedRel) -> HashSet { + let by_name: HashMap = p.parts.iter().map(|q| (normalize(&q.name), q)).collect(); + let mut seen = HashSet::new(); + let mut queue = VecDeque::from([normalize(&root.target)]); + while let Some(n) = queue.pop_front() { + if !seen.insert(n.clone()) { + continue; + } + if let Some(q) = by_name.get(&n) { + queue.extend(q.rels.iter().filter(|r| !r.external).map(|r| normalize(&r.target))); + } + } + seen +} + +/// Every formula (``) of a chart part, depth first. +fn chart_formulas(el: &xml::El, out: &mut Vec) { + if el.name == "f" && !el.text.trim().is_empty() { + out.push(el.text.trim().to_string()); + } + for c in &el.children { + chart_formulas(c, out); + } +} + +/// The sheet names a formula such as `'My Sheet'!$A$1:$A$9,Data!$B$1` refers to. References to +/// other workbooks (`[1]Sheet!A1`) are not sheets of this one and are left out. +pub(crate) fn sheet_names_in(formula: &str) -> Vec { + let mut names = vec![]; + let chars: Vec = formula.chars().collect(); + let at = |i: usize| chars.get(i).copied(); + let mut i = 0; + while let Some(c) = at(i) { + let mut name = String::new(); + let start = i; + if c == '\'' { + i += 1; + while let Some(c) = at(i) { + if c == '\'' { + if at(i + 1) == Some('\'') { + name.push('\''); + i += 2; + continue; + } + break; + } + name.push(c); + i += 1; + } + i += 1; + } else { + while let Some(c) = at(i).filter(|c| !matches!(c, '!' | ',' | '(' | ')' | '\'' | ' ')) { + name.push(c); + i += 1; + } + } + if at(i) == Some('!') { + if !name.is_empty() && !name.starts_with('[') { + names.push(name); + } + i += 1; + } else if i == start { + i += 1; + } + } + names +} + +/// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets +/// that exist under the same names (the chart is kept as it was, so a series on a deleted or +/// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name. +fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool { + let same = |a: &str, b: &str| a.chars().take(31).collect::().eq_ignore_ascii_case(&b.chars().take(31).collect::()); + if wb.sheets.iter().any(|w| same(&w.name, &s.name)) { + return false; + } + let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false }; + let parts = reachable(p, root); + let mut formulas = vec![]; + for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) { + if let Ok(x) = xml::parse(&q.data) { + chart_formulas(&x, &mut formulas); + } + } + formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n))) +} + +/// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back +/// (see [`chart_sheet_usable`]) are left out with the parts only they reach. +pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept { + let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() }; + let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s)); + let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id); + let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect(); + // Parts reached only by dropped chart sheets go with them. + let mut gone: HashSet = dropped.iter().filter_map(|s| root_of(s)).flat_map(|r| reachable(p, r)).collect(); + if !gone.is_empty() { + let others = p.rels.iter().filter(|r| !dropped_ids.contains(r.id.as_str())).flat_map(|r| reachable(p, r)); + for n in others { + gone.remove(&n); + } + } + let parts: Vec = p.parts.iter().filter(|q| !gone.contains(&normalize(&q.name))).cloned().collect(); + let rels: Vec = p.rels.iter().filter(|r| !dropped_ids.contains(r.id.as_str())).cloned().collect(); + let names = write_parts(out, &parts); + let ids = attach(wb_rels, &rels, &names); for r in &p.package_rels { if let Some(target) = rel_target(r, &names) { out.kept.root.push((r.rel_type.clone(), target, r.external)); } } - p.elements.iter().filter(|e| e.slot == "externalReferences").filter_map(|e| remap(&e.xml, &ids)).collect() + WorkbookKept { + elements: p.elements.iter().filter(|e| e.slot == "externalReferences").filter_map(|e| remap(&e.xml, &ids)).collect(), + sheets: live + .iter() + .filter_map(|s| Some(KeptSheet { before: s.before, name: s.name.clone(), visibility: s.visibility, rid: ids.get(&s.rel_id)?.clone() })) + .collect(), + } } /// The workbook's kept elements of `slot` that refer to no part (`fileVersion`, `ext`). @@ -958,3 +1162,136 @@ impl<'a> SheetKept<'a> { s } } + +// ---------------------------------------------------------------- linked workbooks + +/// `wb` as it is saved into the folder `to` (absolute): the relative paths of its linked +/// workbooks, relative to the folder it was read from, are rewritten relative to `to`, as Excel +/// does on Save As, or made absolute when no relative path exists (another drive). `None` when +/// nothing changes. +pub fn rebase_links(wb: &Workbook, to: &Path) -> Option { + let p = wb.preserved.as_deref()?; + let from = Folder::of(wb.source_dir.as_deref()?)?; + let to = Folder::of(to)?; + if from == to { + return None; + } + let mut parts = p.parts.clone(); + let mut changed = false; + for r in parts.iter_mut().flat_map(|part| part.rels.iter_mut()) { + if r.external + && is_kind(r, &["externalLinkPath"]) + && let Some(target) = rebase(&r.target, &from, &to) + { + r.target = target; + changed = true; + } + } + if !changed { + return None; + } + let mut out = wb.clone(); + out.preserved = Some(Arc::new(Preserved { parts, ..p.clone() })); + Some(out) +} + +/// An absolute folder, lexically normalized. +#[derive(PartialEq)] +struct Folder { + /// Drive or share and root separator (`/`, `C:\`, `\\server\share\`). + root: String, + names: Vec, +} + +impl Folder { + fn of(dir: &Path) -> Option { + if !dir.is_absolute() { + return None; + } + let mut f = Folder { root: String::new(), names: vec![] }; + for c in dir.components() { + match c { + Component::Prefix(_) | Component::RootDir => f.root.push_str(&c.as_os_str().to_string_lossy()), + Component::CurDir => {} + Component::ParentDir => { + f.names.pop(); + } + Component::Normal(n) => f.names.push(n.to_string_lossy().into_owned()), + } + } + Some(f) + } +} + +/// A relative link target read from `from`, rewritten for `to`. `None` keeps it as it is: an +/// absolute path or URL, or a path that climbs above the root. +fn rebase(target: &str, from: &Folder, to: &Folder) -> Option { + if !is_relative_path(target) { + return None; + } + // Each name as the file system knows it, and as it is written in a target. Names taken from + // the original target keep their spelling. + let mut path: Vec<(String, String)> = from.names.iter().map(|n| (n.clone(), encode_name(n))).collect(); + for seg in target.split(['/', '\\']) { + match seg { + "" | "." => {} + ".." => { + path.pop()?; + } + _ => path.push((decode_name(seg), seg.to_string())), + } + } + let folders = path.len().checked_sub(1)?; + if !from.root.eq_ignore_ascii_case(&to.root) { + let sep = if from.root.contains('\\') { "\\" } else { "/" }; + let names: Vec<&str> = path.iter().map(|(name, _)| name.as_str()).collect(); + return Some(format!("file:///{}{}", from.root, names.join(sep))); + } + let common = to.names.iter().zip(&path).take(folders).take_while(|(a, (b, _))| *a == b).count(); + let up = "../".repeat(to.names.len().saturating_sub(common)); + let down: Vec<&str> = path.iter().skip(common).map(|(_, written)| written.as_str()).collect(); + Some(format!("{up}{}", down.join("/"))) +} + +/// Not empty, no root, no scheme or drive (`file:`, `http:`, `C:`). +fn is_relative_path(target: &str) -> bool { + let scheme = target.split_once(':').is_some_and(|(s, _)| { + s.starts_with(|c: char| c.is_ascii_alphabetic()) && s.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) + }); + !target.is_empty() && !target.starts_with(['/', '\\']) && !scheme +} + +/// A folder name as a segment of a relationship target (a URI): ASCII characters a URI path +/// can't hold are percent-encoded, other characters are written as they are. +fn encode_name(name: &str) -> String { + let mut s = String::with_capacity(name.len()); + for c in name.chars() { + if !c.is_ascii() || c.is_ascii_alphanumeric() || "-._~!$&'()+,;=@".contains(c) { + s.push(c); + } else { + let _ = write!(s, "%{:02X}", c as u32); + } + } + s +} + +/// A target segment with its percent-escapes decoded; as written when they don't decode to UTF-8. +fn decode_name(seg: &str) -> String { + let b = seg.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let mut i = 0; + while let Some(&c) = b.get(i) { + let hex = b.get(i + 1..i + 3).and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u8::from_str_radix(h, 16).ok()); + match hex { + Some(byte) if c == b'%' => { + out.push(byte); + i += 3; + } + _ => { + out.push(c); + i += 1; + } + } + } + String::from_utf8(out).unwrap_or_else(|_| seg.to_string()) +} diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index ed7996a..1a47908 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -208,6 +208,11 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { file_to_model.push(None); continue; }; + if rel.kind == "chartsheet" { + // Kept as it is and written back in place (see `passthrough`); nothing to model. + file_to_model.push(None); + continue; + } if rel.kind != "worksheet" { cx.warn(format!("sheet \"{name}\" is a {} and was skipped", rel.kind)); file_to_model.push(None); @@ -334,7 +339,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { Err(e) => cx.warn(format!("macros (VBA project) could not be read and were dropped: {e}")), } } - wb.preserved = kept.workbook(&mut cx, &wb_part, &wb_xml); + wb.preserved = kept.workbook(&mut cx, &wb_part, &wb_xml, &file_to_model); // Calls of LAMBDA names are spelled like the names (the parser upper-cases them). wb.apply_name_call_case(); wb.styles = cx.styles; diff --git a/crates/xlsx/src/tests/malformed.rs b/crates/xlsx/src/tests/malformed.rs index 4700f8b..5e47c1f 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -194,6 +194,24 @@ fn hostile_protected_ranges_stay_locked() { } } +#[test] +fn an_overlong_number_format_code_does_not_stop_the_open() { + // A format code far over the 255-character cap: the file still opens, the format code is kept as + // stored (display falls back to General) and the cell keeps its value. + let code = format!("# ?/{}", "0".repeat(65_536)); + let styles = format!( + r#" + "# + ); + let rels = r#""#; + let body = r#"0.5"#; + let (wb, _) = read_xlsx(&minimal(body, &[("xl/styles.xml", &styles)], rels, "")).unwrap(); + let s = wb.sheet(0).unwrap(); + let cell = s.cell(gridcraft_core::CellRef::parse("A1").unwrap()).unwrap(); + assert_eq!(s.value(gridcraft_core::CellRef::parse("A1").unwrap()), gridcraft_core::Value::Number(0.5)); + assert_eq!(wb.styles.get(cell.style).num_fmt.as_str(), code); +} + #[test] fn hyperlinks_past_the_cell_cap_are_reported() { // A styles part, so the only possible warning is the hyperlink one. diff --git a/crates/xlsx/src/tests/passthrough_tests.rs b/crates/xlsx/src/tests/passthrough_tests.rs index aac9b6b..51d37ab 100644 --- a/crates/xlsx/src/tests/passthrough_tests.rs +++ b/crates/xlsx/src/tests/passthrough_tests.rs @@ -484,6 +484,234 @@ fn kept_vml_drawings_are_charged_to_the_kept_total() { assert!(wb.sheets.iter().all(|s| s.preserved.as_ref().is_some_and(|p| p.vml.is_some()))); } +/// Where a link with `target`, in a workbook read from `from`, points once saved into `to`. +#[cfg(unix)] +fn rebased(target: &str, from: &str, to: &str) -> String { + let link = |rel_type: &str| gridcraft_model::PreservedRel { + id: "rId1".into(), + rel_type: format!("{NS_R}/{rel_type}"), + target: target.into(), + external: true, + }; + let part = gridcraft_model::PreservedPart { + name: "xl/externalLinks/externalLink1.xml".into(), + content_type: String::new(), + data: Arc::from(&b""[..]), + rels: vec![link("externalLinkPath"), link("hyperlink")], + }; + let mut wb = gridcraft_model::Workbook::new(); + wb.preserved = Some(Arc::new(gridcraft_model::Preserved { parts: vec![part], ..Default::default() })); + wb.source_dir = Some(from.into()); + let Some(out) = crate::rebase_links(&wb, std::path::Path::new(to)) else { return target.into() }; + let rels = &out.preserved.as_ref().unwrap().parts[0].rels; + assert_eq!(rels[1].target, target, "only paths of linked workbooks change"); + rels[0].target.clone() +} + +#[test] +#[cfg(unix)] +fn saving_into_another_folder_rebases_relative_links_to_workbooks() { + assert_eq!(rebased("Source.xlsx", "/links", "/links/move2"), "../Source.xlsx"); + assert_eq!(rebased("Source.xlsx", "/links", "/other/deeper"), "../../links/Source.xlsx"); + assert_eq!(rebased("../Source.xlsx", "/links/move2", "/links"), "Source.xlsx"); + assert_eq!(rebased("sub\\Source.xlsx", "/links", "/links/sub"), "Source.xlsx"); + assert_eq!(rebased("./sub/../Source.xlsx", "/links", "/links/sub/"), "../Source.xlsx"); + // Folder names from the file system are written as a URI path; the target's own spelling stays. + assert_eq!(rebased("My%20Book.xlsx", "/links/q2 data", "/links"), "q2%20data/My%20Book.xlsx"); + assert_eq!(rebased("Résumé.xlsx", "/a/Été", "/a"), "Été/Résumé.xlsx"); + // The same folder, absolute paths, URLs and paths above the root are kept as they are. + assert_eq!(rebased("Source.xlsx", "/links/", "/links/x/.."), "Source.xlsx"); + for target in ["/links/Source.xlsx", "file:///C:/links/Source.xlsx", "C:\\links\\Source.xlsx", "https://example.com/Source.xlsx", "../../x.xlsx"] + { + assert_eq!(rebased(target, "/links", "/links/move2"), target); + } + // Without the folder it was read from, nothing to rebase. + let (mut wb, _) = read_xlsx(&rich_package()).unwrap(); + assert!(crate::rebase_links(&wb, std::path::Path::new("/elsewhere")).is_none()); + wb.source_dir = Some("relative/dir".into()); + assert!(crate::rebase_links(&wb, std::path::Path::new("/elsewhere")).is_none()); +} + +// ------------------------------------------------------------------ chart sheets (#727) + +const NS_C: &str = "http://schemas.openxmlformats.org/drawingml/2006/chart"; +const CT_PREFIX: &str = "application/vnd.openxmlformats-officedocument"; + +fn chart_xml(formula: &str) -> String { + format!( + r#"{formula}"# + ) +} + +fn chartsheet_xml(note: &str) -> String { + format!( + r#""# + ) +} + +fn drawing_xml(note: &str) -> String { + format!( + r#""# + ) +} + +fn worksheet_xml(value: &str) -> String { + format!( + r#"{value}"# + ) +} + +/// Sheets in file order: Data, Chart1 (a chart sheet on Data), More, Spare, Last (a hidden chart +/// sheet on More, last in the list). "More" has a print area; the active tab is "More". +fn chartsheet_package() -> Vec { + let workbook = format!( + r#"More!$A$1:$B$2"# + ); + let content_types = format!( + r#""# + ); + let (cs1, cs2) = (chartsheet_xml("first"), chartsheet_xml("last")); + let (d1, d2) = (drawing_xml("first"), drawing_xml("last")); + let (c1, c2) = (chart_xml("Data!$A$1:$A$3"), chart_xml("'More'!$A$1")); + let (s1, s2, s3) = (worksheet_xml("data"), worksheet_xml("more"), worksheet_xml("spare")); + make_zip(&[ + ("[Content_Types].xml", content_types.as_bytes()), + ("_rels/.rels", rels(&[("rId1", "officeDocument", "xl/workbook.xml")]).as_bytes()), + ("xl/workbook.xml", workbook.as_bytes()), + ( + "xl/_rels/workbook.xml.rels", + rels(&[ + ("rId1", "worksheet", "worksheets/sheet1.xml"), + ("rId2", "chartsheet", "chartsheets/sheet1.xml"), + ("rId3", "worksheet", "worksheets/sheet2.xml"), + ("rId4", "worksheet", "worksheets/sheet3.xml"), + ("rId5", "chartsheet", "chartsheets/sheet2.xml"), + ]) + .as_bytes(), + ), + ("xl/worksheets/sheet1.xml", s1.as_bytes()), + ("xl/worksheets/sheet2.xml", s2.as_bytes()), + ("xl/worksheets/sheet3.xml", s3.as_bytes()), + ("xl/chartsheets/sheet1.xml", cs1.as_bytes()), + ("xl/chartsheets/sheet2.xml", cs2.as_bytes()), + ( + "xl/chartsheets/_rels/sheet1.xml.rels", + rels(&[("rId1", "drawing", "../drawings/drawing1.xml"), ("rId2", "printerSettings", "../printerSettings/printerSettings1.bin")]) + .as_bytes(), + ), + ("xl/chartsheets/_rels/sheet2.xml.rels", rels(&[("rId1", "drawing", "../drawings/drawing2.xml")]).as_bytes()), + ("xl/printerSettings/printerSettings1.bin", PRINTER), + ("xl/drawings/drawing1.xml", d1.as_bytes()), + ("xl/drawings/drawing2.xml", d2.as_bytes()), + ("xl/drawings/_rels/drawing1.xml.rels", rels(&[("rId1", "chart", "../charts/chart1.xml")]).as_bytes()), + ("xl/drawings/_rels/drawing2.xml.rels", rels(&[("rId1", "chart", "../charts/chart2.xml")]).as_bytes()), + ("xl/charts/chart1.xml", c1.as_bytes()), + ("xl/charts/chart2.xml", c2.as_bytes()), + ]) +} + +/// The sheets of a written workbook: (name, state) in file order. +fn written_sheets(u: &Unpacked) -> Vec<(String, String)> { + let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap(); + wb.child("sheets").unwrap().kids("sheet").map(|s| (s.attr("name").unwrap().to_string(), s.attr("state").unwrap_or("").to_string())).collect() +} + +/// The part a written chart sheet is in, found through the workbook relationship; its drawing and +/// that drawing's chart, followed through relationships. +fn chart_sheet_chain(u: &Unpacked, name: &str) -> (String, String, String) { + let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap(); + let id = wb.child("sheets").unwrap().kids("sheet").find(|s| s.attr("name") == Some(name)).unwrap().attr("id").unwrap().to_string(); + let (ty, part) = u.target("xl/workbook.xml", &id); + assert!(ty.ends_with("/chartsheet"), "{ty}"); + let drawing = u.rels(&part).into_iter().find(|r| r.1.ends_with("/drawing")).unwrap().2; + let chart = u.rels(&drawing).into_iter().find(|r| r.1.ends_with("/chart")).unwrap().2; + (part, drawing, chart) +} + +#[test] +fn chart_sheets_survive_a_round_trip_in_place() { + let (wb, rep) = read_xlsx(&chartsheet_package()).unwrap(); + assert!(!rep.warnings.iter().any(|w| w.contains("skipped") || w.contains("dropped")), "{:?}", rep.warnings); + let names: Vec<&str> = wb.sheets.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, ["Data", "More", "Spare"], "chart sheets are not worksheets"); + + let out = write_xlsx(&wb).unwrap(); + let u = Unpacked::new(&out); + u.assert_valid(); + // Same order, same visibility, the way Excel shows the tabs. + assert_eq!( + written_sheets(&u), + [("Data", ""), ("Chart1", ""), ("More", ""), ("Spare", ""), ("Last", "hidden")].map(|(a, b)| (a.to_string(), b.to_string())), + ); + // Sheet ids are unique. + let wbx = u.text("xl/workbook.xml"); + let mut ids: Vec = wbx.match_indices("`. + let (cs_part, _, _) = chart_sheet_chain(&u, "Chart1"); + let ps = u.target(&cs_part, &attr_of(&u.text(&cs_part), "pageSetup", "r:id")); + assert!(ps.0.ends_with("/printerSettings")); + assert_eq!(u.parts.get(&ps.1).unwrap(), PRINTER); + + // Reading the output again finds the same two chart sheets, and a second save is stable. + let (wb2, rep2) = read_xlsx(&out).unwrap(); + assert!(!rep2.warnings.iter().any(|w| w.contains("skipped") || w.contains("dropped")), "{:?}", rep2.warnings); + let again = Unpacked::new(&write_xlsx(&wb2).unwrap()); + again.assert_valid(); + assert_eq!(written_sheets(&again), written_sheets(&u)); +} + +#[test] +fn a_chart_sheet_whose_data_sheet_was_renamed_is_dropped_with_its_parts() { + let (mut wb, _) = read_xlsx(&chartsheet_package()).unwrap(); + wb.sheet_mut(0).unwrap().name = "Renamed".into(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + u.assert_valid(); + // "Chart1" plots Data, which is gone under that name; "Last" plots More, which is not. + assert_eq!(written_sheets(&u).iter().map(|s| s.0.as_str()).collect::>(), ["Renamed", "More", "Spare", "Last"]); + assert!(u.find(chart_xml("Data!$A$1:$A$3").as_bytes()).is_empty()); + assert!(u.find(drawing_xml("first").as_bytes()).is_empty()); + assert!(u.find(PRINTER).is_empty()); + chart_sheet_chain(&u, "Last"); + let wbx = u.text("xl/workbook.xml"); + assert_eq!(attr_of(&wbx, "workbookView", "activeTab"), "1"); + assert_eq!(attr_of(&wbx, "definedName", "localSheetId"), "1"); +} + +#[test] +fn a_chart_sheet_whose_name_a_worksheet_took_is_dropped() { + let (mut wb, _) = read_xlsx(&chartsheet_package()).unwrap(); + wb.sheet_mut(2).unwrap().name = "LAST".into(); + let u = Unpacked::new(&write_xlsx(&wb).unwrap()); + u.assert_valid(); + assert_eq!(written_sheets(&u).iter().map(|s| s.0.as_str()).collect::>(), ["Data", "Chart1", "More", "LAST"]); + assert!(u.find(chart_xml("'More'!$A$1").as_bytes()).is_empty()); + chart_sheet_chain(&u, "Chart1"); +} + +#[test] +fn formulas_name_the_sheets_a_chart_plots() { + use crate::passthrough::sheet_names_in; + assert_eq!(sheet_names_in("Data!$A$1:$A$3"), ["Data"]); + assert_eq!(sheet_names_in("'My ''Sheet'!$A$1,Data!$B$1"), ["My 'Sheet", "Data"]); + assert_eq!(sheet_names_in("SUM(Data!A1:A3,'Other sheet'!B2)"), ["Data", "Other sheet"]); + assert!(sheet_names_in("[1]Ext!A1").is_empty() && sheet_names_in("[1]Ext!A1:A3").is_empty()); + assert!(sheet_names_in("Name").is_empty() && sheet_names_in("").is_empty()); +} + #[test] fn file_version_and_calc_features_survive_a_save() { // Excel trusts the saved cell values only when workbook.xml carries both and the diff --git a/crates/xlsx/src/write.rs b/crates/xlsx/src/write.rs index dddba98..06761f0 100644 --- a/crates/xlsx/src/write.rs +++ b/crates/xlsx/src/write.rs @@ -224,9 +224,15 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { }; let mut wb_rels = Rels::default(); + // Kept content first: the chart sheets among the worksheets need their relationship ids. + let kept = crate::passthrough::write_workbook(wb, &mut wb_rels, &mut out); let mut pivots = crate::pivot::PivotWriter::default(); let mut sheets_xml = String::new(); + let mut sheet_id = 1usize; for (i, sheet) in wb.sheets.iter().enumerate() { + let (before, next) = kept.sheet_entries(i, false, sheet_id); + sheets_xml.push_str(&before); + sheet_id = next; let (xml, mut rels) = crate::sheet_write::write_sheet(wb, i, i == active, &mut out); pivots.add_sheet(wb, i, &mut out, &mut rels); let name = format!("xl/worksheets/sheet{}.xml", i + 1); @@ -242,8 +248,10 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { Visibility::VeryHidden => " state=\"veryHidden\"", }; let name: String = sheet.name.chars().take(31).collect(); - let _ = write!(sheets_xml, "", esc_attr(&name), i + 1); + let _ = write!(sheets_xml, "", esc_attr(&name)); + sheet_id += 1; } + sheets_xml.push_str(&kept.sheet_entries(wb.sheets.len(), true, sheet_id).0); // Defined names, then print areas and titles. let mut names = String::new(); @@ -259,7 +267,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result, IoError> { } let _ = write!(names, "