diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..2c4b8cb1a --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,30 @@ +version: 2 +updates: + # Maintain Rust / Cargo dependencies + - package-ecosystem: "cargo" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + # Consolidate updates into a single PR rather than one PR per crate + groups: + cargo-dependencies: + patterns: + - "*" + commit-message: + prefix: "chore(deps)" + + # Maintain GitHub Actions workflows + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + groups: + actions: + patterns: + - "*" + commit-message: + prefix: "ci(deps)" diff --git a/Cargo.lock b/Cargo.lock index 6d618a02a..17ebf848c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1496,7 +1496,7 @@ dependencies = [ [[package]] name = "gridcraft" -version = "0.7.0" +version = "0.8.0" dependencies = [ "arboard", "craft-print", @@ -1518,7 +1518,7 @@ dependencies = [ [[package]] name = "gridcraft-calc" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-core", "gridcraft-formula", @@ -1530,7 +1530,7 @@ dependencies = [ [[package]] name = "gridcraft-chart" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-calc", "gridcraft-core", @@ -1541,7 +1541,7 @@ dependencies = [ [[package]] name = "gridcraft-cli" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-engine", "gridcraft-mcp", @@ -1553,7 +1553,7 @@ dependencies = [ [[package]] name = "gridcraft-core" -version = "0.7.0" +version = "0.8.0" dependencies = [ "js-sys", "serde", @@ -1563,7 +1563,7 @@ dependencies = [ [[package]] name = "gridcraft-engine" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-calc", "gridcraft-chart", @@ -1584,7 +1584,7 @@ dependencies = [ [[package]] name = "gridcraft-formula" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-core", "serde", @@ -1593,7 +1593,7 @@ dependencies = [ [[package]] name = "gridcraft-functions" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-core", "regex", @@ -1601,7 +1601,7 @@ dependencies = [ [[package]] name = "gridcraft-mcp" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-engine", "log", @@ -1610,7 +1610,7 @@ dependencies = [ [[package]] name = "gridcraft-model" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-core", "gridcraft-formula", @@ -1622,18 +1622,18 @@ dependencies = [ [[package]] name = "gridcraft-numfmt" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-core", ] [[package]] name = "gridcraft-pdf" -version = "0.7.0" +version = "0.8.0" [[package]] name = "gridcraft-ui-egui" -version = "0.7.0" +version = "0.8.0" dependencies = [ "craft-ui", "egui", @@ -1655,7 +1655,7 @@ dependencies = [ [[package]] name = "gridcraft-web" -version = "0.7.0" +version = "0.8.0" dependencies = [ "eframe", "egui", @@ -1672,7 +1672,7 @@ dependencies = [ [[package]] name = "gridcraft-xlsx" -version = "0.7.0" +version = "0.8.0" dependencies = [ "aes", "gridcraft-core", @@ -4526,7 +4526,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" [[package]] name = "xtask" -version = "0.7.0" +version = "0.8.0" dependencies = [ "gridcraft-engine", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 98dd572e0..a967b999f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/gridcraft", "apps/gridcraft-cli", "xtask"] [workspace.package] -version = "0.7.0" +version = "0.8.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.90" diff --git a/README.md b/README.md index 678495f1d..3837b9176 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,9 @@ Runs on macOS, Windows, Linux, BSD and the web License: MIT OR Apache-2.0 Agent-drivable over MCP +
+ Scanned by SonarCloud + Reviewed by CodeRabbit

diff --git a/apps/gridcraft/src/control_server.rs b/apps/gridcraft/src/control_server.rs index e9e05a2e6..4c57ccf41 100644 --- a/apps/gridcraft/src/control_server.rs +++ b/apps/gridcraft/src/control_server.rs @@ -1,7 +1,11 @@ //! Localhost JSON-lines control server. Each connection may send many requests; each line //! `{"id":…, "method":…, "params":…}` gets one reply line `{"id":…, "ok":…, …}`. +//! +//! The server only reads requests: a line that is not a JSON object with a string `method` (an +//! HTTP request line, a stray `nc`, binary data) gets one error reply and the connection is +//! closed, so nothing sent after it runs. -use std::io::{BufRead, BufReader, Write}; +use std::io::{BufRead, BufReader, Read, Write}; use std::net::{TcpListener, TcpStream}; use std::sync::mpsc::{Receiver, Sender, channel}; use std::time::Duration; @@ -13,6 +17,13 @@ const MAX_LINE: usize = 64 * 1024 * 1024; pub fn start(port: u16, ctx: egui::Context) -> std::io::Result> { let listener = TcpListener::bind(("127.0.0.1", port))?; + let rx = serve_listener(listener, ctx)?; + log::info!("control channel listening on 127.0.0.1:{port}"); + Ok(rx) +} + +/// Accept connections on `listener`; requests arrive on the returned receiver. +fn serve_listener(listener: TcpListener, ctx: egui::Context) -> std::io::Result> { let (tx, rx) = channel(); std::thread::Builder::new().name("control".into()).spawn(move || { for stream in listener.incoming().flatten() { @@ -21,47 +32,184 @@ pub fn start(port: u16, ctx: egui::Context) -> std::io::Result, max: usize) -> Result { + buf.clear(); + // `take` bounds the bytes buffered for one line: a peer can't make us allocate more than `max`. + let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1); + let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| e.to_string())?; + if n == 0 { + return Ok(false); + } + if buf.last() == Some(&b'\n') { + buf.pop(); + return Ok(true); + } + if buf.len() > max { + return Err(format!("request too large (over {max} bytes); closing the connection")); + } + // Last line without a newline, then end of stream. + Ok(true) +} + +/// One request line, parsed. +enum Line { + Request { + id: Json, + method: String, + params: Json, + }, + Blank, + /// Not a request: reply with this error and close the connection. + Reject(String), +} + +fn parse_line(bytes: &[u8]) -> Line { + let Ok(text) = std::str::from_utf8(bytes) else { + return Line::Reject("not a control request (invalid UTF-8); closing the connection".into()); + }; + let text = text.trim(); + if text.is_empty() { + return Line::Blank; + } + if is_http_request_line(text) { + return Line::Reject("HTTP is not served here: this port takes JSON lines; closing the connection".into()); + } + let msg = match serde_json::from_str::(text) { + Ok(v) => v, + Err(e) => return Line::Reject(format!("invalid JSON ({e}); expected one JSON request object per line; closing the connection")), + }; + let Some(obj) = msg.as_object() else { + return Line::Reject("not a control request (expected a JSON object); closing the connection".into()); + }; + let Some(method) = obj.get("method").and_then(Json::as_str) else { + return Line::Reject("not a control request (missing string `method`); closing the connection".into()); + }; + Line::Request { + id: obj.get("id").cloned().unwrap_or(Json::Null), + method: method.to_string(), + params: obj.get("params").cloned().unwrap_or(json!({})), + } +} + +/// An HTTP/1 request line (`POST / HTTP/1.1`) or the HTTP/2 preface (`PRI * HTTP/2.0`). +fn is_http_request_line(text: &str) -> bool { + const METHODS: [&str; 10] = ["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH", "PRI"]; + text.split_once(' ').is_some_and(|(method, _)| METHODS.contains(&method)) + || text.rsplit_once(' ').is_some_and(|(_, version)| version.starts_with("HTTP/")) +} + fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { let Ok(read) = stream.try_clone() else { return }; let mut out = stream; - let reader = BufReader::new(read); - for line in reader.lines() { - let Ok(line) = line else { break }; - if line.trim().is_empty() { - continue; - } - if line.len() > MAX_LINE { - let _ = writeln!(out, "{}", json!({"ok": false, "error": "request too large"})); - continue; - } - let msg: Json = match serde_json::from_str(&line) { - Ok(v) => v, - Err(e) => { - let _ = writeln!(out, "{}", json!({"ok": false, "error": format!("invalid JSON: {e}")})); - continue; - } + let mut reader = BufReader::new(read); + let mut buf = Vec::new(); + loop { + let reject = match read_bounded_line(&mut reader, &mut buf, MAX_LINE) { + Ok(false) => break, + // The peer is gone before sending a line: nobody to tell. + Err(_) if buf.is_empty() => break, + Err(e) => e, + Ok(true) => match parse_line(&buf) { + Line::Blank => continue, + Line::Reject(e) => e, + Line::Request { id, method, params } => { + let (req, rx) = ControlRequest::new(method, params); + if tx.send(req).is_err() { + break; + } + // Wake the UI thread: egui only runs frames when something happens. + ctx.request_repaint(); + let mut reply = rx + .recv_timeout(Duration::from_secs(60)) + .unwrap_or_else(|_| json!({"ok": false, "error": "timed out waiting for the UI (is the window minimised?)"})); + if let Some(o) = reply.as_object_mut() { + o.insert("id".into(), id); + } + if writeln!(out, "{reply}").is_err() { + break; + } + continue; + } + }, }; - let id = msg.get("id").cloned().unwrap_or(Json::Null); - let method = msg.get("method").and_then(Json::as_str).unwrap_or("").to_string(); - let params = msg.get("params").cloned().unwrap_or(json!({})); - let (req, rx) = ControlRequest::new(method, params); - if tx.send(req).is_err() { - break; + // The connection closes either way; a peer that can't read the reason loses nothing. + let _ = writeln!(out, "{}", json!({"ok": false, "error": reject})); + break; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Send `input` on a new connection and read reply lines until the server closes it. + fn exchange(addr: std::net::SocketAddr, input: &[u8]) -> Vec { + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + c.write_all(input).unwrap(); + // A read timeout ends the list too: the server kept the connection open. + BufReader::new(c).lines().map_while(Result::ok).map(|l| serde_json::from_str(&l).unwrap()).collect() + } + + fn serve_on_loopback() -> (std::net::SocketAddr, Receiver) { + let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let addr = listener.local_addr().unwrap(); + (addr, serve_listener(listener, egui::Context::default()).unwrap()) + } + + /// A browser can POST to the port: its request line closes the connection before the JSON + /// line in its body is read, so nothing is dispatched. + #[test] + fn an_http_request_closes_the_connection_before_its_body_is_dispatched() { + let (addr, rx) = serve_on_loopback(); + let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1:7979\r\nContent-Type: text/plain\r\n\r\n\ + {\"id\":1,\"method\":\"engine.execute\",\"params\":{\"command\":\"cell.set\",\"params\":{\"cell\":\"A1\",\"input\":\"x\"}}}\n"; + let replies = exchange(addr, http); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert!(replies[0]["error"].as_str().unwrap().contains("closing the connection"), "{replies:?}"); + assert!(rx.try_recv().is_err(), "the body was dispatched"); + } + + #[test] + fn a_line_that_is_not_a_request_closes_the_connection() { + let (addr, rx) = serve_on_loopback(); + std::thread::spawn(move || { + for req in rx { + let _ = req.reply.send(json!({"ok": true})); + } + }); + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + c.write_all(b"{\"id\":1,\"method\":\"a\"}\r\n\n{\"id\":2,\"method\":\"b\"}\n").unwrap(); + let mut r = BufReader::new(c); + for id in [1, 2] { + let mut line = String::new(); + r.read_line(&mut line).unwrap(); + assert_eq!(serde_json::from_str::(&line).unwrap()["id"], id, "{line}"); } - // Wake the UI thread: egui only runs frames when something happens. - ctx.request_repaint(); - let mut reply = rx - .recv_timeout(Duration::from_secs(60)) - .unwrap_or_else(|_| json!({"ok": false, "error": "timed out waiting for the UI (is the window minimised?)"})); - if let Some(o) = reply.as_object_mut() { - o.insert("id".into(), id); + for junk in [&b"hello\n"[..], b"[1,2]\n", b"{\"id\":1}\n", b"{\"method\":5}\n", b"\xff\xfe\n", b"GET /x HTTP/1.1\n"] { + let mut input = junk.to_vec(); + input.extend_from_slice(b"{\"id\":2,\"method\":\"b\"}\n"); + let replies = exchange(addr, &input); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert_eq!(replies[0]["ok"], false); } - if writeln!(out, "{reply}").is_err() { - break; - } + assert!(is_http_request_line("OPTIONS * HTTP/1.1")); + assert!(is_http_request_line("PRI * HTTP/2.0")); + assert!(!is_http_request_line("{\"method\":\"GET /\"}")); + } + + #[test] + fn an_overlong_line_is_refused_without_reading_the_rest() { + let mut buf = Vec::new(); + let long = [b'x'; 100]; + let mut input: &[u8] = &long; + assert!(read_bounded_line(&mut input, &mut buf, 10).is_err()); + assert_eq!(input.len(), 100 - 11, "the rest of the line is left unread"); } } diff --git a/contributors/people.toml b/contributors/people.toml index d1b2d8c22..4cdbf7714 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -66,6 +66,9 @@ sync_github_name = true display_name = "Netter" sync_github_name = false +[people.Shnrqpdr] +sync_github_name = true + [people.Harrison-Booth] real_name = "Harrison Booth" sync_github_name = false diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 196c57de7..97f7a8350 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -1266,3 +1266,31 @@ fn offset_extreme_arguments_return_ref() { assert_eq!(crate::evaluate(&wb, 0, c("D1"), formula), Value::Number(expected), "{formula}"); } } + +#[test] +fn median_large_values_recalculate_dependents() { + let mut t = T::new(); + t.set("A1", "1e308"); + t.set("A2", "1.6e308"); + t.set("B1", "=MEDIAN(A1:A2)"); + t.set("B2", "=AGGREGATE(12,0,A1:A2)"); + t.set("C1", "=B1/1e308"); + t.set("C2", "=B2/1e308"); + for cell in ["B1", "B2"] { + assert!((t.num(cell) / 1e308 - 1.3).abs() < 1e-14); + } + for cell in ["C1", "C2"] { + assert!((t.num(cell) - 1.3).abs() < 1e-14); + } + t.set("A2", "-1e308"); + for cell in ["B1", "B2", "C1", "C2"] { + assert_eq!(t.num(cell), 0.0); + } + t.set("A1", "-1.6e308"); + for cell in ["B1", "B2"] { + assert!((t.num(cell) / 1e308 + 1.3).abs() < 1e-14); + } + for cell in ["C1", "C2"] { + assert!((t.num(cell) + 1.3).abs() < 1e-14); + } +} diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 2bdfe797a..2c087f22e 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -210,11 +210,18 @@ impl RangeRef { if let (Some(x), Some(y)) = (CellRef::parse(a), CellRef::parse(b)) { return Some(RangeRef::new(x, y)); } - let strip = |t: &str| t.trim_start_matches('$').to_string(); + let strip = |t: &str| t.strip_prefix('$').unwrap_or(t).to_string(); if let (Some(x), Some(y)) = (letters_to_col(&strip(a)), letters_to_col(&strip(b))) { return Some(RangeRef::cols(x.min(y), x.max(y))); } - let (pa, pb) = (strip(a).parse::().ok()?, strip(b).parse::().ok()?); + let row_num = |t: &str| { + let t = t.strip_prefix('$').unwrap_or(t); + if t.is_empty() || !t.bytes().all(|c| c.is_ascii_digit()) { + return None; + } + t.parse::().ok() + }; + let (pa, pb) = (row_num(a)?, row_num(b)?); if pa == 0 || pb == 0 || pa > MAX_ROWS || pb > MAX_ROWS { return None; } @@ -234,6 +241,18 @@ impl fmt::Display for RangeRef { mod tests { use super::*; + #[test] + fn range_parse_rejects_signed_rows() { + assert!(RangeRef::parse("+1:+3").is_none()); + assert!(RangeRef::parse("-1:3").is_none()); + assert!(RangeRef::parse("$$1:3").is_none()); + assert!(RangeRef::parse(":3").is_none()); + assert!(RangeRef::parse("$$A:$$C").is_none()); + assert!(RangeRef::parse("$A:$C").is_some()); + let r = RangeRef::parse("$1:$3").unwrap(); + assert_eq!((r.start.row, r.end.row), (0, 2)); + } + #[test] fn letters_roundtrip() { for (c, s) in [(0, "A"), (25, "Z"), (26, "AA"), (51, "AZ"), (52, "BA"), (701, "ZZ"), (702, "AAA"), (16383, "XFD")] { diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 383624b70..24bd043f7 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -551,6 +551,8 @@ fn custom_ok(v: &Value, text: &str, op: &str, crit: &str) -> bool { let num = gridcraft_core::parse::parse_number_text(crit); let cmp = match (v.as_f64(), num) { (Some(a), Some(b)) => a.partial_cmp(&b), + // A numeric ordering comparison only matches numbers; blanks and text never do. + (None, Some(_)) if matches!(op, "<" | "<=" | ">" | ">=") => None, _ => Some(gridcraft_core::compare_text(text, crit)), }; let wild = || super::edit::wildcard(&text.to_lowercase(), &crit.to_lowercase()); @@ -1199,7 +1201,7 @@ fn detail(s: &mut Session, p: &Json, hide: bool) -> Result { .collect(); for i in g.start..=g.end { if let Some(e) = map.get_mut(&i) { - e.hidden = hide || inner.iter().any(|o| o.contains(i)); + set_hidden(e, hide || inner.iter().any(|o| o.contains(i))); } } if hide { @@ -1213,6 +1215,16 @@ fn detail(s: &mut Session, p: &Json, hide: bool) -> Result { }) } +/// Hides or shows an outline line. A line shown with a zero size (files that hide columns as +/// `width="0" hidden="1"`) gets the default size back, as Unhide does; otherwise + would mark the +/// group expanded and leave it invisible (#1714). +fn set_hidden(e: &mut LineInfo, hidden: bool) { + e.hidden = hidden; + if !hidden && e.size == Some(0.0) { + e.size = None; + } +} + /// The outline's level buttons: show rows / columns down to outline level `n` (1 = only the /// top-level summaries); deeper detail is hidden and its summary lines marked collapsed. fn show_levels(s: &mut Session, p: &Json) -> Result { @@ -1231,7 +1243,7 @@ fn show_levels(s: &mut Session, p: &Json) -> Result { let Some(n) = n else { continue }; let groups = outline::groups(map); for e in map.values_mut().filter(|e| e.outline > 0) { - e.hidden = e.outline >= n; + set_hidden(e, e.outline >= n); } // Inner groups come first, so a summary line shared with the group around it ends up // as that outer group says. @@ -1367,6 +1379,31 @@ mod tests { assert!(!ok(&mut s, "B1", "150")); } + #[test] + fn showing_detail_gives_zero_width_columns_their_width_back() { + // #1714: templates hide grouped columns as `width="0" hidden="1"`; + must show them. + let mut s = Session::new(); + s.new_workbook(); + let hidden = LineInfo { size: Some(0.0), hidden: true, outline: 1, ..LineInfo::default() }; + let sh = Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap(); + for c in 2..=4 { + sh.cols.insert(c, hidden); + } + sh.cols.insert(5, LineInfo { collapsed: true, ..LineInfo::default() }); + s.execute("data.showDetail", json!({"cols": "C:E", "level": 1})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + for c in 2..=4 { + assert_eq!(sh.col_width(c), sh.default_col_width, "column {c}"); + } + // Hiding again hides them; the level buttons show zero-width columns too. + s.execute("data.hideDetail", json!({"cols": "C:E", "level": 1})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().col_width(2), 0.0); + Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().cols.insert(3, hidden); + s.execute("data.showLevels", json!({"cols": 2})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert_eq!((sh.col_width(2), sh.col_width(3)), (sh.default_col_width, sh.default_col_width)); + } + #[test] fn removing_an_autofilter_keeps_manually_hidden_and_collapsed_rows() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index c69bac208..fe7f1fe8b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -338,20 +338,31 @@ fn range_fill(s: &mut Session, p: &Json) -> Result { let input = str_param(p, "input").unwrap_or("").to_string(); let origin = s.doc()?.selection.active; edit(s, |cx| { - let base = input_to_cell(&input, sheet, origin, &mut cx.wb)?; + // Each cell gets what typing the input there gives: the automatic number format where + // it's General, its own format otherwise, text where it's Text. That depends only on + // the cell's style, so the input is parsed once per style. + let mut by_style: std::collections::HashMap> = std::collections::HashMap::new(); for r in &ranges { if r.count() > 2_000_000 { return Err(bad("range.fill", "range too large")); } for at in r.iter() { - let mut cell = base.clone().unwrap_or_default(); + let style = cx.wb.sheet(sheet).map(|sh| sh.style_id(at)).unwrap_or_default(); + let base = match by_style.get(&style) { + Some(c) => c.clone(), + None => { + let c = input_to_cell(&input, sheet, at, &mut cx.wb)?; + by_style.insert(style, c.clone()); + c + } + }; + let mut cell = base.unwrap_or_default(); if let Some(f) = &cell.formula && let Some(e) = f.expr() { let shifted = gridcraft_formula::adjust::shift_relative(e, at.row as i64 - origin.row as i64, at.col as i64 - origin.col as i64); cell.formula = Some(std::sync::Arc::new(Formula::from_expr(shifted))); } - cell.style = cx.wb.sheet(sheet).and_then(|sh| sh.cell(at)).map(|c| c.style).unwrap_or(cell.style); cx.sheet_mut(sheet)?.set_cell(at, cell); cx.touch(sheet, at); } @@ -601,7 +612,8 @@ fn undo(s: &mut Session, p: &Json) -> Result { let now = d.wb.clone(); for _ in 0..steps { let Some(e) = d.undo.pop() else { break }; - let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: d.selection.clone() }; + // Redo goes back to the undone edit's cells, not wherever the user clicked before undoing (#1855). + let cur = crate::HistoryEntry { label: e.label.clone(), wb: d.wb.clone(), selection: e.selection.clone() }; d.redo.push(cur); label = e.label.clone(); d.wb = e.wb; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 7291ee897..9ddc05611 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -8,7 +8,7 @@ use crate::DocState; pub fn specs() -> Vec { vec![ cmd!(noundo "file.new", "New Workbook", ["File"], Some("Cmd+N"), "{sample?: \"budget\"|\"sales\"|\"grades\"}", always, new_workbook), - cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path} | {name, base64}, plus {password} for a password-protected workbook (xlsx, xlsm, xls import, xlsb and ods data import, csv, tsv, txt, json)", always, open), + cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path} | {name, base64}, plus {password} for a password-protected workbook and {csvAsText?: bool} to keep every csv/tsv/txt field as text, with no number or date conversion (xlsx, xlsm, xls import, xlsb and ods data import, csv, tsv, txt, json)", always, open), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?} (xlsx by default; .csv/.tsv/.json/.html/.pdf by extension)", has_doc, save), cmd!(noundo "file.saveAs", "Save As…", ["File"], Some("Cmd+Shift+S"), "{path} (.xlsx, .csv, .tsv, .json, .html, or .pdf: a PDF of the active sheet)", has_doc, save_as), cmd!(query "file.saveBytes", "Encode Workbook", [], None, "{format?: xlsx|csv|tsv|json|html} → {base64}", has_doc, save_bytes), @@ -56,7 +56,7 @@ fn open(s: &mut Session, p: &Json) -> Result { return ok(); }; let encrypted = gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Encrypted; - let (mut wb, warnings) = crate::io::open_bytes_with_password(&name, &bytes, str_param(p, "password"))?; + let (mut wb, warnings) = crate::io::open_bytes_with(&name, &bytes, str_param(p, "password"), bool_param(p, "csvAsText").unwrap_or(false))?; wb.source_dir = str_param(p, "path").and_then(crate::io::source_dir); let imported = matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb | crate::io::FileKind::Xls)) diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 40df72df0..454c545d0 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -229,7 +229,7 @@ fn valid_name(n: &str) -> bool { if !(first.is_alphabetic() || first == '_' || first == '\\') { return false; } - if n.len() > 255 || CellRef::parse(n).is_some() || is_r1c1(n) { + if n.chars().count() > 255 || CellRef::parse(n).is_some() || is_r1c1(n) { return false; } n.chars().all(|c| c.is_alphanumeric() || matches!(c, '_' | '.' | '\\' | '?')) @@ -610,4 +610,18 @@ mod tests { let r = s.execute("formulas.defineName", json!({"name": "First", "refersTo": "=Sheet1!$A$1"})).unwrap(); assert_eq!(r["refersTo"], "=Sheet1!$A$1"); } + + #[test] + fn arabic_defined_names_count_characters_not_bytes() { + let mut s = Session::new(); + s.new_workbook(); + let arabic = |n: usize| "س".repeat(n); + for ok in [200, 255] { + s.execute("formulas.defineName", json!({"name": arabic(ok), "refersTo": "=1"})).unwrap(); + } + assert!(s.execute("formulas.defineName", json!({"name": arabic(256), "refersTo": "=1"})).is_err()); + s.execute("range.setValues", json!({"range": "A1", "values": [[format!("ع{}", arabic(199))], [1]]})).unwrap(); + let r = s.execute("formulas.createFromSelection", json!({"range": "A1:A2", "top": true})).unwrap(); + assert_eq!(r["created"], 1); + } } diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index d878e4469..9f044613d 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -466,10 +466,11 @@ fn table_style(s: &mut Session, p: &Json) -> Result { } /// Table names follow the Microsoft name rules, as the xlsx writer does: letters, digits, `_` and `.`, -/// starting with a letter, `_` or `\`. Anything else (a hyphen, say) would read as an operator inside a -/// structured reference, and the writer would rename the table on save. +/// starting with a letter, `_` or `\`, and at most 255 characters. Anything else (a hyphen, say) would +/// read as an operator inside a structured reference, and the writer would rename the table on save. fn valid_table_name(name: &str) -> bool { - name.starts_with(|c: char| c.is_alphabetic() || c == '_' || c == '\\') + name.chars().count() <= 255 + && name.starts_with(|c: char| c.is_alphabetic() || c == '_' || c == '\\') && CellRef::parse(name).is_none() && name.chars().enumerate().all(|(i, c)| c.is_alphanumeric() || c == '_' || c == '.' || (i == 0 && c == '\\')) } diff --git a/crates/engine/src/cmd/pivot.rs b/crates/engine/src/cmd/pivot.rs index 8a441fa1b..06050f7e8 100644 --- a/crates/engine/src/cmd/pivot.rs +++ b/crates/engine/src/cmd/pivot.rs @@ -675,7 +675,13 @@ fn insert_pivot(s: &mut Session, p: &Json) -> Result { Json::String(n) => add_to_area("insert.pivotTable", pt, &src, n, area, None, None, None)?, Json::Object(_) => { let n = str_param(f, "field").ok_or_else(|| bad("insert.pivotTable", "a field object needs `field`"))?; - add_to_area("insert.pivotTable", pt, &src, n, area, func_param(str_param(f, "func")), str_param(f, "name"), None)?; + let func = match str_param(f, "func") { + Some(fname) => { + Some(func_param(Some(fname)).ok_or_else(|| bad("insert.pivotTable", format!("unknown function `{fname}`")))?) + } + None => None, + }; + add_to_area("insert.pivotTable", pt, &src, n, area, func, str_param(f, "name"), None)?; let by = group_param(str_param(f, "group")); if let Some(by) = by.filter(|b| *b != PivotDateGroup::None) && matches!(area, "rows" | "columns") diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index a10f19d46..8350e56d7 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -650,6 +650,10 @@ fn cell_look(wb: &Workbook, si: usize, sh: &Sheet, c: CellRef, cf: &mut Option String { /// and no General narrowing (Save As CSV/Text writes this). Formulas give their result. pub fn value_text(wb: &Workbook, sheet: &Sheet, c: CellRef) -> String { let style = wb.styles.get(sheet.style_id(c)); - let f = format(&sheet.value(c), style.num_fmt.as_str(), wb); + value_text_as(wb, &sheet.value(c), style.num_fmt.as_str()) +} + +/// [`value_text`] for a value and a format code given apart from a cell (print takes the code +/// from the cell's effective look, which a conditional format can override). +pub fn value_text_as(wb: &Workbook, v: &Value, code: &str) -> String { + let f = format(v, code, wb); match f.fill { Some((_, pos)) => { let mut t = f.text; diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 3e84036cb..14717a3df 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -55,8 +55,15 @@ pub fn open_bytes(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { /// [`open_bytes`], first decrypting a password-protected workbook with `password`. Errors never /// quote the password. pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str>) -> Result<(Workbook, Vec)> { + open_bytes_with(name, bytes, password, false) +} + +/// [`open_bytes_with_password`], and with `csv_as_text` a CSV/TSV/TXT file keeps every field as +/// the text it holds, with no number, date or boolean conversion (identifiers such as GTINs, SKUs +/// and ZIP codes survive). It does not apply to other formats. +pub fn open_bytes_with(name: &str, bytes: &[u8], password: Option<&str>, csv_as_text: bool) -> Result<(Workbook, Vec)> { if gridcraft_xlsx::sniff(bytes) != gridcraft_xlsx::Format::Encrypted { - let (mut wb, warnings) = open_plain(name, bytes)?; + let (mut wb, warnings) = open_plain(name, bytes, csv_as_text)?; fit_imported_rows(&mut wb); return Ok((wb, warnings)); } @@ -70,7 +77,7 @@ pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str> if gridcraft_xlsx::sniff(&plain) == gridcraft_xlsx::Format::Encrypted { return Err(EngineError::Other(format!("We can't open '{name}': the encrypted file is damaged (it holds another encrypted file)."))); } - let (mut wb, mut warnings) = open_plain(name, &plain)?; + let (mut wb, mut warnings) = open_plain(name, &plain, csv_as_text)?; fit_imported_rows(&mut wb); warnings.insert(0, DECRYPTED_NOTICE.into()); Ok((wb, warnings)) @@ -112,7 +119,7 @@ fn decrypt_message(name: &str, e: gridcraft_xlsx::DecryptError) -> String { } /// Parses bytes that aren't encrypted. -fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { +fn open_plain(name: &str, bytes: &[u8], csv_as_text: bool) -> Result<(Workbook, Vec)> { let kind = FileKind::from_path(name); let sniffed = gridcraft_xlsx::sniff(bytes); if sniffed == gridcraft_xlsx::Format::Xls { @@ -153,7 +160,7 @@ fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { Ok((wb, vec![])) } Some(FileKind::Tsv) => { - let opts = gridcraft_xlsx::CsvOptions { delimiter: b'\t', ..Default::default() }; + let opts = gridcraft_xlsx::CsvOptions { delimiter: b'\t', parse_values: !csv_as_text, ..Default::default() }; let mut wb = gridcraft_xlsx::read_csv(bytes, &opts).map_err(|e| EngineError::Other(e.to_string()))?; // Tab is what makes it a .tsv/.txt; saved as .csv it gets the CSV default. wb.csv_delimiter = None; @@ -161,7 +168,7 @@ fn open_plain(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { Ok((wb, vec![])) } _ => { - let opts = gridcraft_xlsx::CsvOptions { delimiter: 0, ..Default::default() }; + let opts = gridcraft_xlsx::CsvOptions { delimiter: 0, parse_values: !csv_as_text, ..Default::default() }; let mut wb = gridcraft_xlsx::read_csv(bytes, &opts).map_err(|e| EngineError::Other(e.to_string()))?; rename_first_sheet(&mut wb, name); Ok((wb, vec![])) @@ -251,7 +258,7 @@ fn esc(s: &str) -> String { s.replace('&', "&").replace('<', "<").replace('>', ">").replace('"', """) } -fn html_cell_css(wb: &Workbook, sh: &Sheet, c: CellRef) -> String { +fn html_cell_css(wb: &Workbook, sh: &Sheet, c: CellRef, dir: CellDir) -> String { let st = wb.styles.get(sh.style_id(c)); let mut css = String::new(); if st.font.bold { @@ -270,16 +277,59 @@ fn html_cell_css(wb: &Workbook, sh: &Sheet, c: CellRef) -> String { } let v = sh.value(c); let align = match st.align.h { - gridcraft_model::HAlign::Center => "center", - gridcraft_model::HAlign::Right => "right", - gridcraft_model::HAlign::Left => "left", - _ if v.is_number() => "right", - _ => "left", + gridcraft_model::HAlign::Center => Some("center"), + gridcraft_model::HAlign::Right => Some("right"), + gridcraft_model::HAlign::Left => Some("left"), + _ if v.is_number() => Some(if sh.right_to_left { "left" } else { "right" }), + _ if dir == CellDir::Auto => None, + _ => Some("left"), }; - let _ = write!(css, "text-align:{align};"); + if let Some(align) = align { + let _ = write!(css, "text-align:{align};"); + } css } +/// How a cell marks its direction in HTML. Text on a right-to-left sheet, or text with +/// right-to-left letters, follows its first strong character (`dir="auto"`), so General +/// alignment puts Arabic and Hebrew on the right. Numbers on a right-to-left sheet stay left to +/// right so `-5` doesn't read `5-`. +#[derive(Clone, Copy, PartialEq, Eq)] +enum CellDir { + Inherit, + Auto, + Ltr, +} + +impl CellDir { + fn of(sh: &Sheet, c: CellRef, text: &str) -> CellDir { + if sh.value(c).is_number() { + if sh.right_to_left { CellDir::Ltr } else { CellDir::Inherit } + } else if sh.right_to_left || text.chars().any(is_right_to_left_letter) { + CellDir::Auto + } else { + CellDir::Inherit + } + } + + fn attr(self) -> &'static str { + match self { + CellDir::Inherit => "", + CellDir::Auto => " dir=\"auto\"", + CellDir::Ltr => " dir=\"ltr\"", + } + } +} + +/// Hebrew, Arabic, Syriac, Thaana, N'Ko and their presentation forms. +fn is_right_to_left_letter(c: char) -> bool { + matches!(c, '\u{0590}'..='\u{08FF}' | '\u{FB1D}'..='\u{FDFF}' | '\u{FE70}'..='\u{FEFF}') +} + +fn table_dir(sh: &Sheet) -> &'static str { + if sh.right_to_left { " dir=\"rtl\"" } else { "" } +} + const CLIPBOARD_HTML_MAX_BYTES: usize = 4 * 1024 * 1024; const CLIPBOARD_HTML_MAX_CELLS: u64 = 10_000; @@ -292,7 +342,7 @@ pub(crate) fn range_html(wb: &Workbook, sheet: usize, r: RangeRef, plain_text: & } let sh = wb.sheet(sheet)?; let mut out = ClipboardHtml(String::new()); - out.write_str("").ok()?; + write!(out, "", table_dir(sh)).ok()?; for row in r.start.row..=r.end.row { if sh.is_row_hidden(row) { continue; @@ -310,11 +360,12 @@ pub(crate) fn range_html(wb: &Workbook, sheet: usize, r: RangeRef, plain_text: & } else { String::new() }; - let css = html_cell_css(wb, sh, c); - write!(out, "
").ok()?; // Use the visible selected cell, even when the merge's original anchor // is outside the range or hidden. This matches the plain-text copy. let text = crate::display::cell_text(wb, sh, c); + let dir = CellDir::of(sh, c, &text); + let css = html_cell_css(wb, sh, c, dir); + write!(out, "", dir.attr()).ok()?; let mut chars = text.chars().peekable(); while let Some(ch) = chars.next() { match ch { @@ -363,8 +414,9 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result { let mut out = String::new(); let _ = write!( out, - "{}", - esc(&sh.name) + "{}", + esc(&sh.name), + table_dir(sh) ); if let Some(r) = sh.used_range() { if r.height() > HTML_EXPORT_MAX_ROWS { @@ -383,14 +435,16 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result { if sh.merges.iter().any(|m| m.contains(c) && m.start != c) { continue; } - let css = html_cell_css(wb, sh, c); + let text = crate::display::cell_text(wb, sh, c); + let dir = CellDir::of(sh, c, &text); + let css = html_cell_css(wb, sh, c, dir); let span = sh .merges .iter() .find(|m| m.start == c) .map(|m| format!(" rowspan=\"{}\" colspan=\"{}\"", m.height(), m.width())) .unwrap_or_default(); - let _ = write!(out, "", esc(&crate::display::cell_text(wb, sh, c))); + let _ = write!(out, "", dir.attr(), esc(&text)); } out.push_str(""); } @@ -500,6 +554,108 @@ pub fn image_size(data: &[u8]) -> Option<(u32, u32)> { mod tests { use super::*; + fn book(rtl: bool, cells: &[(&str, gridcraft_core::Value, gridcraft_model::HAlign)]) -> Workbook { + let mut wb = Workbook::new(); + let ids: Vec<_> = cells.iter().map(|(_, _, h)| wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |st| st.align.h = *h)).collect(); + let sh = wb.sheet_mut(0).unwrap(); + sh.right_to_left = rtl; + for ((at, value, _), id) in cells.iter().zip(ids) { + let c = CellRef::parse(at).unwrap(); + sh.set_value(c, value.clone()); + sh.set_style(c, id); + } + wb + } + + fn copied(wb: &Workbook, range: &str) -> String { + let r = RangeRef::parse(range).unwrap(); + range_html(wb, 0, r, &crate::display::range_text(wb, 0, r)).unwrap() + } + + #[test] + fn html_export_marks_right_to_left_sheets() { + use gridcraft_core::Value; + use gridcraft_model::HAlign; + let wb = book(true, &[("A1", Value::text("Hello!"), HAlign::General), ("B1", Value::Number(-5.0), HAlign::General)]); + let html = to_html(&wb, 0).unwrap(); + assert!(html.contains("
{}{}
"), "{html}"); + assert!(html.contains(""), "Latin text keeps its own direction: {html}"); + assert!( + html.contains(""), + "numbers mirror to the left and keep their sign in front: {html}" + ); + let clip = copied(&wb, "A1:B1"); + assert!(clip.starts_with("
Hello!-5
-5"), "{clip}"); + + let ltr = book(false, &[("A1", Value::text("Hello!"), HAlign::General)]); + assert!(to_html(<r, 0).unwrap().contains("
")); + assert!(copied(<r, "A1").starts_with("
مرحبا!"), "General Arabic text aligns by its direction: {html}"); + assert!(html.contains(""), "explicit alignment stays: {html}"); + assert!(html.contains(""), "{html}"); + assert!(html.contains(""), "{html}"); + assert!(html.contains("
שלוםمرحباHello
"), "a left-to-right sheet has no table direction: {html}"); + + let clip = copied(&wb, "A1:D1"); + let cells: Vec<&str> = clip.split("
"), HAlign::General), + ("B2", Value::Number(3.0), HAlign::General), + ("A3", Value::text("Total"), HAlign::Right), + ], + ); + wb.sheet_mut(0).unwrap().merges.push(RangeRef::parse("A3:B3").unwrap()); + assert_eq!( + to_html(&wb, 0).unwrap(), + "Sheet1\ + \ + \ + \ +
NameQty
Apples & <pears>3
Total
" + ); + let cell = "border:1px solid #d4d4d4;padding:2px 4px;white-space:pre-wrap;"; + assert_eq!( + copied(&wb, "A1:B2"), + format!( + "\ + \ +
NameQty
Apples & <pears>3
" + ) + ); + } + #[test] fn html_export_fails_instead_of_dropping_cells_past_the_row_limit() { let mut wb = Workbook::new(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..2ac290654 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -253,6 +253,22 @@ fn undo_redo() { assert!(s.doc().unwrap().is_dirty()); } +#[test] +fn redo_selects_the_redone_cell() { + // #1855: type into A1..A3, click A1, undo, redo: the selection follows the edit to A3. + let mut s = s(); + for (cell, text) in [("A1", "first"), ("A2", "second"), ("A3", "third")] { + s.execute("selection.set", json!({"cell": cell})).unwrap(); + s.execute("cell.set", json!({"cell": cell, "input": text})).unwrap(); + } + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.doc().unwrap().selection.a1(), "A3"); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(v(&s, "A3"), Value::from("third")); + assert_eq!(s.doc().unwrap().selection.a1(), "A3"); +} + #[test] fn shape_text_edit_undo_redo_and_noop() { let mut s = s(); @@ -1206,6 +1222,26 @@ fn table_rename_keeps_a_defined_name_with_the_same_name() { assert_eq!((v(&s, "E1"), v(&s, "F1")), (Value::Number(100.0), Value::Number(7.0))); } +#[test] +fn table_names_are_limited_to_255_characters() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Qty"], [7]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:A2", "header": true, "name": "Original"})).unwrap(); + + let name_255 = "ع".repeat(255); + s.execute("table.rename", json!({"table": "Original", "name": name_255})).unwrap(); + assert!(s.doc().unwrap().wb.table(&"ع".repeat(255)).is_some()); + + let name_256 = "ع".repeat(256); + let error = s.execute("table.rename", json!({"table": &"ع".repeat(255), "name": name_256})).unwrap_err(); + assert!(error.to_string().contains("not valid"), "{error}"); + assert!(s.doc().unwrap().wb.table(&"ع".repeat(255)).is_some()); + + let name_200 = "ع".repeat(200); + s.execute("table.rename", json!({"table": &"ع".repeat(255), "name": name_200})).unwrap(); + assert!(s.doc().unwrap().wb.table(&"ع".repeat(200)).is_some()); +} + #[test] fn slicer_filters_table() { let mut s = s(); @@ -3282,3 +3318,29 @@ fn paste_special_validation_moves_relative_references() { assert!(!ok(&mut s, "C3", "5"), "the pasted rule checks C3, not A1"); assert!(ok(&mut s, "C3", "20")); } + +#[test] +fn csv_as_text_keeps_identifier_columns_as_written() { + let csv = "gtin,zip,flag,day\n8435392892278,01234,TRUE,2026-03-04\n"; + let open = |s: &mut Session, extra: serde_json::Value| { + let mut p = json!({"name": "feed.csv", "base64": crate::io::base64_encode(csv.as_bytes())}); + if let (Some(p), Some(extra)) = (p.as_object_mut(), extra.as_object()) { + p.extend(extra.clone()); + } + s.execute("file.open", p).unwrap(); + }; + let value = |s: &mut Session, cell: &str| s.execute("cell.get", json!({"cell": cell})).unwrap()["value"].clone(); + // By default fields are typed like typed input: the leading zero is lost and the GTIN is a number. + let mut s = s(); + open(&mut s, json!({})); + assert!(value(&mut s, "A2").is_number()); + assert_eq!(value(&mut s, "B2").as_f64(), Some(1234.0)); + // With csvAsText every field stays the text the file holds, and saves back unchanged. + let mut s = self::s(); + open(&mut s, json!({"csvAsText": true})); + for (cell, text) in [("A2", "8435392892278"), ("B2", "01234"), ("C2", "TRUE"), ("D2", "2026-03-04")] { + assert_eq!(value(&mut s, cell), json!(text), "{cell}"); + } + let saved = s.execute("file.saveBytes", json!({"format": "csv"})).unwrap(); + assert_eq!(crate::io::base64_decode(saved["base64"].as_str().unwrap()).unwrap(), csv.replace('\n', "\r\n").as_bytes()); +} diff --git a/crates/engine/tests/custom_filter_blanks.rs b/crates/engine/tests/custom_filter_blanks.rs new file mode 100644 index 000000000..9bfd395d2 --- /dev/null +++ b/crates/engine/tests/custom_filter_blanks.rs @@ -0,0 +1,20 @@ +use gridcraft_engine::Session; +use serde_json::json; + +fn visible_rows(op: &str) -> Vec { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range":"A1", "values":[["n"], ["3"], ["10"], [""], ["abc1"], ["20"]]})).unwrap(); + s.execute("selection.set", json!({"range":"A1:A6"})).unwrap(); + s.execute("data.filter", json!({})).unwrap(); + s.execute("data.filterBy", json!({"column":0, "custom":{"op":op, "value":"5"}})).unwrap(); + let doc = s.doc().unwrap(); + let sh = doc.wb.active().unwrap(); + (1..6u32).filter(|r| !sh.rows.get(r).is_some_and(|i| i.hidden)).collect() +} + +#[test] +fn less_than_numeric_criterion_hides_blank_and_text_rows() { + assert_eq!(visible_rows("<"), vec![1]); + assert_eq!(visible_rows("<="), vec![1]); +} diff --git a/crates/engine/tests/pivot_unknown_func.rs b/crates/engine/tests/pivot_unknown_func.rs new file mode 100644 index 000000000..f4d2427f0 --- /dev/null +++ b/crates/engine/tests/pivot_unknown_func.rs @@ -0,0 +1,33 @@ +use gridcraft_engine::Session; +use serde_json::json; + +fn session() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range":"A1", "values":[["Region", "Sales"], ["East", "10"], ["East", "2"], ["West", "4"]]})).unwrap(); + s +} + +#[test] +fn insert_pivot_table_rejects_unknown_func() { + for func in ["stdev", "sumx"] { + let mut s = session(); + let r = s.execute( + "insert.pivotTable", + json!({"source":"Sheet1!A1:B4", "destination":"Sheet1!F1", "rows":["Region"], "values":[{"field":"Sales", "func":func}]}), + ); + assert!(r.is_err(), "unknown func `{func}` accepted silently"); + assert!(s.doc().unwrap().wb.active().unwrap().pivots.is_empty()); + } +} + +#[test] +fn insert_pivot_table_accepts_known_func() { + let mut s = session(); + s.execute( + "insert.pivotTable", + json!({"source":"Sheet1!A1:B4", "destination":"Sheet1!F1", "rows":["Region"], "values":[{"field":"Sales", "func":"stddev"}]}), + ) + .unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().pivots.len(), 1); +} diff --git a/crates/engine/tests/range_fill_formats.rs b/crates/engine/tests/range_fill_formats.rs new file mode 100644 index 000000000..ce1d19fb6 --- /dev/null +++ b/crates/engine/tests/range_fill_formats.rs @@ -0,0 +1,93 @@ +//! Ctrl+Enter (`range.fill`) gives every cell what typing the input there gives it: a General +//! cell gets the input's automatic number format, a formatted cell keeps its own. + +use gridcraft_engine::Session; +use serde_json::{Value, json}; + +fn session() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s +} + +fn get(s: &mut Session, a: &str) -> Value { + s.execute("cell.get", json!({"cell": a})).unwrap() +} + +fn fmt_of(s: &Session, a: &str) -> String { + let wb = &s.doc().unwrap().wb; + let sh = wb.active().unwrap(); + wb.styles.get(sh.style_id(gridcraft_engine::core::CellRef::parse(a).unwrap())).num_fmt.as_str().to_string() +} + +/// Selects `range` with A1 active and fills it with `input`, as Ctrl+Enter does. +fn fill(s: &mut Session, range: &str, input: &str) { + s.execute("selection.set", json!({"range": range, "active": "A1"})).unwrap(); + s.execute("range.fill", json!({"input": input})).unwrap(); +} + +#[test] +fn filled_cells_that_already_hold_a_value_get_the_automatic_format() { + for input in ["5%", "$5", "3/4/2026", "1:30 PM", "1.5E+3"] { + let mut s = session(); + // What typing the input into a General cell gives. + s.execute("cell.set", json!({"cell": "C1", "input": input})).unwrap(); + let typed = get(&mut s, "C1"); + // A1 blank, A2 holding a number: both are General. + s.execute("cell.set", json!({"cell": "A2", "input": "1"})).unwrap(); + fill(&mut s, "A1:A2", input); + for a in ["A1", "A2"] { + let filled = get(&mut s, a); + assert_eq!(filled["text"], typed["text"], "{input} in {a}"); + assert_eq!(filled["value"], typed["value"], "{input} in {a}"); + assert_eq!(fmt_of(&s, a), fmt_of(&s, "C1"), "{input} in {a}"); + } + } +} + +#[test] +fn filled_cells_with_their_own_format_keep_it() { + let mut s = session(); + s.execute("cell.set", json!({"cell": "A2", "input": "1"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A2", "style": {"num_fmt": "0.000"}})).unwrap(); + // Column B is formatted but has no cells yet; row 3 is Text. + s.execute("home.formatCells", json!({"range": "B:B", "style": {"num_fmt": "0.00"}})).unwrap(); + s.execute("home.formatCells", json!({"range": "3:3", "style": {"num_fmt": "@"}})).unwrap(); + fill(&mut s, "A1:B3", "5%"); + assert_eq!(fmt_of(&s, "A1"), "0%"); + assert_eq!(get(&mut s, "A1")["text"], "5%"); + assert_eq!(fmt_of(&s, "A2"), "0.000"); + assert_eq!(get(&mut s, "A2")["text"], "0.050"); + assert_eq!(fmt_of(&s, "B1"), "0.00"); + assert_eq!(get(&mut s, "B1")["text"], "0.05"); + // A Text cell keeps what was typed, as text. + for a in ["A3", "B3"] { + assert_eq!(get(&mut s, a)["type"], "text", "{a}"); + assert_eq!(get(&mut s, a)["text"], "5%", "{a}"); + } +} + +#[test] +fn filled_formulas_still_shift_and_get_their_date_format() { + let mut s = session(); + s.execute("cell.set", json!({"cell": "A2", "input": "1"})).unwrap(); + fill(&mut s, "A1:A2", "=DATE(2026,3,ROW())"); + assert_eq!(get(&mut s, "A2")["formula"], "=DATE(2026,3,ROW())"); + for (a, text) in [("A1", "3/1/2026"), ("A2", "3/2/2026")] { + assert_eq!(fmt_of(&s, a), "m/d/yyyy", "{a}"); + assert_eq!(get(&mut s, a)["text"], text, "{a}"); + } + fill(&mut s, "A1:A2", "=C1*2"); + assert_eq!(get(&mut s, "A2")["formula"], "=C2*2"); +} + +#[test] +fn a_fill_undoes_in_one_step() { + let mut s = session(); + s.execute("cell.set", json!({"cell": "A2", "input": "1"})).unwrap(); + fill(&mut s, "A1:A2", "5%"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(get(&mut s, "A1")["type"], "empty"); + assert_eq!(get(&mut s, "A2")["value"].as_f64(), Some(1.0)); + assert_eq!(fmt_of(&s, "A2"), "General"); +} diff --git a/crates/functions/src/stat.rs b/crates/functions/src/stat.rs index 7c50abeaa..9d8642e24 100644 --- a/crates/functions/src/stat.rs +++ b/crates/functions/src/stat.rs @@ -54,7 +54,7 @@ fn median(v: Vec) -> R { let s = sorted(v); let n = s.len(); let hi = s.get(n / 2).copied().unwrap_or(0.0); - if n % 2 == 1 { Ok(hi) } else { Ok((s.get(n / 2 - 1).copied().unwrap_or(0.0) + hi) / 2.0) } + if n % 2 == 1 { Ok(hi) } else { Ok(s.get(n / 2 - 1).copied().unwrap_or(0.0).midpoint(hi)) } } /// Values occurring at least twice, ordered by first occurrence, with their (equal, maximal) count. @@ -2255,6 +2255,46 @@ mod tests { av(col(xs)) } + // The two middle values are finite and their midpoint is representable even when + // adding them first would overflow. Exercise both callers of the shared helper. + #[test] + fn median_finite_midpoint_does_not_overflow() { + for (values, expected) in [ + (vec![1e308, 1e308], 1e308), + (vec![-1e308, -1e308], -1e308), + (vec![1.6e308, 1e308], 1.3e308), + (vec![-1e308, -1.6e308], -1.3e308), + (vec![f64::MAX, f64::MAX], f64::MAX), + (vec![-f64::MAX, -f64::MAX], -f64::MAX), + (vec![-f64::MAX, f64::MAX], 0.0), + (vec![1.7e308, -1.7e308, 1e308, 1.6e308], 1.3e308), + ] { + close(ev("MEDIAN", values.iter().copied().map(n).collect()), expected); + close(ev("MEDIAN", vec![rf(col(&values))]), expected); + close(aggregate_values(12, &values.iter().copied().map(nv).collect::>()), expected); + } + } + + #[test] + fn median_midpoint_keeps_small_values_and_argument_semantics() { + // Halving each operand unconditionally can lose the smallest numbers. Use exact + // equality here: the usual aggregate-test tolerance would accept zero. + for value in [f64::MIN_POSITIVE, -f64::MIN_POSITIVE, f64::from_bits(1), -f64::from_bits(1)] { + assert_eq!(ev("MEDIAN", vec![n(value), n(value)]), nv(value)); + assert_eq!(aggregate_values(12, &[nv(value), nv(value)]), nv(value)); + } + close(ev("MEDIAN", vec![c(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0])]), 3.5); + close(ev("MEDIAN", vec![c(&[1e308, -1e308, 3.0])]), 3.0); + close(ev("MEDIAN", vec![n(1e308)]), 1e308); + close(ev("MEDIAN", vec![t("4"), b(true)]), 2.5); + close(ev("MEDIAN", vec![rf(arr(vec![vec![nv(1.0), tv("ignored"), Value::Empty, Value::Bool(true), nv(6.0)]]))]), 3.5); + is_err(ev("MEDIAN", vec![t("invalid")]), CellError::Value); + is_err(ev("MEDIAN", vec![e(CellError::NA), n(1e308)]), CellError::NA); + is_err(ev("MEDIAN", vec![rf(arr(vec![vec![tv("ignored"), Value::Empty]]))]), CellError::Num); + is_err(aggregate_values(12, &[]), CellError::Num); + is_err(aggregate_values(12, &[nv(1e308), Value::Error(CellError::Div0)]), CellError::Div0); + } + #[test] fn averages_and_counts() { close(ev("AVERAGE", vec![n(1.0), n(2.0), n(6.0)]), 3.0); diff --git a/crates/functions/src/text.rs b/crates/functions/src/text.rs index 91962d6e7..621c9b53b 100644 --- a/crates/functions/src/text.rs +++ b/crates/functions/src/text.rs @@ -505,16 +505,33 @@ fn delimiters(v: &Value) -> R>> { as_array(v).iter().map(|d| d.to_text().map(|s| chars(&s))).collect() } -/// Non-overlapping delimiter matches (start, end) in char positions, scanning left to right and -/// preferring the longest delimiter at each position. Empty delimiters are ignored unless all +/// Non-overlapping delimiter matches (start, end) in char positions, in ascending order. Scans left +/// to right preferring the longest delimiter starting at each position, or (`from_end`) right to +/// left preferring the longest delimiter ending at each position. Empty delimiters are ignored unless all /// delimiters are empty, in which case every position matches with zero length. -fn find_matches(text: &[char], delims: &[Vec], insensitive: bool) -> Vec<(usize, usize)> { +fn find_matches(text: &[char], delims: &[Vec], insensitive: bool, from_end: bool) -> Vec<(usize, usize)> { let t = if insensitive { fold(text) } else { text.to_vec() }; let ds: Vec> = delims.iter().filter(|d| !d.is_empty()).map(|d| if insensitive { fold(d) } else { d.clone() }).collect(); if ds.is_empty() { return (0..=t.len()).map(|i| (i, i)).collect(); } let mut out = Vec::new(); + if from_end { + let mut j = t.len(); + while j > 0 { + let best = + ds.iter().filter(|d| j.checked_sub(d.len()).and_then(|st| t.get(st..j)).is_some_and(|w| w == d.as_slice())).map(|d| d.len()).max(); + match best { + Some(l) => { + out.push((j - l, j)); + j -= l; + } + None => j -= 1, + } + } + out.reverse(); + return out; + } let mut i = 0; while i < t.len() { let best = ds.iter().filter(|d| t.get(i..i + d.len()).is_some_and(|w| w == d.as_slice())).map(|d| d.len()).max(); @@ -538,7 +555,7 @@ fn text_before_after(a: &[Arg], before: bool) -> R { if !(0..=1).contains(&mode) || inst == 0 || inst.unsigned_abs() as usize > s.len().max(1) { return Err(CellError::Value); } - let mut m = find_matches(&s, &delims, mode == 1); + let mut m = find_matches(&s, &delims, mode == 1, inst < 0); if match_end { if inst > 0 { if m.last().is_none_or(|x| x.0 != s.len()) { @@ -573,7 +590,7 @@ fn split_by(s: &[char], delims: &[Vec], insensitive: bool) -> Vec, /// `None`: the shape's default (top for text boxes, middle for other shapes). pub anchor: Option, + /// Paragraphs run right to left (`a:pPr rtl`). + pub rtl: bool, } #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] diff --git a/crates/ui-egui/src/desktop_theme.rs b/crates/ui-egui/src/desktop_theme.rs index 2eb5d8371..9d67c337a 100644 --- a/crates/ui-egui/src/desktop_theme.rs +++ b/crates/ui-egui/src/desktop_theme.rs @@ -16,7 +16,7 @@ const POLL: Duration = Duration::from_secs(4); /// there is no session bus or no portal to ask (a bare X server, a container), so the caller /// keeps whatever it chose itself. pub fn prefers_dark() -> Option { - read() + read(&mut Link::default()) } /// Watches the desktop and reports its choice when it changes. [`prefers_dark`] covers the @@ -33,13 +33,14 @@ impl DesktopTheme { pub fn start() -> Self { let (tx, rx) = channel(); let started = std::thread::Builder::new().name("gridcraft-desktop-theme".into()).spawn(move || { - let mut last = read(); + let mut link = Link::default(); + let mut last = read(&mut link); if tx.send(last).is_err() { return; } loop { std::thread::sleep(POLL); - let now = read(); + let now = read(&mut link); if now != last { last = now; if tx.send(now).is_err() { @@ -66,17 +67,32 @@ impl DesktopTheme { } } -/// Ask the portal once. D-Bus calls block, so this belongs off the UI thread: the watcher's -/// thread is its only caller. +/// The session-bus connection the watcher keeps between reads, opened on first use. #[cfg(target_os = "linux")] -fn read() -> Option { +type Link = Option; +#[cfg(not(target_os = "linux"))] +type Link = (); + +/// Ask the portal once, over `link`'s connection (opened if absent, dropped if the bus went +/// away). D-Bus calls block, so this belongs off the UI thread: the watcher's thread is its +/// only repeated caller. +#[cfg(target_os = "linux")] +fn read(link: &mut Link) -> Option { const PORTAL: &str = "org.freedesktop.portal.Desktop"; const SETTINGS: &str = "org.freedesktop.portal.Settings"; const SCHEMAS: [(&str, &str); 2] = [("org.gnome.desktop.interface", "color-scheme"), ("org.freedesktop.interface.color-scheme", "color-scheme")]; - let connection = zbus::blocking::Connection::session().ok()?; + if link.is_none() { + *link = Some(zbus::blocking::Connection::session().ok()?); + } + let connection = link.as_ref()?; for (schema, key) in SCHEMAS { - let Ok(reply) = connection.call_method(Some(PORTAL), "/org/freedesktop/portal/desktop", Some(SETTINGS), "ReadOne", &(schema, key)) else { - continue; + let reply = match connection.call_method(Some(PORTAL), "/org/freedesktop/portal/desktop", Some(SETTINGS), "ReadOne", &(schema, key)) { + Ok(reply) => reply, + Err(zbus::Error::InputOutput(_)) => { + *link = None; + return None; + } + Err(_) => continue, }; let Ok((answer,)) = reply.body().deserialize::<(zbus::zvariant::OwnedValue,)>() else { continue }; if let Ok(scheme) = String::try_from(&*answer) { @@ -99,7 +115,7 @@ fn parse(scheme: &str) -> Option { /// macOS and Windows answer through winit, which the caller already has. #[cfg(not(target_os = "linux"))] -fn read() -> Option { +fn read(_: &mut Link) -> Option { None } diff --git a/crates/ui-egui/src/editor.rs b/crates/ui-egui/src/editor.rs index 4aada2953..799109f89 100644 --- a/crates/ui-egui/src/editor.rs +++ b/crates/ui-egui/src/editor.rs @@ -238,6 +238,13 @@ pub fn formula_refs(text: &str) -> Vec<(usize, usize, RangeRef, Option, out } +/// Whether a reference of the formula being edited belongs on the sheet shown: a reference with a +/// sheet name is on that sheet, one without is on the sheet the edit started on, even while +/// pointing at another sheet. +pub fn ref_is_on_sheet(ref_sheet: Option<&str>, origin: &str, shown: &str) -> bool { + ref_sheet.unwrap_or(origin).eq_ignore_ascii_case(shown) +} + /// Excel-style AutoComplete: the one distinct text entry in the same column (contiguous data /// above and below) that starts with `typed` (case-insensitive). `None` when ambiguous. pub fn column_completion(sheet: &gridcraft_engine::model::Sheet, at: CellRef, typed: &str) -> Option { @@ -329,4 +336,14 @@ mod tests { assert_eq!(r[1].2.a1(), "B2:C3"); assert_eq!(r[3].3.as_deref(), Some("Sheet2")); } + + #[test] + fn refs_are_outlined_on_their_own_sheet_only() { + // Pointing at Sheet2 while the edit started on Sheet1: `A1` stays on Sheet1, `Sheet2!B2` shows on Sheet2. + assert!(ref_is_on_sheet(None, "Sheet1", "Sheet1")); + assert!(!ref_is_on_sheet(None, "Sheet1", "Sheet2")); + assert!(ref_is_on_sheet(Some("Sheet2"), "Sheet1", "Sheet2")); + assert!(!ref_is_on_sheet(Some("Sheet2"), "Sheet1", "Sheet1")); + assert!(ref_is_on_sheet(Some("sheet2"), "Sheet1", "Sheet2"), "sheet names compare ignoring case"); + } } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 00e0c18be..106bc6684 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -149,6 +149,8 @@ pub struct GridState { pub hscroll: Option, /// Ctrl+wheel zoom in progress (see [`WheelZoom`]). pub wheel_zoom: Option, + /// A one-finger touch drag in progress: it pans the sheet instead of selecting. + pub touch_pan: bool, } /// Ctrl+wheel zoom for one (doc uid, sheet). egui eases a notch in over several frames and @@ -482,6 +484,10 @@ fn look(wb: &Workbook, si: usize, sh: &Sheet, c: CellRef, cf: &mut CfCache) -> ( if st.font.strike { style.font.strike = true; } + // In a differential style General means "not set" (#1163). + if st.num_fmt.as_str() != "General" { + style.num_fmt = st.num_fmt.clone(); + } for (dst, src) in [ (&mut style.borders.left, st.borders.left), (&mut style.borders.right, st.borders.right), @@ -637,6 +643,19 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { view.scroll -= d / z; } } + // A one-finger touch drag pans the sheet, the content following the finger; it doesn't select. + // (Two fingers pinch-zoom through `zoom_delta` above.) + if resp.drag_started() && ui.input(|i| i.any_touches()) { + app.grid.touch_pan = true; + } + if app.grid.touch_pan && resp.dragged() && ui.input(|i| i.multi_touch().is_none()) { + let z = if paper { make_geo(app, view.scroll).z } else { sh.zoom as f32 / 100.0 * DISPLAY_SCALE }; + let mut d = resp.drag_delta(); + if sh.right_to_left { + d.x = -d.x; + } + 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). @@ -750,8 +769,19 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { } // Interaction. let previous_popups = (app.grid.filter_menu, app.grid.list_picker, app.grid.header_menu); - crate::paper_view::interact(app, ui, &resp, &geo, sh, &wb); - interact(app, ui, &resp, &geo, sh, &wb); + // A pan the grid never saw end (it was hidden mid-drag) must not swallow the next press. + if app.grid.touch_pan && !resp.dragged() && !resp.drag_stopped() { + app.grid.touch_pan = false; + } + if app.grid.touch_pan { + // Skip the frames of a touch pan, its release included. + if !resp.dragged() { + app.grid.touch_pan = false; + } + } else { + crate::paper_view::interact(app, ui, &resp, &geo, sh, &wb); + interact(app, ui, &resp, &geo, sh, &wb); + } crate::text_box::show(app, ui, &geo, sh); if app.editor.as_ref().is_some_and(|e| !e.from_formula_bar) { in_cell_editor(app, ui, &geo, sh, &wb); @@ -1174,7 +1204,7 @@ pub(crate) fn paint_quadrant( _ if numeric => HAlign::Right, _ => HAlign::Left, }, - h => h, + h => painted_halign(h, geo.rtl), }; let indent = st.align.indent as f32 * 9.0 * z; let pad = 2.5 * z; @@ -1292,7 +1322,7 @@ pub(crate) fn paint_quadrant( } else { galley.clone() }; - let pos = rotated_text_pos(*rect, galley.size(), angle, st.align.h, st.align.v, pad, indent); + let pos = rotated_text_pos(*rect, galley.size(), angle, painted_halign(st.align.h, geo.rtl), st.align.v, pad, indent); let mut shape = egui::epaint::TextShape::new(pos, galley, color); shape.angle = angle; cp.add(shape); @@ -1619,8 +1649,9 @@ pub(crate) fn paint_selection( if let Some(ed) = &app.editor && ed.is_formula() { + let origin = app.session.active().and_then(|d| d.wb.sheet(ed.sheet)).map_or(sh.name.as_str(), |o| o.name.as_str()); for (_, _, r, sheet, idx) in crate::editor::formula_refs(&ed.text) { - if sheet.as_deref().is_some_and(|s| !s.eq_ignore_ascii_case(&sh.name)) { + if !crate::editor::ref_is_on_sheet(sheet.as_deref(), origin, &sh.name) { continue; } if !range_in_pane(r, rows, cols) { @@ -3332,8 +3363,34 @@ mod selection_fill_tests { } } +/// The horizontal alignment to paint with. Cells are painted left to right and a right-to-left +/// sheet is mirrored afterwards, so an explicit Left or Right is swapped first to end up on the +/// side the user chose: Align Right stays on the right in either sheet direction, as in Excel. +/// General alignment is resolved from the value before this and follows the sheet direction. +fn painted_halign(h: HAlign, rtl: bool) -> HAlign { + match h { + HAlign::Left if rtl => HAlign::Right, + HAlign::Right if rtl => HAlign::Left, + h => h, + } +} + #[cfg(test)] mod tests { + + #[test] + fn explicit_left_and_right_keep_their_side_on_a_right_to_left_sheet() { + use super::{HAlign, painted_halign}; + // Painted before the mirror: swapped, so they land where the user put them. + assert_eq!(painted_halign(HAlign::Right, true), HAlign::Left); + assert_eq!(painted_halign(HAlign::Left, true), HAlign::Right); + // Left-to-right sheets and the other alignments are painted as they are. + assert_eq!(painted_halign(HAlign::Right, false), HAlign::Right); + assert_eq!(painted_halign(HAlign::Left, false), HAlign::Left); + for h in [HAlign::Center, HAlign::Justify, HAlign::Fill] { + assert_eq!(painted_halign(h, true), h); + } + } use super::*; #[test] @@ -3406,6 +3463,28 @@ mod tests { assert_eq!(center_across_end(0, 1, |_| true), 1); } + #[test] + fn conditional_number_format_reaches_the_cell_text() { + // #1163: a rule's number format replaces the cell's own; unmatched cells keep theirs. + let mut session = gridcraft_engine::Session::new(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "A1", "values": [[1], [3], [4]]})).unwrap(); + session.execute("home.numberFormat", json!({"range": "A3", "code": "0.0"})).unwrap(); + let rule = json!({"type": "cellIs", "operator": "greater", "value": "2", "style": {"num_fmt": "0.00"}}); + session.execute("home.conditionalFormat", json!({"range": "A1:A3", "rule": rule})).unwrap(); + let wb = session.active().unwrap().wb.clone(); + let sh = wb.sheet(0).unwrap(); + let mut cf = CfCache::new(); + let text = |row: u32, cf: &mut CfCache| { + let c = CellRef::new(row, 0); + let (lk, _) = look(&wb, 0, sh, c, cf); + display_text(&wb, sh, c, &sh.value(c), &lk.style, 200.0, 7.0).0 + }; + assert_eq!(text(0, &mut cf), "1"); + assert_eq!(text(1, &mut cf), "3.00"); + assert_eq!(text(2, &mut cf), "4.00"); + } + #[test] fn filter_menu_uses_the_table_whose_header_was_clicked() { use gridcraft_engine::model::Table; diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 061698df9..98919f8a4 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -647,6 +647,59 @@ mod tests { assert_eq!(Language::parse("xx"), None); } + #[test] + fn german_cell_and_header_context_menus_are_fully_translated() { + for label in [ + "Cut", + "Copy", + "Paste", + "Paste Special…", + "Insert…", + "Delete…", + "Clear Contents", + "Filter by Selected Cell's Value", + "Sort A to Z", + "Sort Z to A", + "New Comment", + "New Note", + "Format Cells…", + "Pick From Drop-down List…", + "Define Name…", + "Insert Copied Cells", + "Insert Copied Cells…", + "Insert Cut Cells", + "Insert", + "Delete", + "Row Height…", + "AutoFit Row Height", + "Column Width…", + "AutoFit Column Width", + "Hide", + "Unhide", + "Group", + ] { + assert_ne!(Language::De.tr(label), label, "German context menu label missing: {label}"); + } + } + + #[test] + fn translations_never_show_the_context_prefix() { + // `tr` returns a found translation as it is, so a catalog that keeps `context|` in its + // value shows it on screen (File › Options read "Options|Général" in French). + for language in Language::ALL { + for (english, _) in JAPANESE { + let shown = language.tr(english); + assert!(!shown.contains('|'), "{} shows a context prefix for {english}: {shown}", language.code()); + } + } + assert_eq!(Language::Fr.tr("Options|General"), "Général"); + assert_eq!(Language::Fr.tr("Options|Save"), "Enregistrer"); + assert_eq!(Language::Fr.tr("Sheet Options|View"), "Affichage"); + assert_eq!(Language::Sr.tr("Options|General"), "Opšte"); + assert_eq!(Language::Sr.tr("Options|Save"), "Sačuvaj"); + assert_eq!(Language::Sr.tr("Sheet Options|View"), "Prikaz"); + } + #[test] fn unknown_saved_language_keeps_the_other_preferences() { let ui: crate::UiState = serde_json::from_str(r#"{"dark": true, "language": "xx"}"#).unwrap(); diff --git a/crates/ui-egui/src/i18n/de.rs b/crates/ui-egui/src/i18n/de.rs index c20c7d1b3..371ea7981 100644 --- a/crates/ui-egui/src/i18n/de.rs +++ b/crates/ui-egui/src/i18n/de.rs @@ -411,6 +411,18 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Dark" => "Dunkel", "Sheet Options|View" => "Anzeige", "Print" => "Drucken", + // grid.rs cell and row/column header context menus. + "Cut" => "Ausschneiden", + "Copy" => "Kopieren", + "Insert…" => "Einfügen…", + "Delete…" => "Löschen…", + "Filter by Selected Cell's Value" => "Nach Wert der gewählten Zelle filtern", + "New Comment" => "Neuer Kommentar", + "Pick From Drop-down List…" => "Aus Dropdownliste auswählen…", + "Define Name…" => "Namen definieren…", + "Link…" => "Link…", + "Hide" => "Ausblenden", + "Unhide" => "Einblenden", _ => return None, }) } diff --git a/crates/ui-egui/src/i18n/fr.rs b/crates/ui-egui/src/i18n/fr.rs index 5d20ab0ca..82027e847 100644 --- a/crates/ui-egui/src/i18n/fr.rs +++ b/crates/ui-egui/src/i18n/fr.rs @@ -305,7 +305,7 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Lookup &\nReference" => "Recherche et\nréférence", "Math &\nTrig" => "Maths et\ntrigonométrie", "More\nFunctions" => "Autres\nfonctions", - "Sheet Options|View" => "Options de la feuille|Affichage", + "Sheet Options|View" => "Affichage", "Print" => "Impression", "Click to add data" => "Cliquez pour ajouter des données", "Add header" => "Ajouter un en-tête", @@ -402,8 +402,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Save the active sheet as comma-separated text." => "Enregistrer la feuille active sous forme de texte séparé par des points-virgules.", "Web Page (.html)" => "Page web (.html)", "Save the workbook as a web page." => "Enregistrer le classeur en tant que page web.", - "Options|General" => "Options|Général", - "Options|Save" => "Options|Enregistrer", + "Options|General" => "Général", + "Options|Save" => "Enregistrer", "Show formula bar" => "Afficher la barre de formule", "Enable AutoComplete for cell values" => "Activer la saisie semi-automatique des valeurs de cellule", "Save .xlsx workbooks automatically after every change" => "Enregistrer automatiquement les classeurs .xlsx après chaque modification", diff --git a/crates/ui-egui/src/i18n/sr.rs b/crates/ui-egui/src/i18n/sr.rs index cbf18d4c4..85bd2da72 100644 --- a/crates/ui-egui/src/i18n/sr.rs +++ b/crates/ui-egui/src/i18n/sr.rs @@ -297,7 +297,7 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Lookup &\nReference" => "Pretraga i\nreference", "Math &\nTrig" => "Matematika i\ntrigonometrija", "More\nFunctions" => "Još\nfunkcija", - "Sheet Options|View" => "Opcije lista|Prikaz", + "Sheet Options|View" => "Prikaz", "Print" => "Štampaj", "Click to add data" => "Kliknite da dodate podatke", "Add header" => "Dodaj zaglavlje", @@ -394,8 +394,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Save the active sheet as comma-separated text." => "Sačuvaj aktivni list kao tekst odvojen zapetama.", "Web Page (.html)" => "Veb stranica (.html)", "Save the workbook as a web page." => "Sačuvaj radnu svesku kao veb stranicu.", - "Options|General" => "Opcije|Opšte", - "Options|Save" => "Opcije|Sačuvaj", + "Options|General" => "Opšte", + "Options|Save" => "Sačuvaj", "Show formula bar" => "Prikaži traku formule", "Enable AutoComplete for cell values" => "Omogući automatsko dovršavanje vrednosti ćelija", "Save .xlsx workbooks automatically after every change" => "Automatski sačuvaj .xlsx radne sveske posle svake izmene", diff --git a/crates/ui-egui/src/keytips.rs b/crates/ui-egui/src/keytips.rs index e1163d9bb..eda675e7b 100644 --- a/crates/ui-egui/src/keytips.rs +++ b/crates/ui-egui/src/keytips.rs @@ -24,6 +24,8 @@ pub enum Action { pub struct KeyTips { prefix: Option, alt_armed: bool, + // Left Alt is down: Alt+H (held, as Excel allows) types keytip letters. + alt_held: bool, // Keep consuming a held mnemonic after its action closes keytips. held: Vec, } @@ -36,6 +38,7 @@ impl KeyTips { pub fn clear(&mut self) { self.prefix = None; self.alt_armed = false; + self.alt_held = false; self.held.clear(); } @@ -61,6 +64,9 @@ impl KeyTips { Event::PointerButton { pressed: true, .. } | Event::MouseWheel { .. } | Event::WindowFocused(false) | Event::Ime(_) => self.clear(), Event::Copy | Event::Cut | Event::Paste(_) => self.clear(), Event::ModifiersChanged(m) => { + if !m.alt { + self.alt_held = false; + } if command_modifier(*m) || (mac && m.alt) { self.clear(); } else if m.shift { @@ -68,6 +74,7 @@ impl KeyTips { } } Event::Key { key: Key::AltLeft, pressed, repeat, modifiers, .. } if !mac && !command_modifier(*modifiers) && !modifiers.shift => { + self.alt_held = *pressed; if *pressed { if !repeat { self.alt_armed = true; @@ -80,8 +87,20 @@ impl KeyTips { } Event::Key { key, pressed, repeat, modifiers, .. } => { self.alt_armed = false; + // A letter typed with left Alt held continues a sequence (Alt+H, O, A), as + // in Excel; any other Alt combination cancels keytips. + let alt_letter = + self.alt_held && modifiers.alt && !modifiers.shift && !command_modifier(*modifiers) && key.name().len() == 1 && { + let next = format!("{}{}", self.prefix.as_deref().unwrap_or_default(), key.name()); + keytip_table::lookup(&next).is_some() || keytip_table::has_deeper(&next) + }; + if alt_letter && *pressed && self.prefix.is_none() { + self.prefix = Some(String::new()); + } + // The release of that letter still carries Alt. + let alt_release = !*pressed && self.held.contains(key); if command_modifier(*modifiers) - || modifiers.alt + || (modifiers.alt && !alt_letter && !alt_release) || matches!(key, Key::AltLeft | Key::AltRight | Key::ControlLeft | Key::ControlRight | Key::SuperLeft | Key::SuperRight) { self.clear(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f96cdac3c..f549aec2b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -209,9 +209,31 @@ pub struct Perf { last: f64, } +/// Which parts of the app's own window chrome are drawn. Everything is on by default; a host +/// that embeds the sheet in its own window (and owns files itself) can turn off the title bar +/// and the File tab, whose Backstage view then never opens. Not persisted: it describes the +/// host, not a user preference. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Chrome { + /// The title bar: AutoSave, Home, Save, Undo/Redo, the file name, Comments, Share, Search and + /// the theme switch. + pub title_bar: bool, + /// The ribbon's File tab and the Backstage view it opens (also reached by keyboard and + /// commands such as `ui.backstage`). + pub file_tab: bool, +} + +impl Default for Chrome { + fn default() -> Self { + Chrome { title_bar: true, file_tab: true } + } +} + pub struct SheetApp { pub session: Session, pub ui: UiState, + /// The window chrome to draw (see [`Chrome`]). + pub chrome: Chrome, pub views: HashMap<(u64, usize), SheetView>, pub editor: Option, pub text_box_editor: Option, @@ -270,6 +292,7 @@ impl SheetApp { SheetApp { session, ui: UiState::default(), + chrome: Chrome::default(), views: HashMap::new(), editor: None, text_box_editor: None, @@ -752,6 +775,9 @@ impl SheetApp { /// Opens the File tab's Backstage view on `page`, committing a cell still being edited first /// (File actions work on the whole workbook). A draft that can't be committed keeps the sheet. pub fn open_backstage(&mut self, page: backstage::Page) { + if !self.chrome.file_tab { + return; + } if !self.commit_edit(0, 0, false, false) { return; } @@ -929,7 +955,19 @@ impl SheetApp { } self.toast = Some((msg, now_ms())); } - let r = if fill_selection { + // Committing nothing into an empty cell changes nothing: no undo step (#1855, Backspace has + // already cleared the cell as its own step). + let unchanged = !fill_selection + && !array + && text.is_empty() + && self + .session + .active() + .and_then(|d| d.wb.sheet(ed.sheet)) + .is_some_and(|sh| sh.cell(ed.cell).is_none_or(|c| c.formula.is_none() && c.value == gridcraft_engine::core::Value::Empty)); + let r = if unchanged { + Ok(json!({})) + } else if fill_selection { self.session.run("range.fill", json!({"input": text})) } else { self.session.run("cell.set", json!({"cell": ed.cell.a1(), "input": text, "array": array})) @@ -1103,10 +1141,16 @@ impl SheetApp { // The language the widgets translate with this frame (see `i18n::current`). i18n::set_current(&ctx, self.ui.language); let t = theme::Tokens::get(&ctx); + // A host can turn the File tab off while Backstage is already open: close it. + if !self.chrome.file_tab { + self.backstage = None; + } if self.backstage.is_none() { self.ribbon_keys(&ctx); } - ribbon::title_bar(self, ui); + if self.chrome.title_bar { + ribbon::title_bar(self, ui); + } if self.backstage.is_some() { // File tab: the Backstage view replaces everything below the title bar. backstage::show(self, ui); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 9c6d89eeb..a0ab5a339 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -176,7 +176,9 @@ pub fn show(app: &mut SheetApp, ui: &mut Ui) { ui.horizontal(|ui| { ui.spacing_mut().item_spacing.x = 2.0; ui.add_space(8.0); - file_menu(app, ui); + if app.chrome.file_tab { + file_menu(app, ui); + } let mut tabs: Vec<&str> = TABS.to_vec(); let ctx_tabs = contextual_tabs(app); tabs.extend(ctx_tabs.iter()); @@ -2452,14 +2454,25 @@ pub fn shortcut(app: &mut SheetApp, key: Key, m: Modifiers) { Key::Num9 => Some(("home.hideRows", json!({}))), Key::Num0 if shift => Some(("home.unhideColumns", json!({}))), Key::Num0 => Some(("home.hideColumns", json!({}))), - Key::Minus => { - app.open_dialog("deleteCells", json!({})); - None - } - Key::Plus | Key::Equals if shift => { - app.open_dialog("insertCells", json!({})); - None - } + // As in Excel: with whole rows or whole columns selected, Ctrl+Shift+= / Ctrl+- insert or + // delete them at once; any other selection asks which way to shift. + Key::Minus => match whole_lines(app) { + Some(true) => Some(("home.deleteRows", json!({}))), + Some(false) => Some(("home.deleteColumns", json!({}))), + None => { + app.open_dialog("deleteCells", json!({})); + None + } + }, + // Insert works on one area at a time, so several areas still go through the dialog. + Key::Plus | Key::Equals if shift => match whole_lines(app).filter(|_| single_area(app)) { + Some(true) => Some(("home.insertRows", json!({}))), + Some(false) => Some(("home.insertColumns", json!({}))), + None => { + app.open_dialog("insertCells", json!({})); + None + } + }, Key::Semicolon => { // Insert today's date. let text = app @@ -2514,3 +2527,23 @@ mod checkbox_on_tests { assert!(!checkbox_on(false, true)); } } + +/// Whether the selection is a single area. +fn single_area(app: &SheetApp) -> bool { + app.session.active().is_some_and(|d| d.selection.ranges.len() == 1) +} + +/// `Some(true)` when every selected area is whole rows, `Some(false)` when every one is whole +/// columns, `None` otherwise (including the whole sheet). +fn whole_lines(app: &SheetApp) -> Option { + let sel = &app.session.active()?.selection; + let rows = sel.ranges.iter().all(|r| r.is_full_rows() && !r.is_full_cols()); + let cols = sel.ranges.iter().all(|r| r.is_full_cols() && !r.is_full_rows()); + if rows { + Some(true) + } else if cols { + Some(false) + } else { + None + } +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 485b9a2f3..08097ba27 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -252,7 +252,8 @@ fn candidates(role: &str) -> Vec<(String, u32)> { let mac_sys = "/System/Library/Fonts"; let mac_sup = "/System/Library/Fonts/Supplemental"; let win = std::env::var("WINDIR").map(|w| format!("{w}\\Fonts")).unwrap_or_else(|_| "C:\\Windows\\Fonts".into()); - let lin = ["/usr/share/fonts/truetype", "/usr/share/fonts", "/usr/local/share/fonts", "/usr/share/fonts/TTF"]; + // ChromeOS's Linux container (Crostini) mounts the host's fonts under `chromeos/` (#1762). + let lin = ["/usr/share/fonts/truetype", "/usr/share/fonts", "/usr/local/share/fonts", "/usr/share/fonts/TTF", "/usr/share/fonts/chromeos"]; let mut v: Vec<(String, u32)> = Vec::new(); let mut add = |s: String, i: u32| v.push((s, i)); match role { @@ -498,10 +499,10 @@ fn fallback_faces(han: HanOrder) -> Vec<(&'static str, Vec<(String, u32)>)> { } /// Linux directories that hold CJK fonts across distributions: Debian/Ubuntu (`opentype/noto`, -/// `truetype/wqy`), Fedora (`google-noto-cjk`), Arch (`noto-cjk`, `wenquanyi`), plus local and -/// flat layouts. +/// `truetype/wqy`), Fedora (`google-noto-cjk`), Arch (`noto-cjk`, `wenquanyi`), ChromeOS's Linux +/// container (`chromeos/notocjk`, #1762), plus local and flat layouts. #[cfg(not(target_arch = "wasm32"))] -const LINUX_CJK_DIRS: [&str; 10] = [ +const LINUX_CJK_DIRS: [&str; 11] = [ "/usr/share/fonts/opentype/noto", "/usr/share/fonts/google-noto-cjk", "/usr/share/fonts/noto-cjk", @@ -512,6 +513,7 @@ const LINUX_CJK_DIRS: [&str; 10] = [ "/usr/share/fonts", "/usr/local/share/fonts", "/usr/local/share/fonts/noto-cjk", + "/usr/share/fonts/chromeos/notocjk", ]; /// Japanese Han + Kana faces (Japanese kanji glyph shapes). @@ -581,7 +583,13 @@ fn thai() -> Vec<(String, u32)> { let mac_sys = "/System/Library/Fonts"; let mac_sup = "/System/Library/Fonts/Supplemental"; let win = std::env::var("WINDIR").map(|w| format!("{w}\\Fonts")).unwrap_or_else(|_| "C:\\Windows\\Fonts".into()); - let lin = ["/usr/share/fonts/truetype/noto", "/usr/share/fonts/opentype/noto", "/usr/share/fonts/truetype", "/usr/share/fonts"]; + let lin = [ + "/usr/share/fonts/truetype/noto", + "/usr/share/fonts/opentype/noto", + "/usr/share/fonts/truetype", + "/usr/share/fonts", + "/usr/share/fonts/chromeos/noto", // ChromeOS's Linux container (#1762) + ]; let mut v: Vec<(String, u32)> = Vec::new(); v.push((format!("{win}\\LeelawUI.ttf"), 0)); // Leelawadee UI v.push((format!("{win}\\tahoma.ttf"), 0)); // Tahoma @@ -603,7 +611,13 @@ fn hebrew() -> Vec<(String, u32)> { let mac_sys = "/System/Library/Fonts"; let mac_sup = "/System/Library/Fonts/Supplemental"; let win = std::env::var("WINDIR").map(|w| format!("{w}\\Fonts")).unwrap_or_else(|_| "C:\\Windows\\Fonts".into()); - let lin = ["/usr/share/fonts/truetype/noto", "/usr/share/fonts/opentype/noto", "/usr/share/fonts/truetype", "/usr/share/fonts"]; + let lin = [ + "/usr/share/fonts/truetype/noto", + "/usr/share/fonts/opentype/noto", + "/usr/share/fonts/truetype", + "/usr/share/fonts", + "/usr/share/fonts/chromeos/noto", // ChromeOS's Linux container (#1762) + ]; let mut v: Vec<(String, u32)> = Vec::new(); v.push((format!("{win}\\david.ttf"), 0)); // David v.push((format!("{win}\\frank.ttf"), 0)); // FrankRuehl @@ -657,6 +671,7 @@ fn symbols_more() -> Vec<(String, u32)> { "/usr/share/fonts/google-noto-sans-symbols-2-fonts", "/usr/share/fonts/google-noto", "/usr/local/share/fonts/noto", + "/usr/share/fonts/chromeos/noto", ] { v.push((format!("{d}/NotoSansSymbols2-Regular.ttf"), 0)); } @@ -888,6 +903,22 @@ mod tests { } } + #[test] + fn chromeos_font_layout_is_probed() { + // #1762: Crostini mounts the host's fonts under /usr/share/fonts/chromeos, one folder per + // family set; a script whose list misses it renders as tofu with the font installed. + let probes = |cands: Vec<(String, u32)>, path: &str| cands.iter().any(|(p, _)| p == path); + let cros = "/usr/share/fonts/chromeos"; + assert!(probes(thai(), &format!("{cros}/noto/NotoSansThai-Regular.ttf"))); + assert!(probes(hebrew(), &format!("{cros}/noto/NotoSansHebrew-Regular.ttf"))); + assert!(probes(cjk_han_japanese(), &format!("{cros}/notocjk/NotoSansCJK-Regular.ttc"))); + assert!(probes(cjk_han_simplified(), &format!("{cros}/notocjk/NotoSansCJK-Regular.ttc"))); + assert!(probes(cjk_hangul(), &format!("{cros}/notocjk/NotoSansCJK-Regular.ttc"))); + assert!(probes(symbols_more(), &format!("{cros}/noto/NotoSansSymbols2-Regular.ttf"))); + assert!(probes(candidates(UI), &format!("{cros}/noto/NotoSans-Regular.ttf"))); + assert!(probes(candidates(CELL), &format!("{cros}/crosextra/Carlito-Regular.ttf"))); + } + #[test] fn unparseable_font_files_are_rejected_not_fatal() { // #226: epaint panics in `set_fonts` on a face it can't parse ("An offset was out of diff --git a/crates/ui-egui/tests/file_menu.rs b/crates/ui-egui/tests/file_menu.rs index 691eef367..6cce9cdba 100644 --- a/crates/ui-egui/tests/file_menu.rs +++ b/crates/ui-egui/tests/file_menu.rs @@ -132,6 +132,33 @@ fn file_options_opens_the_options_page_and_pages_are_reachable_by_command() { assert!(h.state().backstage.is_none()); } +#[test] +fn translated_options_headings_show_no_context_prefix() { + for (code, headings, prefixed, (view, view_prefixed)) in [ + ("fr", ["Général", "Enregistrer"], ["Options|Général", "Options|Enregistrer"], ("Affichage", "Options de la feuille|Affichage")), + ("sr", ["Opšte", "Sačuvaj"], ["Opcije|Opšte", "Opcije|Sačuvaj"], ("Prikaz", "Opcije lista|Prikaz")), + ] { + let mut h = harness(Services::default()); + h.state_mut().run("app.language.set", serde_json::json!({"language": code})).unwrap(); + h.state_mut().run("file.options", serde_json::json!({})).unwrap(); + h.run_steps(3); + assert_eq!(page(&h), Some("options")); + for heading in headings { + assert!(h.query_all_by_label(heading).next().is_some(), "{code}: no {heading} heading"); + } + for label in prefixed { + assert!(h.query_by_label(label).is_none(), "{code}: {label} shows its context"); + } + // Page Layout › Sheet Options: the Gridlines and Headings "View" checkboxes had the same + // prefix. The View tab may share the word, so count at least the two checkboxes. + h.state_mut().run("ui.backstage", serde_json::json!({"page": "close"})).unwrap(); + h.state_mut().ui.ribbon_tab = "Page Layout".into(); + h.run_steps(3); + assert!(h.query_by_label(view_prefixed).is_none(), "{code}: {view_prefixed} shows its context"); + assert!(h.query_all_by_label(view).count() >= 2, "{code}: the Sheet Options checkboxes don't read {view}"); + } +} + #[test] fn export_as_pdf_is_delivered_as_a_download() { let files = Arc::new(Mutex::new(Vec::<(String, Vec)>::new())); diff --git a/crates/ui-egui/tests/host_chrome.rs b/crates/ui-egui/tests/host_chrome.rs new file mode 100644 index 000000000..627e24025 --- /dev/null +++ b/crates/ui-egui/tests/host_chrome.rs @@ -0,0 +1,65 @@ +//! A host that embeds the sheet in its own window can turn off the app's title bar and File tab. + +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_ui_egui::{Chrome, Services, SheetApp, backstage::Page}; + +fn harness(chrome: Chrome) -> egui_kittest::Harness<'static, SheetApp> { + let mut session = Session::new(); + session.new_workbook(); + let mut app = SheetApp::new(session, Services::default()); + app.chrome = chrome; + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ); + h.run_steps(4); + h +} + +#[test] +fn the_title_bar_and_file_tab_are_drawn_by_default() { + let h = harness(Chrome::default()); + assert!(h.query_by_label("AutoSave").is_some(), "the title bar is drawn"); + assert!(h.query_by_label("File").is_some(), "the File tab is drawn"); +} + +#[test] +fn a_host_can_leave_out_the_title_bar_and_the_file_tab() { + let full = harness(Chrome::default()); + let h = harness(Chrome { title_bar: false, file_tab: false }); + assert!(h.query_by_label("AutoSave").is_none(), "no title bar"); + assert!(h.query_by_label("File").is_none(), "no File tab"); + // The ribbon tabs are painted text, so measure the move with the first ribbon control. + let (with_bar, without) = (full.get_by_label("Paste").rect(), h.get_by_label("Paste").rect()); + let moved = with_bar.min.y - without.min.y; + assert!((25.0..=40.0).contains(&moved), "the ribbon moves up by the title bar's height (28 on macOS, 38 elsewhere), moved {moved}"); + assert_eq!(with_bar.min.x, without.min.x, "the ribbon's groups keep their place across"); +} + +#[test] +fn without_the_file_tab_the_backstage_view_never_opens() { + let mut h = harness(Chrome { title_bar: true, file_tab: false }); + h.state_mut().open_backstage(Page::Home); + h.run_steps(3); + assert!(h.state().backstage.is_none(), "opening Backstage directly is ignored"); + h.state_mut().open_backstage(Page::Print); + h.run_steps(3); + assert!(h.state().backstage.is_none(), "so is every page, Print and Save As included"); + assert!(h.query_by_label("AutoSave").is_some(), "the title bar can stay on its own"); +} + +#[test] +fn turning_the_file_tab_off_closes_an_open_backstage() { + let mut h = harness(Chrome::default()); + h.state_mut().open_backstage(Page::Home); + h.run_steps(3); + assert!(h.state().backstage.is_some(), "Backstage opens with the File tab on"); + h.state_mut().chrome.file_tab = false; + h.run_steps(3); + assert!(h.state().backstage.is_none(), "and closes when the host turns the File tab off"); +} diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index b40769cf8..02084ffb8 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -892,3 +892,22 @@ fn rotated_wrapped_text_is_drawn_rotated_inside_its_cell() { assert!(clip.width() < 70.0, "clip spans one 6-character column, got {clip:?}"); } } + +#[test] +fn undo_after_backspace_restores_the_cleared_cell() { + // #1855: Backspace clears the cell and opens an empty editor; committing it must not add a + // second, empty undo step, so one Ctrl+Z brings the value back and leaves other edits alone. + let mut h = harness(blank()); + text(&mut h, "3200"); + key(&mut h, Key::Enter, Modifiers::NONE); + click_cell(&mut h, "C5"); + text(&mut h, "11750"); + key(&mut h, Key::Enter, Modifiers::NONE); + click_cell(&mut h, "A1"); + key(&mut h, Key::Backspace, Modifiers::NONE); + key(&mut h, Key::Enter, Modifiers::NONE); + assert_eq!(value(&h, "A1"), Value::Empty); + key(&mut h, Key::Z, Modifiers::COMMAND); + assert_eq!(value(&h, "A1"), Value::Number(3200.0)); + assert_eq!(value(&h, "C5"), Value::Number(11750.0)); +} diff --git a/crates/ui-egui/tests/keytips.rs b/crates/ui-egui/tests/keytips.rs index 24be742a5..1cf286eac 100644 --- a/crates/ui-egui/tests/keytips.rs +++ b/crates/ui-egui/tests/keytips.rs @@ -267,6 +267,80 @@ fn mouse_press_and_window_focus_loss_cancel_keytips() { assert!(h.state().editor.is_none()); } +fn alt_down(h: &mut Harness, alt: Key) { + events(h, [Event::ModifiersChanged(Modifiers::ALT), key_event(alt, true, Modifiers::ALT)]); +} + +fn alt_up(h: &mut Harness, alt: Key) { + events(h, [Event::ModifiersChanged(Modifiers::NONE), key_event(alt, false, Modifiers::NONE)]); +} + +fn alt_letter(h: &mut Harness, key: Key) { + events(h, [key_event(key, true, Modifiers::ALT), key_event(key, false, Modifiers::ALT)]); +} + +fn line_size(h: &Harness, cols: bool) -> Option { + let sh = h.state().session.active().unwrap().wb.active().unwrap(); + if cols { &sh.cols } else { &sh.rows }.get(&0).and_then(|e| e.size) +} + +#[test] +fn letters_typed_with_alt_held_run_autofit_sequences() { + // #1846: Excel accepts Alt+H, O, A with Alt still held for the first letter (or all of them). + for os in [OperatingSystem::Windows, OperatingSystem::Nix] { + let mut s = blank(); + s.execute("cell.set", json!({"cell": "A1", "input": "a much longer piece of text than the column"})).unwrap(); + s.execute("home.fontSize", json!({"range": "A1", "size": 28})).unwrap(); + s.execute("home.rowHeight", json!({"rows": "1:1", "height": 10})).unwrap(); + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + let mut h = harness_on(s, os); + + // Alt held through H, O and I: AutoFit Column Width. + alt_down(&mut h, Key::AltLeft); + alt_letter(&mut h, Key::H); + assert_eq!(h.state().keytips.prefix(), Some("H"), "{os:?}: Alt+H opens the Home keytips"); + alt_letter(&mut h, Key::O); + alt_letter(&mut h, Key::I); + alt_up(&mut h, Key::AltLeft); + assert_eq!(h.state().keytips.prefix(), None, "{os:?}"); + assert!(line_size(&h, true).is_some_and(|w| w > 100.0), "{os:?}: column A was autofitted"); + + // Alt released after H: AutoFit Row Height. + alt_down(&mut h, Key::AltLeft); + alt_letter(&mut h, Key::H); + alt_up(&mut h, Key::AltLeft); + assert_eq!(h.state().keytips.prefix(), Some("H"), "{os:?}: releasing Alt keeps the level"); + letter(&mut h, Key::O, false); + letter(&mut h, Key::A, false); + assert!(line_size(&h, false).is_some_and(|r| r > 10.0), "{os:?}: row 1 was autofitted"); + assert_eq!(value(&h, "A1"), Value::Text("a much longer piece of text than the column".into())); + assert!(h.state().editor.is_none(), "{os:?}: no letter reached the grid"); + } +} + +#[test] +fn alt_combinations_outside_a_sequence_still_cancel_keytips() { + let mut h = harness_on(blank(), OperatingSystem::Windows); + alt_down(&mut h, Key::AltLeft); + alt_letter(&mut h, Key::X); + alt_up(&mut h, Key::AltLeft); + assert_eq!(h.state().keytips.prefix(), None, "X starts no sequence"); + + // AltGr types characters (and is Ctrl+Alt on Windows): it never starts keytips. + let mut h = harness_on(blank(), OperatingSystem::Nix); + alt_down(&mut h, Key::AltRight); + alt_letter(&mut h, Key::H); + alt_up(&mut h, Key::AltRight); + assert_eq!(h.state().keytips.prefix(), None); + + // macOS: Option is a text modifier (see #1864). + let mut h = harness_on(blank(), OperatingSystem::Mac); + alt_down(&mut h, Key::AltLeft); + alt_letter(&mut h, Key::H); + alt_up(&mut h, Key::AltLeft); + assert_eq!(h.state().keytips.prefix(), None); +} + #[test] fn held_keys_do_not_toggle_twice_or_leak_repeated_letters() { let mut h = harness(blank()); diff --git a/crates/ui-egui/tests/touch_pan.rs b/crates/ui-egui/tests/touch_pan.rs new file mode 100644 index 000000000..f55e84246 --- /dev/null +++ b/crates/ui-egui/tests/touch_pan.rs @@ -0,0 +1,91 @@ +//! Touch input on the grid: one finger pans, a tap selects (issue #1907). + +use egui::{Event, Modifiers, PointerButton, TouchDeviceId, TouchId, TouchPhase}; +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_ui_egui::SheetApp; + +fn harness(session: Session) -> egui_kittest::Harness<'static, SheetApp> { + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn blank() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s +} + +/// One finger, sent the way egui-winit does: a touch event plus the emulated pointer events. +fn touch(h: &mut egui_kittest::Harness<'static, SheetApp>, phase: TouchPhase, pos: egui::Pos2) { + let ev = &mut h.input_mut().events; + ev.push(Event::Touch { device_id: TouchDeviceId(1), id: TouchId(0), phase, pos, force: None }); + match phase { + TouchPhase::Start => { + ev.push(Event::PointerMoved(pos)); + ev.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + } + TouchPhase::Move => ev.push(Event::PointerMoved(pos)), + TouchPhase::End | TouchPhase::Cancel => { + ev.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + ev.push(Event::PointerGone); + } + } +} + +fn swipe(h: &mut egui_kittest::Harness<'static, SheetApp>, from: egui::Pos2, by: egui::Vec2) { + touch(h, TouchPhase::Start, from); + h.run_steps(2); + for i in 1..=10 { + touch(h, TouchPhase::Move, from + by * (i as f32 / 10.0)); + h.run_steps(1); + } + touch(h, TouchPhase::End, from + by); + h.run_steps(3); +} + +fn assert_selection_untouched(h: &egui_kittest::Harness<'static, SheetApp>) { + let sel = &h.state().session.active().unwrap().selection; + assert_eq!(sel.active, CellRef::new(0, 0), "a touch pan must not move the active cell: {sel:?}"); + assert_eq!(sel.ranges.len(), 1, "{sel:?}"); + assert_eq!(sel.ranges[0].start, CellRef::new(0, 0), "{sel:?}"); + assert_eq!(sel.ranges[0].end, CellRef::new(0, 0), "a touch pan must not select cells: {sel:?}"); +} + +#[test] +fn one_finger_drag_pans_the_grid_instead_of_selecting() { + let mut h = harness(blank()); + let cells = h.state().grid.cells_rect.unwrap(); + swipe(&mut h, cells.center() + egui::vec2(0.0, 100.0), egui::vec2(0.0, -200.0)); + let scroll = h.state().view().scroll; + assert!(scroll.y > 0.0, "dragging a finger up scrolls down: scroll = {scroll:?}"); + assert_selection_untouched(&h); + + swipe(&mut h, cells.center() + egui::vec2(100.0, 0.0), egui::vec2(-200.0, 0.0)); + let scroll = h.state().view().scroll; + assert!(scroll.x > 0.0, "dragging a finger left scrolls right: scroll = {scroll:?}"); + assert_selection_untouched(&h); +} + +#[test] +fn one_finger_tap_still_selects_the_cell() { + let mut h = harness(blank()); + let cells = h.state().grid.cells_rect.unwrap(); + let p = cells.min + egui::vec2(5.0, 5.0) + egui::vec2(0.0, cells.height() / 2.0); + touch(&mut h, TouchPhase::Start, p); + h.run_steps(1); + touch(&mut h, TouchPhase::End, p); + h.run_steps(3); + let sel = &h.state().session.active().unwrap().selection; + assert_eq!(sel.active.col, 0, "{sel:?}"); + assert!(sel.active.row > 0, "a tap selects the tapped cell: {sel:?}"); + assert_eq!(h.state().view().scroll, egui::Vec2::ZERO); +} diff --git a/crates/ui-egui/tests/whole_line_shortcuts.rs b/crates/ui-egui/tests/whole_line_shortcuts.rs new file mode 100644 index 000000000..2bbcf3adf --- /dev/null +++ b/crates/ui-egui/tests/whole_line_shortcuts.rs @@ -0,0 +1,65 @@ +//! Ctrl+Shift+= and Ctrl+- on whole rows or columns insert or delete them at once, as in Excel; +//! any other selection opens the Insert or Delete dialog. + +use egui::{Key, Modifiers}; +use gridcraft_engine::Session; +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness(selection: &str) -> Harness { + let mut session = Session::new(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "A1", "values": [["a1", "b1"], ["a2", "b2"], ["a3", "b3"]]})).unwrap(); + session.execute("selection.set", json!({"range": selection})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn value(h: &Harness, a1: &str) -> Value { + h.state().session.active().unwrap().wb.active().unwrap().value(CellRef::parse(a1).unwrap()) +} + +fn press(h: &mut Harness, key: Key, shift: bool) { + let modifiers = Modifiers { shift, ..Modifiers::COMMAND }; + h.key_press_modifiers(modifiers, key); + h.run_steps(3); +} + +#[test] +fn whole_rows_are_inserted_and_deleted_at_once() { + let mut h = harness("2:2"); + press(&mut h, Key::Equals, true); + assert!(h.state().dialog.is_none(), "no dialog for whole rows"); + assert!(value(&h, "A2").is_empty(), "a row was inserted above row 2"); + assert_eq!(value(&h, "A3"), Value::from("a2")); + press(&mut h, Key::Minus, false); + assert_eq!(value(&h, "A2"), Value::from("a2"), "and deleted again"); +} + +#[test] +fn whole_columns_are_inserted_and_deleted_at_once() { + let mut h = harness("A:A"); + press(&mut h, Key::Equals, true); + assert!(h.state().dialog.is_none(), "no dialog for whole columns"); + assert_eq!(value(&h, "B1"), Value::from("a1")); + press(&mut h, Key::Minus, false); + assert_eq!(value(&h, "A1"), Value::from("a1")); +} + +#[test] +fn other_selections_still_ask_which_way_to_shift() { + let mut h = harness("A2:B2"); + press(&mut h, Key::Minus, false); + assert_eq!(h.state().dialog.as_ref().map(|d| d.name().to_string()).as_deref(), Some("deleteCells")); +} diff --git a/crates/xlsx/src/drawing.rs b/crates/xlsx/src/drawing.rs index 6dbd17e7e..a6892f0b8 100644 --- a/crates/xlsx/src/drawing.rs +++ b/crates/xlsx/src/drawing.rs @@ -228,8 +228,8 @@ fn read_shape(obj: &El, anchor: Anchor) -> Option { } /// A shape's text formatting: vertical anchor (`a:bodyPr anchor`), the first paragraph's -/// alignment (`a:pPr algn`) and the first run's properties (`a:rPr`: size, bold, italic, fill, -/// outline, `a:latin` typeface). +/// alignment and direction (`a:pPr algn`, `rtl`) and the first run's properties (`a:rPr`: size, +/// bold, italic, fill, outline, `a:latin` typeface). fn read_text_format(body: Option<&El>) -> ShapeFormat { let mut f = ShapeFormat::default(); let Some(body) = body else { return f }; @@ -238,12 +238,13 @@ fn read_text_format(body: Option<&El>) -> ShapeFormat { "b" => ShapeTextAnchor::Bottom, _ => ShapeTextAnchor::Top, }); - let first_p = body.kids("p").next(); - f.align = first_p.and_then(|p| p.child("pPr")).and_then(|p| p.attr("algn")).map(|a| match a { + let ppr = body.kids("p").next().and_then(|p| p.child("pPr")); + f.align = ppr.and_then(|p| p.attr("algn")).map(|a| match a { "ctr" => ShapeTextAlign::Center, "r" => ShapeTextAlign::Right, _ => ShapeTextAlign::Left, }); + f.rtl = ppr.is_some_and(|p| p.flag("rtl", false)); let rpr = body.kids("p").flat_map(|p| p.kids("r")).find_map(|r| r.child("rPr")); if let Some(r) = rpr { // `sz` is in hundredths of a point (100..=400000 in the schema). @@ -415,12 +416,14 @@ pub fn write_drawing(sheet: &Sheet, theme: &Theme, objs: &[Obj<'_>]) -> String { None => "", }; let _ = write!(s, ""); - let ppr = match f.align { + let algn = match f.align { None => "", - Some(ShapeTextAlign::Left) => "", - Some(ShapeTextAlign::Center) => "", - Some(ShapeTextAlign::Right) => "", + Some(ShapeTextAlign::Left) => " algn=\"l\"", + Some(ShapeTextAlign::Center) => " algn=\"ctr\"", + Some(ShapeTextAlign::Right) => " algn=\"r\"", }; + let rtl = if f.rtl { " rtl=\"1\"" } else { "" }; + let ppr = if algn.is_empty() && rtl.is_empty() { String::new() } else { format!("") }; let (rpr_attrs, rpr_body) = run_props(f, theme); for line in sh.text.split('\n') { if line.is_empty() { diff --git a/crates/xlsx/src/pivot.rs b/crates/xlsx/src/pivot.rs index 609ae530e..743a746e7 100644 --- a/crates/xlsx/src/pivot.rs +++ b/crates/xlsx/src/pivot.rs @@ -208,8 +208,13 @@ impl Stats { Value::Array(_) => self.add(&v.scalar()), } } + /// Excel counts booleans and errors as strings in the cache field flags. + fn strings(&self) -> bool { + self.text || self.boolean || self.error + } + /// Kinds for `containsMixedTypes`: strings (text, booleans, errors) and numbers. fn kinds(&self) -> usize { - [self.text, self.number, self.boolean, self.error].iter().filter(|b| **b).count() + usize::from(self.strings()) + usize::from(self.number) } } @@ -709,11 +714,10 @@ fn cache_xml(c: &Cache, records_rid: Option<&str>, sys: DateSystem) -> String { } let _ = g; } else { - let semi = st.text || st.blank || st.boolean || st.error; - if !semi { + if !st.strings() && !st.blank { s.push_str(" containsSemiMixedTypes=\"0\""); } - if !st.text { + if !st.strings() { s.push_str(" containsString=\"0\""); } if st.blank { @@ -816,10 +820,12 @@ fn table_xml(pt: &PivotTable, idx: usize, cache_id: u32, c: &Cache, fmt_id: &dyn if !pt.show_headers { s.push_str(" showHeaders=\"0\""); } - if !pt.grand_totals_rows { + // `rowGrandTotals` is the grand total row below the report (Excel's "On for Columns"), + // `colGrandTotals` the grand total column at its right ("On for Rows"). + if !pt.grand_totals_cols { s.push_str(" rowGrandTotals=\"0\""); } - if !pt.grand_totals_cols { + if !pt.grand_totals_rows { s.push_str(" colGrandTotals=\"0\""); } s.push_str(" multipleFieldFilters=\"0\">"); @@ -1262,8 +1268,8 @@ fn read_table(cx: &mut Ctx<'_>, x: &El, cache: &CacheIn, id: u32) -> Option(); assert!(kept <= one + VML.len() / 2, "kept {kept} bytes"); assert!(wb.sheet(0).unwrap().preserved.as_ref().is_some_and(|p| p.vml.is_some())); - assert!(!wb.sheet(1).unwrap().preserved.as_ref().is_some_and(|p| p.vml.is_some())); + assert!(wb.sheet(1).unwrap().preserved.as_ref().is_none_or(|p| p.vml.is_none())); assert!(rep.warnings.iter().any(|w| w.contains("\"Other\"") && w.contains("too large")), "{:?}", rep.warnings); // Both sheets keep their drawings when there is room. let (wb, _) = read_xlsx(&rich_package_with(CONTENT_TYPES, true)).unwrap(); diff --git a/crates/xlsx/src/tests/pivot_tests.rs b/crates/xlsx/src/tests/pivot_tests.rs index 99a15d6e7..9ec1f687d 100644 --- a/crates/xlsx/src/tests/pivot_tests.rs +++ b/crates/xlsx/src/tests/pivot_tests.rs @@ -355,6 +355,10 @@ fn pivot_xml_details() { let t2 = String::from_utf8(parts.get("xl/pivotTables/pivotTable2.xml").unwrap().clone()).unwrap(); assert!(t2.contains("pivotShowAs=\"rankDescending\"")); assert!(t2.contains("showDataAs=\"runTotal\"")); + // Pivot 1 has no Grand Total column (`colGrandTotals`), pivot 2 no Grand Total row + // (`rowGrandTotals`). + assert!(t1.contains(" colGrandTotals=\"0\"") && !t1.contains("rowGrandTotals"), "{t1}"); + assert!(t2.contains(" rowGrandTotals=\"0\"") && !t2.contains("colGrandTotals"), "{t2}"); // The custom value format is registered in the styles part. let styles = String::from_utf8(parts.get("xl/styles.xml").unwrap().clone()).unwrap(); assert!(styles.contains("formatCode=\"0.0" u"\"")); @@ -402,6 +406,67 @@ fn large_cache_is_written_without_records() { assert_eq!(back.sheet(0).unwrap().pivots.len(), 1); } +/// #1447 item 1: Excel counts booleans and errors as strings in ``; writing +/// `containsString="0"` for them makes a file Excel can't open. +#[test] +fn boolean_and_error_cache_flags_match_excel() { + let mut wb = Workbook::new(); + let s = wb.sheet_mut(0).unwrap(); + let na = || Value::Error(gridcraft_core::CellError::NA); + let cols: [(&str, [Value; 3]); 7] = [ + ("K", [Value::text("a"), Value::text("b"), Value::text("a")]), + ("BoolOnly", [Value::Bool(true), Value::Bool(false), Value::Bool(true)]), + ("NumBool", [Value::number(1.0), Value::Bool(true), Value::number(2.5)]), + ("ErrOnly", [na(), na(), na()]), + ("NumErr", [Value::number(1.0), na(), Value::number(3.0)]), + ("TextBool", [Value::text("x"), Value::Bool(true), Value::text("y")]), + ("BoolBlank", [Value::Bool(true), Value::Empty, Value::Bool(false)]), + ]; + for (c, (h, vals)) in cols.iter().enumerate() { + s.cells.set(CellRef::new(0, c as u32), Cell::value(Value::text(*h))); + for (r, v) in vals.iter().enumerate() { + if !matches!(v, Value::Empty) { + s.cells.set(CellRef::new(r as u32 + 1, c as u32), Cell::value(v.clone())); + } + } + } + s.pivots.push(PivotTable { + id: 1, + source: "Sheet1!$A$1:$G$4".into(), + anchor: at("J1"), + rows: vec![field("K")], + values: vec![PivotValue { source_col: "NumBool".into(), name: "Sum of NumBool".into(), ..PivotValue::default() }], + last_range: Some(rr("J1:K4")), + ..PivotTable::default() + }); + let bytes = write_xlsx(&wb).unwrap(); + let parts = unzip(&bytes); + let c = String::from_utf8(parts.get("xl/pivotCache/pivotCacheDefinition1.xml").unwrap().clone()).unwrap(); + let start = |name: &str| -> String { + let i = c.find(&format!("').unwrap(); + c[j..=k].trim_end_matches('>').trim_end_matches('/').to_string() + }; + let flags = |name: &str| -> String { + let t = start(name); + // Drop the trailing count attribute, which depends on itemization. + match t.find(" count=") { + Some(i) => t[..i].to_string(), + None => t, + } + }; + assert_eq!(flags("BoolOnly"), ""#; const PT_CACHE_RELS: &str = r#""#; @@ -579,6 +644,25 @@ fn values_ahead_of_column_fields_read_last() { assert_eq!(pt.values_position, None); } +// As Excel saves them: `rowGrandTotals="0"` drops the Grand Total row under the report (leaving +// Excel's "On for Rows Only"), `colGrandTotals="0"` the Grand Total column at its right ("On for +// Columns Only"). LibreOffice's Pivot2_Row_Subtotals.xlsx (`rowGrandTotals="0"`) shows no Grand +// Total row; ClosedXML's TemplateWithTableSourcePivotTables.xlsx pivotTable5 (`colGrandTotals="0"`, +// row and column fields) shows a Grand Total row and no Grand Total column. +#[test] +fn grand_totals_read_in_excel_direction() { + let table = |attr: &str| { + format!( + r#""# + ) + }; + for (attr, rows, cols) in [("rowGrandTotals=\"0\"", true, false), ("colGrandTotals=\"0\"", false, true), ("", true, true)] { + let (wb, _) = read_xlsx(&with_pivot(&table(attr), Some(GOOD_CACHE))).unwrap(); + let pt = &wb.sheet(0).unwrap().pivots[0]; + assert_eq!((pt.grand_totals_rows, pt.grand_totals_cols), (rows, cols), "{attr}"); + } +} + #[test] fn data_on_rows_with_one_value_field_reads_without_warning() { let table = r#""#; diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..7c7c19056 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -365,6 +365,7 @@ fn sample() -> Workbook { text_outline: Some((Color::Rgb(0xFF0000), 1.5)), align: Some(ShapeTextAlign::Right), anchor: Some(ShapeTextAnchor::Bottom), + rtl: true, }, }); let base_chart = Chart { diff --git a/crates/xlsx/src/tests/shape_text_rtl.rs b/crates/xlsx/src/tests/shape_text_rtl.rs new file mode 100644 index 000000000..36bdb861d --- /dev/null +++ b/crates/xlsx/src/tests/shape_text_rtl.rs @@ -0,0 +1,86 @@ +//! Text boxes and shapes keep their paragraphs' right-to-left direction (`a:pPr rtl`). + +use std::io::Read; +use std::sync::Arc; + +use gridcraft_model::{Anchor, Color, Shape, ShapeFormat, ShapeKind, ShapeTextAlign, Sheet, Workbook}; + +use super::minimal; +use crate::{read_xlsx, write_xlsx}; + +const ARABIC_TEXT_BOX: &str = r#" +10104040 + + + +مرحبا بالعالم +"#; + +fn read_drawing(drawing: &str) -> Workbook { + let rels = r#""#; + let bytes = minimal( + r#""#, + &[("xl/worksheets/_rels/sheet1.xml.rels", rels), ("xl/drawings/drawing1.xml", drawing)], + "", + "", + ); + read_xlsx(&bytes).unwrap().0 +} + +fn written_drawing(wb: &Workbook) -> String { + let bytes = write_xlsx(wb).unwrap(); + let mut archive = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut drawing = String::new(); + archive.by_name("xl/drawings/drawing1.xml").unwrap().read_to_string(&mut drawing).unwrap(); + drawing +} + +fn text_box(text: &str, format: ShapeFormat) -> Workbook { + let mut sheet = Sheet::new("Sheet1"); + sheet.shapes.push(Shape { + id: 1, + kind: ShapeKind::TextBox, + anchor: Anchor { width: 120.0, height: 40.0, ..Default::default() }, + fill: Color::Auto, + line: Color::Auto, + text: text.into(), + format, + }); + Workbook { sheets: vec![Arc::new(sheet)], ..Default::default() } +} + +#[test] +fn shape_text_keeps_right_to_left_paragraphs() { + let wb = read_drawing(ARABIC_TEXT_BOX); + let shape = &wb.sheet(0).unwrap().shapes[0]; + assert_eq!(shape.text, "مرحبا بالعالم"); + assert!(shape.format.rtl, "rtl=\"1\" on the first paragraph is read"); + assert_eq!(shape.format.align, Some(ShapeTextAlign::Right)); + + let xml = written_drawing(&wb); + assert!(xml.contains(r#""#), "saved paragraph keeps rtl: {xml}"); + + let (reopened, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + let after = &reopened.sheet(0).unwrap().shapes[0]; + assert!(after.format.rtl); + assert_eq!(after.format.align, Some(ShapeTextAlign::Right)); + assert_eq!(after.text, "مرحبا بالعالم"); + + let xml = written_drawing(&text_box("سطر\nآخر", ShapeFormat { rtl: true, ..Default::default() })); + assert_eq!(xml.matches(r#""#).count(), 2, "every paragraph is right-to-left without an alignment: {xml}"); +} + +#[test] +fn shape_text_without_rtl_writes_unchanged_ppr() { + let wb = read_drawing(&ARABIC_TEXT_BOX.replace(r#" rtl="1""#, "")); + assert!(!wb.sheet(0).unwrap().shapes[0].format.rtl); + let wb = read_drawing(&ARABIC_TEXT_BOX.replace(r#" rtl="1""#, r#" rtl="0""#)); + assert!(!wb.sheet(0).unwrap().shapes[0].format.rtl); + + let xml = written_drawing(&text_box("left\nto right", ShapeFormat { align: Some(ShapeTextAlign::Right), ..Default::default() })); + assert_eq!(xml.matches(r#""#).count(), 2, "{xml}"); + assert!(!xml.contains("rtl=\"1\""), "{xml}"); + + let xml = written_drawing(&text_box("plain", ShapeFormat::default())); + assert!(!xml.contains(">(), ["Sheet1"]); + assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0)); + assert!(report.warnings.iter().any(|w| w.contains("Dialog sheets")), "{:?}", report.warnings); +} + #[test] fn sheets_share_one_record_budget() { // Many BoundSheet records point into one long substream (at successive nested BOFs), so each @@ -594,3 +608,31 @@ fn a_format_shorter_than_its_count_keeps_its_characters() { assert_eq!(fmt("A3"), "0.000"); assert!(report.warnings.iter().any(|w| w.contains("declared length")), "{:?}", report.warnings); } + +fn one_sheet_with(sheet_records: &[u8]) -> Vec { + let globals = |pos: u32| cat(&[&bof(0x0005), &rec(0x0085, &cat(&[&pos.to_le_bytes(), &[0, 0, 6, 0], b"Sheet1"])), &eof()]); + let sheet = cat(&[&bof(0x0010), sheet_records, &rec(0x0203, &cat(&[&cell(0, 0, 0), &7.0f64.to_le_bytes()])), &eof()]); + compound("Workbook", &cat(&[&globals(globals(0).len() as u32), &sheet])) +} + +#[test] +fn xls_window2_right_to_left_sheet_opens_right_to_left() { + let window2 = |flags: u16| rec(0x023E, &u16s(&[flags, 0, 0, 64, 0, 0, 0, 0, 0])); + let view = |file: &[u8]| { + let (wb, _) = read_xls(file).unwrap(); + let sh = &wb.sheets[0]; + (sh.right_to_left, sh.show_gridlines, sh.show_headings, sh.show_zeros, sh.show_formulas) + }; + assert_eq!(view(&one_sheet_with(&window2(0x0040 | 0x0004))), (true, false, true, false, false)); + assert_eq!(view(&one_sheet_with(&window2(0x0001 | 0x0002 | 0x0010))), (false, true, false, true, true)); +} + +#[test] +fn xls_short_window2_is_ignored() { + for payload in [&[][..], &[0x40][..], &[0x40, 0][..], &[0x40, 0, 0, 0, 0, 0, 0, 0, 0][..]] { + let (wb, _) = read_xls(&one_sheet_with(&rec(0x023E, payload))).unwrap(); + let sh = &wb.sheets[0]; + assert_eq!((sh.right_to_left, sh.show_gridlines, sh.show_headings, sh.show_zeros), (false, true, true, true)); + assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0)); + } +} diff --git a/crates/xlsx/src/xls/mod.rs b/crates/xlsx/src/xls/mod.rs index dde8641e5..ceaf3e26d 100644 --- a/crates/xlsx/src/xls/mod.rs +++ b/crates/xlsx/src/xls/mod.rs @@ -40,6 +40,9 @@ const MAX_RECORDS: usize = 40_000_000; const BOF: u16 = 0x0809; const EOF: u16 = 0x000A; const BIFF8: u16 = 0x0600; +/// WsBool (MS-XLS 2.4.351): its first byte's `fDialog` bit marks a dialog sheet, whose BOF says worksheet. +const WSBOOL: u16 = 0x0081; +const WSBOOL_DIALOG: u8 = 0x10; pub(crate) fn invalid(msg: impl Into) -> IoError { IoError::Format(format!("XLS: {}", msg.into())) @@ -269,11 +272,9 @@ fn globals(stream: &[u8], budget: &mut Budget, report: &mut ReadReport) -> Resul budget.text(code.len())?; g.formats.insert(ifmt, code); } - 0x00E0 => { - if g.xf_formats.len() < MAX_XFS { - c.u16()?; - g.xf_formats.push(c.u16()?); - } + 0x00E0 if g.xf_formats.len() < MAX_XFS => { + c.u16()?; + g.xf_formats.push(c.u16()?); } 0x00FC => g.sst = shared_strings(&mut c, budget)?, 0x01AE => { @@ -524,6 +525,7 @@ fn worksheet( continue; } _ if depth > 1 => continue, + WSBOOL if rec.parts.first().and_then(|p| p.first()).is_some_and(|b| b & WSBOOL_DIALOG != 0) => return Ok(None), _ => {} } sheet_record(&rec, &mut st, budget, report)?; @@ -561,6 +563,9 @@ fn worksheet( Ok(Some((st.sheet, st.calc))) } +/// The shortest WINDOW2 a file holds (the chart-sheet form); a shorter one is truncated. +const WINDOW2_MIN_LEN: usize = 10; + fn sheet_record(rec: &Record<'_>, st: &mut SheetState<'_>, budget: &mut Budget, report: &mut ReadReport) -> Result<(), IoError> { let mut c = Cursor::new(&rec.parts); // A formula's string result is the String record right after it (shared/array records may sit between). @@ -739,6 +744,16 @@ fn sheet_record(rec: &Record<'_>, st: &mut SheetState<'_>, budget: &mut Budget, st.sheet.default_row_height = pt_to_px(f64::from(height) / 20.0); } } + 0x023E => { + let flags = if c.remaining() >= WINDOW2_MIN_LEN { c.u16().ok() } else { None }; + let Some(flags) = flags else { return Ok(()) }; + let on = |bit: u16| flags & bit != 0; + st.sheet.show_formulas = on(0x0001); + st.sheet.show_gridlines = on(0x0002); + st.sheet.show_headings = on(0x0004); + st.sheet.show_zeros = on(0x0010); + st.sheet.right_to_left = on(0x0040); + } 0x001C => report.warn("Cell comments in .xls files are not imported yet."), 0x00EC => report.warn("Pictures, charts and shapes in .xls files are not imported yet."), 0x01B0 | 0x0879 => report.warn("Conditional formatting in .xls files is not imported yet."), diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 7cc6ec230..e82ae7299 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -15,6 +15,10 @@ Then send newline-delimited JSON to `127.0.0.1:7979`. Every request line gets on Errors come back as `{"id": …, "ok": false, "error": "…"}`. +A line that is not a JSON object with a string `method` (an HTTP request, text that isn't JSON, +bytes that aren't UTF-8, a line over 64 MiB) gets one error reply and the server closes the +connection; nothing sent after it runs. Blank lines are skipped. + ## Methods | Method | Params | What it does | diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 158149fb3..5a7588986 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (.xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -29,7 +29,7 @@ and repair-free opening remain unverified. | `.xlsm` Macro-Enabled Workbook | R/W | yes | yes, macros kept, not run | The VBA project is kept byte-for-byte and written back only into `.xlsm`/`.xltm` (#6); macros never run. Saved with the macro-enabled workbook content type | `roundtrip.rs` (`vba_project_roundtrip`, `package_kind_sets_workbook_content_type`), engine `io.rs` (`macros_are_saved_only_into_xlsm`) | | `.xltx` / `.xltm` templates | R/W | yes | partial | Saved with the template (and macro-enabled template) content type, chosen by extension; `.xltm` keeps a VBA project, `.xltx` drops it; "new from template" behaviour missing | `roundtrip.rs` (`package_kind_sets_workbook_content_type`), engine `io.rs` | | `.xlsb` Binary Workbook | R/W | **values only** (#101) | no | Names, visibility, numbers, text, booleans, errors, merges, cached formula results; no formulas, formatting, charts, names | `xlsb_tests.rs` | -| `.xls` Excel 97-2003 (BIFF8) | R/W | **yes, partial** (#121) | no (Save As `.xlsx`) | Clean-room from [MS-CFB] and [MS-XLS]. Read: worksheets and visibility, shared strings, numbers, text, booleans, errors, formulas (text for the common tokens, shared and array formulas, 3-D refs, names, newer functions called by name) with cached results, number formats (dates display as dates), 1904 date system, merges, column widths, row heights, defined names, print area and titles, calculation settings (mode, iteration with its maximum iterations and change, precision as displayed). Not read: fonts, fills, borders, alignment, charts, pictures, comments, CF, validation, hyperlinks, freeze panes, external links (formula kept as its value). Chart and macro sheets skipped with a warning; encrypted (FILEPASS) and BIFF5 (Excel 5.0/95) files refused with a message. Opens as a new unsaved workbook. Unverified on a real-world corpus | `xls_tests.rs` | +| `.xls` Excel 97-2003 (BIFF8) | R/W | **yes, partial** (#121) | no (Save As `.xlsx`) | Clean-room from [MS-CFB] and [MS-XLS]. Read: worksheets and visibility, shared strings, numbers, text, booleans, errors, formulas (text for the common tokens, shared and array formulas, 3-D refs, names, newer functions called by name) with cached results, number formats (dates display as dates), 1904 date system, merges, column widths, row heights, defined names, print area and titles, calculation settings (mode, iteration with its maximum iterations and change, precision as displayed), sheet display options (right-to-left, gridlines, headings, zero values, formulas; a truncated WINDOW2 is ignored). Not read: fonts, fills, borders, alignment, charts, pictures, comments, CF, validation, hyperlinks, freeze panes, external links (formula kept as its value). Chart and macro sheets skipped with a warning; encrypted (FILEPASS) and BIFF5 (Excel 5.0/95) files refused with a message. Opens as a new unsaved workbook. Unverified on a real-world corpus | `xls_tests.rs` | | `.xlt` Excel 97-2003 template | R/W | yes (read as `.xls`) | no | | — | | `.xlam` / `.xla` add-ins | R/W | no | no | VBA; out of scope | — | | `.xml` XML Spreadsheet 2003 | R/W | no | no | Simple to support | — | @@ -85,6 +85,12 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | .xls import skips dialog sheets (WsBool `fDialog`), which share the worksheet BOF type and were read as empty worksheets | +| 2026-10-19 | minor | `.xls` import reads WINDOW2: right-to-left sheets (common in older Arabic and Hebrew files) open right-to-left, and hidden gridlines, headings and zero values and Show Formulas carry over | +| 2026-10-19 | trivial | XLSX text boxes and shapes keep right-to-left paragraph direction (`a:pPr rtl`, ECMA-376 Part 1 §21.1.2.2.7): read from the first paragraph and written on every paragraph; output without it is unchanged | +| 2026-10-18 | trivial | `file.open {csvAsText: true}` keeps every CSV/TSV/TXT field as the text the file holds, so identifiers (GTINs, SKUs, ZIP codes) keep their digits and leading zeros (#1808; a preference and a CLI flag are separate work) | +| 2026-10-18 | trivial | XLSX PivotTables: `rowGrandTotals` is the Grand Total row and `colGrandTotals` the Grand Total column. Both were read and written the other way round, which a GridCraft round trip hid, so an Excel file with only one grand total opened showing the other | +| 2026-10-19 | minor | HTML (Save as Web Page and clipboard) marks right-to-left sheets and right-to-left cell text with `dir` | | 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | diff --git a/docs/function-parity.md b/docs/function-parity.md index 274800699..2343e0de9 100644 --- a/docs/function-parity.md +++ b/docs/function-parity.md @@ -1,6 +1,6 @@ # Function parity: worksheet functions and formulas -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-13 · **Change:** minor (clearing large sparse ranges recalculates every changed cell's dependents, #1032) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's worksheet functions, category by category, against GridCraft's function library. Part of [target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md). @@ -101,6 +101,11 @@ fraction sections with literal text (`#\:#=/=#`, `#_#/#`), zero fractions, zero- denominators, `?` placeholders padding the thousands separator, and booleans through the text section (`TEXT(TRUE,";;;\@@\@")` = `@TRUE@`). +`MEDIAN` and `AGGREGATE` function number 12 use an overflow-safe midpoint for even-sized +inputs, so large finite middle values do not become `#NUM!` merely because their sum overflows (#1579). +Sorting, argument coercion and error propagation are unchanged; this is a numerical correction +under the documented median rule, not an extreme-value comparison against desktop Excel. + Work to close (16–26 h in total): the in-scope missing functions (10–16 h with the table above), an oracle corpus of Excel-computed results per function (owner runs Excel on generated workbooks; results checked in as data, not Excel files) and fixing what it finds (6–10 h). @@ -109,6 +114,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | MEDIAN and AGGREGATE median use an overflow-safe midpoint (#1579); regressions cover finite extremes, tiny values and dependent recalculation | | 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) | | 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) | | 2026-10-11 | minor | TEXT / number formats fixed against Excel-cached results: fraction literals and zero fractions, `0` denominators zero padded, `?` pads the thousands separator, booleans use the text section (226 mismatches → 0 in 352 TEXT cells) | diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 09197ff6c..64876633d 100644 --- a/docs/localization-parity.md +++ b/docs/localization-parity.md @@ -1,6 +1,6 @@ # Localization parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (Format Cells dialog looks its labels up in the catalog) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (defined-name length limit counts characters, so long Arabic names are accepted; Table names are limited to 255 characters; right-to-left sheets in `.xls` files open right-to-left; text boxes and shapes keep right-to-left paragraph direction through XLSX save; Save as Web Page and copied HTML keep right-to-left; French and Serbian `context|label` translations no longer show the context) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Per-language status against Excel, which ships its UI in dozens of languages (Windows language packs cover 100+), with localized function names, argument separators, number and date formats, @@ -28,11 +28,16 @@ right-to-left sheets and full complex-script support. Part of - **Script support.** System-font fallback for CJK, Thai and Hebrew landed in #38 (closes #32; #142 is still open pending confirmation). There is no complex-text shaping: Arabic and Devanagari don't join or reorder, and there is no bidirectional layout. **Sheet Right-to-Left** - (`view.rightToLeft`, Page Layout › Sheet Options, XLSX `rightToLeft`) mirrors the grid (#146): + (`view.rightToLeft`, Page Layout › Sheet Options, XLSX `rightToLeft`, `.xls` WINDOW2 fRightToLeft) mirrors the grid (#146): column A and the row headers on the right, the vertical scrollbar on the left; hit-testing, arrow keys, editor, popups and floating objects follow; text stays left-to-right inside its box - (no bidi), and the sheet tabs, horizontal scrollbar and printing/PDF are not mirrored yet. IME input goes + (no bidi), and the sheet tabs, horizontal scrollbar and printing/PDF are not mirrored yet. + Text boxes and shapes keep their paragraphs' right-to-left direction (`a:pPr rtl`) through XLSX + save, though GridCraft still draws that text left to right. IME input goes through egui/winit and is unverified for Chinese, Japanese and Korean. +- **Names in any script.** Defined names (Define Name, Create from Selection) are limited to + 255 characters, counted as characters rather than UTF-8 bytes, so a 255-letter Arabic name is + accepted as in Excel. Sheet names already counted characters (31). - **PDF export** uses only the 14 standard PDF fonts: non-Latin text cannot export. ## Languages @@ -78,6 +83,12 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | Defined-name length counts characters, not bytes: Arabic names up to 255 letters are accepted, and Create from Selection keeps long Arabic headers | +| 2026-10-19 | minor | Table names are limited to 255 Unicode characters | +| 2026-10-19 | minor | Right-to-left sheets in legacy `.xls` files (common in older Arabic workbooks) open right-to-left | +| 2026-10-19 | trivial | Text boxes and shapes keep right-to-left paragraph direction (`a:pPr rtl`) on XLSX save, so Excel-authored Arabic and Hebrew text boxes reopen right to left | +| 2026-10-19 | minor | Save as Web Page and copied HTML mark right-to-left sheets `dir="rtl"`, let General-aligned Arabic and Hebrew text align by its own direction (`dir="auto"`), and keep numbers on right-to-left sheets left to right and on the left | +| 2026-10-19 | minor | French and Serbian kept the context in three `context\|label` translations ("Options\|Général", "Opcije\|Opšte" for `Options\|General`; also `Options\|Save`, `Sheet Options\|View`). `tr` shows a found translation as it is, so File › Options headings and the Sheet Options "View" checkboxes showed the prefix. Fixed; a test checks that no catalog shows a `\|` | | 2026-10-12 | minor | Format Cells looks up its tabs, labels, options and note through `Language::tr` (Fill tab keyed `Format Cells\|Fill`); dialog titles and OK/Cancel too. Labels already in a table (Number, Currency, Fill…) show translated; the rest fall back to English until the tables add them | | 2026-10-12 | minor | Brazilian Portuguese catalog (`i18n/pt.rs`) grows from 377 to 923 entries: every key the other catalogs translate, including the dialog strings #669 starts looking up (~42%) | | 2026-10-11 | minor | German ribbon catalog (`i18n/de.rs`, 266 labels, ~12%); `de` locale tags (de-DE, de-AT, de-CH…) resolve to German. PR #163 (localized German formulas/numbers) still open | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..34cb502f5 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (horizontal scrollbar in the sheet tab bar, #840) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** minor (Ctrl+Enter fill gives each cell the automatic number format typing gives it; keytip letters typed with Alt held, #1846) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -77,7 +77,7 @@ this logic, with one undo step. | Ctrl+1, Ctrl+; and Ctrl+Shift+; , Ctrl+D/R, Ctrl+E, Ctrl+T/L, Ctrl+K, Ctrl+Shift+L, Ctrl+Shift+~!@#$%^&_ | yes | done | | F2, F4, F5, F7, F9 family, Shift+F2/F3/F10/F11, F12 | yes | done; F11 chart sheet, F8 extend mode, F3 Paste Name, F4 Repeat missing | | Mac-specific (Cmd variants, Control variants, Fn keys) | yes | done where a feature exists (`keymap.rs` picks the Mac variant) | -| Sequential Alt access keys (Alt, H, N, …) on every ribbon tab | yes | 364-sequence keytip table for all tabs: per-control badges, prefix waiting, dropdowns open for editing (type/arrows/Enter), pending bindings for not-yet-built commands; QAT/Backstage excluded | +| Sequential Alt access keys (Alt, H, N, …) on every ribbon tab | yes | 364-sequence keytip table for all tabs: per-control badges, prefix waiting, dropdowns open for editing (type/arrows/Enter), pending bindings for not-yet-built commands; letters also work with left Alt held (Alt+H, O, A), as in Excel; QAT/Backstage excluded | | Requests | — | #110 (more shortcuts and hot keys), #33 (sequential Alt; initial support landed in #43) | ### Shortcut audit (#110) @@ -161,6 +161,8 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-19 | minor | Ctrl+Enter (`range.fill`) gives each cell what typing the input there gives: a General cell gets the automatic number format (`5%`, `$5`, `3/4/2026`, `=TODAY()`) even when it already held a value, and a cell with its own format, a formatted row or column, or Text keeps it. Cells that held a value used to keep General (`5%` showed 0.05), and empty cells took the active cell's format | +| 2026-10-18 | minor | Keytip letters typed with left Alt held continue the sequence (Alt+H, O, A / Alt+H, O, I ran nothing before: Alt+letter cancelled keytips); other Alt combinations, AltGr and macOS Option are unchanged (#1846) | | 2026-10-12 | minor | The File view's recent list tells when each workbook was last changed, read off the UI thread | | 2026-10-13 | minor | Horizontal scrollbar in the sheet tab bar: drag, track click pages, frozen columns excluded from the page (#840) | | 2026-10-12 | minor | Ctrl+wheel zoom steps along a ladder of percentages about 20% apart that never skips 100%, gliding to each step with animations on, so a notch out undoes a notch in (it drifted: 100 → 122 → 101); Excel steps 15 points, see decisions.md |