From 99bbd237fcfdd851dd56f26cff0efc6758151c89 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 11:53:25 -0800 Subject: [PATCH 1/8] Updates: the updater reports installing, ready, installed and failed, and installs at exit craft-update, craft-window, craft-print and craft-ui move from craft-libs c5a1cbfb to c7c84230, a branch with storytold/craft-libs#687 and #696 merged into craft-libs main. - The check result window shows whenever the updater's status is not idle, so the first start after an update reports that it installed or why it failed. Restart to Update asks the app to quit once nothing is unsaved. - An update's close request quits the app (Quit asks about each unsaved workbook), so every workbook window closes. - on_exit starts the install helper of a ready update before the macOS process exit. --- Cargo.lock | 17 ++++----- Cargo.toml | 6 +-- apps/gridcraft/Cargo.toml | 2 +- apps/gridcraft/src/main.rs | 3 ++ crates/ui-egui/Cargo.toml | 2 +- crates/ui-egui/src/app/prefs.rs | 8 ++++ crates/ui-egui/src/updates_ui.rs | 63 +++++++++++++++++++++++--------- 7 files changed, 69 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a39620f96..5d78dbed4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -910,12 +910,12 @@ dependencies = [ [[package]] name = "craft-fonts-build" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" [[package]] name = "craft-i18n" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" dependencies = [ "fixed_decimal", "icu_decimal", @@ -938,7 +938,7 @@ dependencies = [ [[package]] name = "craft-print" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" dependencies = [ "craft-winprint-gdi", "log", @@ -952,7 +952,7 @@ dependencies = [ [[package]] name = "craft-ui" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" dependencies = [ "craft-fonts-build", "craft-i18n", @@ -972,7 +972,7 @@ dependencies = [ [[package]] name = "craft-update" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" dependencies = [ "chrono", "dunce", @@ -1018,7 +1018,7 @@ dependencies = [ [[package]] name = "craft-window" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=c5a1cbfb5f9049f9270955caffb5bee5997aebf7#c5a1cbfb5f9049f9270955caffb5bee5997aebf7" +source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" dependencies = [ "egui", "objc2 0.6.5", @@ -1029,7 +1029,7 @@ dependencies = [ [[package]] name = "craft-winprint-gdi" version = "0.1.0" -source = "git+https://github.com/storytold/winprint.rs?rev=417ac3c4d14d3da5485a99c7d5ce8f05664ec312#417ac3c4d14d3da5485a99c7d5ce8f05664ec312" +source = "git+https://github.com/storytold/winprint.rs?rev=64e7afd4d1483512385012c340ecaaaf0f3beb7f#64e7afd4d1483512385012c340ecaaaf0f3beb7f" dependencies = [ "windows-sys 0.61.2", ] @@ -6485,7 +6485,7 @@ dependencies = [ [[package]] name = "winprint" version = "0.2.1" -source = "git+https://github.com/storytold/winprint.rs?rev=5f80b9baf6b58937c401656634e42d33f00ddf49#5f80b9baf6b58937c401656634e42d33f00ddf49" +source = "git+https://github.com/storytold/winprint.rs?rev=64e7afd4d1483512385012c340ecaaaf0f3beb7f#64e7afd4d1483512385012c340ecaaaf0f3beb7f" dependencies = [ "flate2", "fmt-derive", @@ -6935,7 +6935,6 @@ dependencies = [ "nix", "typed-path", "windows-sys 0.61.2", - "zopfli", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 594b29273..bc69f4847 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,9 +26,9 @@ gridcraft-mcp = { path = "crates/mcp" } gridcraft-pdf = { path = "crates/pdf" } gridcraft-math = { path = "crates/math" } # Every craft-libs crate (craft-update and craft-window here, craft-ui in ui-egui, craft-print in -# the app) comes from one storytold/craft-libs revision. +# the app) comes from one craft-libs revision. # craft-update: app-specific pretend versions apply to startup and manual checks. -craft-update = { git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7", version = "=0.1.0", features = ["egui"] } +craft-update = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae", version = "=0.1.0", features = ["egui"] } # foundation thiserror = "2" @@ -71,7 +71,7 @@ skrifa = { version = "0.44", default-features = false, features = ["std"] } epaint_default_fonts = "0.37" # One native window per workbook: the window list, Window menu titles and the immediate-viewport # driver. Same craft-libs revision as craft-update above. -craft-window = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7", default-features = false, features = ["egui"] } +craft-window = { version = "0.1.0", git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae", default-features = false, features = ["egui"] } [workspace.lints.rust] unsafe_code = "forbid" diff --git a/apps/gridcraft/Cargo.toml b/apps/gridcraft/Cargo.toml index a7472821f..e706279ce 100644 --- a/apps/gridcraft/Cargo.toml +++ b/apps/gridcraft/Cargo.toml @@ -34,7 +34,7 @@ winit = { version = "0.30", default-features = false, features = ["x11", "waylan [target.'cfg(not(target_arch = "wasm32"))'.dependencies] # Same craft-libs revision as the workspace's craft-update and craft-window (see the root # Cargo.toml). -craft-print = { git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7" } +craft-print = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae" } craft-update = { workspace = true } rfd = { workspace = true } arboard = { workspace = true, features = ["image-data"] } diff --git a/apps/gridcraft/src/main.rs b/apps/gridcraft/src/main.rs index 2aa98b6eb..cc67e241a 100644 --- a/apps/gridcraft/src/main.rs +++ b/apps/gridcraft/src/main.rs @@ -64,6 +64,9 @@ impl eframe::App for App { fn on_exit(&mut self) { print::finish_pending_jobs(); save_prefs(&self.0); + // A ready update installs once the process has exited. The macOS exit below skips + // dropping the app, so the helper is started here. + self.0.start_pending_update(); // macOS (#117): end the process here, before eframe tears the window down. AccessKit's // macOS adapter subclasses the window's content view and restores its class on teardown; // AppKit's Touch Bar support observes that view (KVO), so in the next display cycle diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index fd18f7041..7dccfd584 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -11,7 +11,7 @@ workspace = true [dependencies] # The shared capture slot (`craft_ui::capture`) comes from the one craft-libs revision every # craft-libs dependency uses (see the root Cargo.toml). -craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "c5a1cbfb5f9049f9270955caffb5bee5997aebf7" } +craft-ui = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae" } gridcraft-engine = { workspace = true } gridcraft-chart = { workspace = true } # `monochrome_emoji_fonts` restores the Noto Emoji and emoji-icon faces that 0.37 moved behind diff --git a/crates/ui-egui/src/app/prefs.rs b/crates/ui-egui/src/app/prefs.rs index 216e4a568..151374e6e 100644 --- a/crates/ui-egui/src/app/prefs.rs +++ b/crates/ui-egui/src/app/prefs.rs @@ -64,4 +64,12 @@ impl SheetApp { self.message = Some(("GridCraft".into(), "The web version is always the latest release.".into())); } } + + /// Starts the install helper of an update that is ready, so it installs once the process has + /// exited (a no-op on the web, and when no update is ready). Call it in `on_exit` when the + /// process ends there, since the app is then never dropped. + pub fn start_pending_update(&mut self) { + #[cfg(not(target_arch = "wasm32"))] + updates_ui::start_pending_update(self); + } } diff --git a/crates/ui-egui/src/updates_ui.rs b/crates/ui-egui/src/updates_ui.rs index 2553fbbc7..cded96c85 100644 --- a/crates/ui-egui/src/updates_ui.rs +++ b/crates/ui-egui/src/updates_ui.rs @@ -15,13 +15,11 @@ pub(crate) struct Runtime { startup: bool, /// Help ▸ Check for Updates… was chosen; the next `tick` starts it. manual: bool, - /// The result dialog of a requested check is showing. - result_open: bool, } impl Default for Runtime { fn default() -> Self { - Self { updater: Updater::new(&crate::UPDATE_APP, VERSION), startup: false, manual: false, result_open: false } + Self { updater: Updater::new(&crate::UPDATE_APP, VERSION), startup: false, manual: false } } } @@ -38,19 +36,26 @@ pub(crate) fn tick(app: &mut SheetApp, ctx: &egui::Context) { let rt = &mut app.updates_rt; if std::mem::take(&mut rt.manual) { rt.updater.check_now(ctx); - rt.result_open = true; } else if rt.startup { rt.updater.check_at_start(ctx); } rt.updater.tick(ctx); - // A requested check that found a release leaves the status idle and the prompt opens. - if matches!(rt.updater.status(), Status::Idle) { - rt.result_open = false; - } +} + +/// Starts the helper of a ready update; it installs once the process has exited. +pub(crate) fn start_pending_update(app: &mut SheetApp) { + app.updates_rt.updater.start_pending_update(); +} + +/// Whether the check result window shows: for every status but idle (a requested check, and the +/// first start after an update, which reports that it installed or why it failed), and never +/// over the startup prompt. +fn result_window_shows(status: &Status, prompt_open: bool) -> bool { + !prompt_open && !matches!(status, Status::Idle) } pub(crate) fn show(app: &mut SheetApp, ctx: &egui::Context) { - // The restart helper is scheduled before the app closes, so no update starts over unsaved work. + // The close for a ready update waits until no workbook holds unsaved work. let clean = !app.session.documents().iter().any(|d| d.is_dirty()); match app.updates_rt.updater.show_if(ctx, clean) { Action::OpenUrl(url) => { @@ -58,20 +63,42 @@ pub(crate) fn show(app: &mut SheetApp, ctx: &egui::Context) { open(&url); } } - // The guarded close path asks about unsaved work; the restart helper relaunches once the - // process has exited. - Action::Close => ctx.send_viewport_cmd(egui::ViewportCommand::Close), + // Quit closes every workbook window. The install helper starts once the app exits. + Action::Close => app.request_quit(ctx), Action::None => {} } - if app.updates_rt.result_open { - let rt = &mut app.updates_rt; + let rt = &mut app.updates_rt; + if result_window_shows(rt.updater.status(), rt.updater.prompt_open()) { match craft_update::CheckResultDialog::new(&crate::UPDATE_APP, VERSION).show(ctx, rt.updater.status()) { - CheckResultResponse::Close => { - rt.updater.dismiss_status(); - rt.result_open = false; - } + CheckResultResponse::Close => rt.updater.dismiss_status(), CheckResultResponse::CheckAgain => rt.updater.check_now(ctx), + // The next `show_if` asks the window to close once nothing is unsaved. + CheckResultResponse::RestartToUpdate => rt.updater.restart_to_update(), CheckResultResponse::None => {} } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn result_window_shows_every_status_but_idle() { + let shown = [ + Status::Checking, + Status::UpToDate, + Status::Failed("no network".into()), + Status::Installing, + Status::Ready, + Status::Managed, + Status::Updated("0.2.0".into()), + Status::InstallFailed("disk full".into()), + ]; + for status in &shown { + assert!(result_window_shows(status, false), "{status:?}"); + assert!(!result_window_shows(status, true), "{status:?} over the prompt"); + } + assert!(!result_window_shows(&Status::Idle, false)); + } +} From 3004a05e463c3aac87b8901152d81a99b63672ac Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 19:45:34 -0800 Subject: [PATCH 2/8] Updates: take craft-update from craft-libs main at 1b34eba41 --- Cargo.lock | 14 +++++++------- Cargo.toml | 4 ++-- apps/gridcraft/Cargo.toml | 2 +- crates/ui-egui/Cargo.toml | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d78dbed4..cb783e546 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -910,12 +910,12 @@ dependencies = [ [[package]] name = "craft-fonts-build" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" [[package]] name = "craft-i18n" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "fixed_decimal", "icu_decimal", @@ -938,7 +938,7 @@ dependencies = [ [[package]] name = "craft-print" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "craft-winprint-gdi", "log", @@ -952,7 +952,7 @@ dependencies = [ [[package]] name = "craft-ui" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "craft-fonts-build", "craft-i18n", @@ -972,7 +972,7 @@ dependencies = [ [[package]] name = "craft-update" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "chrono", "dunce", @@ -1018,7 +1018,7 @@ dependencies = [ [[package]] name = "craft-window" version = "0.1.0" -source = "git+https://github.com/dotCooCoo/craft-libs?rev=c7c84230631cbc01ee928f0a0e0d838b1ed612ae#c7c84230631cbc01ee928f0a0e0d838b1ed612ae" +source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" dependencies = [ "egui", "objc2 0.6.5", @@ -6485,7 +6485,7 @@ dependencies = [ [[package]] name = "winprint" version = "0.2.1" -source = "git+https://github.com/storytold/winprint.rs?rev=64e7afd4d1483512385012c340ecaaaf0f3beb7f#64e7afd4d1483512385012c340ecaaaf0f3beb7f" +source = "git+https://github.com/storytold/winprint.rs?rev=51934db382f225454cf152a8657eb03789be63c4#51934db382f225454cf152a8657eb03789be63c4" dependencies = [ "flate2", "fmt-derive", diff --git a/Cargo.toml b/Cargo.toml index bc69f4847..1941c9d1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ gridcraft-math = { path = "crates/math" } # Every craft-libs crate (craft-update and craft-window here, craft-ui in ui-egui, craft-print in # the app) comes from one craft-libs revision. # craft-update: app-specific pretend versions apply to startup and manual checks. -craft-update = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae", version = "=0.1.0", features = ["egui"] } +craft-update = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", version = "=0.1.0", features = ["egui"] } # foundation thiserror = "2" @@ -71,7 +71,7 @@ skrifa = { version = "0.44", default-features = false, features = ["std"] } epaint_default_fonts = "0.37" # One native window per workbook: the window list, Window menu titles and the immediate-viewport # driver. Same craft-libs revision as craft-update above. -craft-window = { version = "0.1.0", git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae", default-features = false, features = ["egui"] } +craft-window = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", default-features = false, features = ["egui"] } [workspace.lints.rust] unsafe_code = "forbid" diff --git a/apps/gridcraft/Cargo.toml b/apps/gridcraft/Cargo.toml index e706279ce..75acee3ff 100644 --- a/apps/gridcraft/Cargo.toml +++ b/apps/gridcraft/Cargo.toml @@ -34,7 +34,7 @@ winit = { version = "0.30", default-features = false, features = ["x11", "waylan [target.'cfg(not(target_arch = "wasm32"))'.dependencies] # Same craft-libs revision as the workspace's craft-update and craft-window (see the root # Cargo.toml). -craft-print = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae" } +craft-print = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d" } craft-update = { workspace = true } rfd = { workspace = true } arboard = { workspace = true, features = ["image-data"] } diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 7dccfd584..b4c049afa 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -11,7 +11,7 @@ workspace = true [dependencies] # The shared capture slot (`craft_ui::capture`) comes from the one craft-libs revision every # craft-libs dependency uses (see the root Cargo.toml). -craft-ui = { git = "https://github.com/dotCooCoo/craft-libs", rev = "c7c84230631cbc01ee928f0a0e0d838b1ed612ae" } +craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d" } gridcraft-engine = { workspace = true } gridcraft-chart = { workspace = true } # `monochrome_emoji_fonts` restores the Noto Emoji and emoji-icon faces that 0.37 moved behind From 7102429a9a2fd9f6fba8e06238a1cce5807fb1b0 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Mon, 9 Nov 2026 08:46:53 +0100 Subject: [PATCH 3/8] perf(grid): skip hidden row runs and keep sheet geometry and CF statistics between frames (#3580) (#3629) A sheet whose filter hides a million rows walked every hidden row several times per frame (cells, gridlines, headers), rebuilt the row index, the outline and the used range every frame, and a conditional format over a whole column rescanned it every frame for its statistics. - LineIndex::next_visible / visible step over a run of hidden lines in one O(log n) search; the grid's row and column loops use them. - The grid keeps the active sheet's line index, outlines, used range and CfCache between frames, tied to the sheet by a Weak: any change goes through Arc::make_mut, which moves or copies a sheet that has a weak reference, so the cache can't outlive a change. - Scroll offsets for the scrollbars and the scroll limit come from the line index instead of walking every custom row. Co-authored-by: Claude Opus 5.5 --- crates/model/src/sheet.rs | 99 ++++++++++++++++++++++++++++- crates/ui-egui/src/grid.rs | 8 +++ crates/ui-egui/src/grid/geo.rs | 67 ++++++++++++++++--- crates/ui-egui/src/grid/quadrant.rs | 24 ++----- crates/ui-egui/src/grid/show.rs | 50 ++++++++++++--- crates/ui-egui/src/grid/tests.rs | 83 ++++++++++++++++++++++++ 6 files changed, 294 insertions(+), 37 deletions(-) diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 440647d42..409e93023 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -566,7 +566,9 @@ impl LineIndex { let mut prev = 0u32; for (&line, info) in custom { pos += (line - prev) as f64 * default; - let size = if info.hidden { 0.0 } else { info.size.map_or(default, |s| s as f64) }; + // A negative size (only a malformed file has one) counts as zero, so positions only + // grow and the searches below stay valid. + let size = if info.hidden { 0.0 } else { info.size.map_or(default, |s| s as f64).max(0.0) }; lines.push((line, pos, size)); pos += size; prev = line + 1; @@ -584,6 +586,44 @@ impl LineIndex { }, } } + /// The first line at or after `line` (and before the axis end) with a size above zero, or + /// `None` when every line from there on is hidden or zero-sized. O(log n): a run of hidden + /// lines all start where the next shown line starts, so one search steps over the whole run. + pub fn next_visible(&self, line: u32) -> Option { + if line >= self.max { + return None; + } + let i = self.lines.partition_point(|e| e.0 < line); + let custom = self.lines.get(i).is_some_and(|e| e.0 == line); + if !custom && self.default > 0.0 { + return Some(line); + } + // Every line from `line` up to the answer is zero-sized, so they all start at `p`; the + // last custom line starting at `p` either has a size (the answer) or is the end of the run. + let p = self.start(line); + let k = self.lines.partition_point(|e| e.1 <= p); + let &(last, _, size) = self.lines.get(k.checked_sub(1)?)?; + if last < line { + return None; + } + if size > 0.0 { + return Some(last).filter(|l| *l < self.max); + } + // The line after the run has no custom line (a custom one would start at `p` too and be + // part of the run): it is shown when the default size is. + let next = last.checked_add(1).filter(|n| *n < self.max)?; + (self.default > 0.0).then_some(next) + } + /// The lines from `first` to `last` (inclusive) with a size above zero, in order. Runs of + /// hidden lines cost one O(log n) step each, not one per line. + pub fn visible(&self, first: u32, last: u32) -> impl Iterator + '_ { + let mut next = self.next_visible(first); + std::iter::from_fn(move || { + let line = next.filter(|l| *l <= last)?; + next = line.checked_add(1).and_then(|n| self.next_visible(n)); + Some(line) + }) + } /// The line containing `pos`. pub fn at(&self, pos: f64) -> u32 { if pos <= 0.0 { @@ -644,6 +684,63 @@ mod tests { use super::*; use crate::cell::Formula; + /// `LineIndex::visible` lists exactly the lines a line-by-line size check shows, for hidden + /// runs at the start, in the middle and up to the end of the axis, custom sizes (zero and + /// negative included), lines with only a style, and a zero default size. + #[test] + fn visible_lines_match_a_line_by_line_check() { + let max = 400u32; + let mut seed = 0x2545_f491_4f6c_dd1du64; + let mut rnd = |n: u64| { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + seed % n + }; + for case in 0..300 { + let mut custom = BTreeMap::new(); + let runs = rnd(6); + for _ in 0..runs { + let start = rnd(u64::from(max)) as u32; + let len = rnd(if case % 3 == 0 { 400 } else { 30 }) as u32; + for l in start..(start + len).min(max) { + custom.insert(l, LineInfo { hidden: true, ..Default::default() }); + } + } + // Runs that start at the first line or end at the last one. + if case % 5 == 0 { + for l in 0..rnd(20) as u32 { + custom.insert(l, LineInfo { hidden: true, ..Default::default() }); + } + } + if case % 5 == 1 { + for l in max - rnd(20) as u32 - 1..max { + custom.insert(l, LineInfo { hidden: true, ..Default::default() }); + } + } + for _ in 0..rnd(8) { + let size = [Some(0.0), Some(3.0), Some(20.0), Some(-5.0), None][rnd(5) as usize]; + custom.insert(rnd(u64::from(max)) as u32, LineInfo { size, ..Default::default() }); + } + let default = if case % 7 == 0 { 0.0 } else { 15.0 }; + let size = |l: u32| match custom.get(&l) { + Some(i) if i.hidden => 0.0, + Some(LineInfo { size: Some(s), .. }) => s.max(0.0), + _ => default, + }; + let index = LineIndex::new(&custom, default, max); + for (first, last) in [(0, max - 1), (rnd(u64::from(max)) as u32, max - 1), (5, 5), (max - 1, max - 1)] { + let want: Vec = (first..=last).filter(|&l| size(l) > 0.0).collect(); + assert_eq!(index.visible(first, last).collect::>(), want, "case {case}, {first}..={last}"); + } + assert_eq!(index.next_visible(max), None); + } + // The row after a million hidden rows comes straight after the last shown one. + let custom: BTreeMap = (47..1_048_422).map(|r| (r, LineInfo { hidden: true, ..Default::default() })).collect(); + let index = LineIndex::new(&custom, 15.0, 1_048_576); + assert_eq!(index.visible(40, 1_048_424).collect::>(), [40, 41, 42, 43, 44, 45, 46, 1_048_422, 1_048_423, 1_048_424]); + } + #[test] fn a_sheet_keeps_its_identity_through_clones_and_a_new_sheet_gets_its_own() { let a = Sheet::new("A"); diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index f0ae6270e..1a5d71621 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -235,6 +235,14 @@ impl Trace { #[derive(Default)] pub struct GridState { + /// The active sheet's line positions and outlines, kept with a weak reference to the sheet + /// they were built from: while it still upgrades to the very sheet shown, the sheet hasn't + /// changed (every change goes through `Arc::make_mut`, which moves or copies a sheet that + /// has a weak reference to it), so they still hold. + pub(crate) lines: Option<(std::sync::Weak, geo::SheetLines)>, + /// The conditional formatting statistics (minimum, maximum, percentiles, duplicates) of the + /// active sheet's rules, kept the same way: they are read from that sheet's cells alone. + pub(crate) cf: Option<(std::sync::Weak, CfCache)>, pub drag: Drag, pub ensure_visible: bool, /// With `ensure_visible`: scroll to the moving end of the selection instead of the active cell diff --git a/crates/ui-egui/src/grid/geo.rs b/crates/ui-egui/src/grid/geo.rs index a1005b32f..7786f4048 100644 --- a/crates/ui-egui/src/grid/geo.rs +++ b/crates/ui-egui/src/grid/geo.rs @@ -24,28 +24,60 @@ pub struct Geo { /// vertical centre line on screen: painted shapes by `mirror_painted`, widgets by /// `mirror_rect`/`mirror_pos`, and pointer input back by `mirror_pos` (an involution). pub rtl: bool, - /// Prefix sums over custom row heights / column widths, built once per frame. + /// Prefix sums over custom row heights / column widths, from [`SheetLines`]. pub(super) ri: std::sync::Arc, pub(super) ci: std::sync::Arc, + /// The sheet's used range, from [`SheetLines`]. + pub(super) used: Option, /// Page Layout view: cells sit on pages (`scroll` is then in page-layout units). pub paper: Option>, } +/// The parts of the grid's geometry that depend on the sheet alone: where each row and column +/// starts, and the row and column outlines. Building them walks every row and column that has a +/// size, a style or an outline level (a million rows in a file whose filter hides most of them), +/// so the grid keeps them while the sheet stays the same (`show::sheet_lines`). +#[derive(Clone)] +pub struct SheetLines { + pub(super) ri: Arc, + pub(super) ci: Arc, + pub(super) row_outline: Arc, + pub(super) col_outline: Arc, + /// The sheet's used range (it walks every stored row): how far the scrollbars reach. + pub(super) used: Option, +} + +impl SheetLines { + pub fn of(sh: &Sheet) -> SheetLines { + SheetLines { + ri: Arc::new(LineIndex::new(&sh.rows, sh.default_row_height, MAX_ROWS)), + ci: Arc::new(LineIndex::new(&sh.cols, sh.default_col_width, MAX_COLS)), + row_outline: Arc::new(crate::outline::Axis::of(sh, true)), + col_outline: Arc::new(crate::outline::Axis::of(sh, false)), + used: sh.used_range(), + } + } +} + impl Geo { pub fn new(sh: &Sheet, rect: Rect, scroll: egui::Vec2) -> Geo { + Geo::with_lines(sh, rect, scroll, &SheetLines::of(sh)) + } + /// [`Geo::new`] with the sheet's `lines` already built (`SheetLines::of(sh)`). + pub fn with_lines(sh: &Sheet, rect: Rect, scroll: egui::Vec2, lines: &SheetLines) -> Geo { let zoom = (sh.zoom as f32 / 100.0).clamp(0.1, 4.0); let z = zoom * DISPLAY_SCALE; let header_h = if sh.show_headings { (COL_HEADER_H * zoom.max(0.75)).round() } else { 0.0 }; - let ri = std::sync::Arc::new(LineIndex::new(&sh.rows, sh.default_row_height, MAX_ROWS)); - let ci = std::sync::Arc::new(LineIndex::new(&sh.cols, sh.default_col_width, MAX_COLS)); + let ri = lines.ri.clone(); + let ci = lines.ci.clone(); let last_row = ri.at((scroll.y + rect.height() / z) as f64 + 200.0) + 1; let digits = (last_row as f32).log10().floor() as i32 + 1; let header_w = if sh.show_headings { ((digits.max(2) as f32 * 7.0 + 14.0) * zoom.max(0.75)).round() } else { 0.0 }; let (fr, fc) = sh.freeze.unwrap_or((0, 0)); let frozen_w = (ci.start(fc) as f32) * z; let frozen_h = (ri.start(fr) as f32) * z; - let row_outline = Arc::new(crate::outline::Axis::of(sh, true)); - let col_outline = Arc::new(crate::outline::Axis::of(sh, false)); + let row_outline = lines.row_outline.clone(); + let col_outline = lines.col_outline.clone(); // Show Outline Symbols off: no bands (the outline's hidden lines stay hidden). let symbols = sh.show_outline_symbols; let mut band_w = if symbols { crate::outline::band(row_outline.levels, zoom) } else { 0.0 }; @@ -76,6 +108,7 @@ impl Geo { rtl: sh.right_to_left, ri, ci, + used: lines.used, paper: None, } } @@ -90,7 +123,11 @@ impl Geo { /// Page Layout view geometry: `rect` excludes the rulers; cells are drawn at the print scale /// on pages laid out by `grid`; frozen panes don't apply (as in Excel). pub fn paged(sh: &Sheet, rect: Rect, scroll: egui::Vec2, grid: Arc) -> Geo { - let mut g = Geo::new(sh, rect, scroll); + Geo::paged_with_lines(sh, rect, scroll, grid, &SheetLines::of(sh)) + } + /// [`Geo::paged`] with the sheet's `lines` already built. + pub fn paged_with_lines(sh: &Sheet, rect: Rect, scroll: egui::Vec2, grid: Arc, lines: &SheetLines) -> Geo { + let mut g = Geo::with_lines(sh, rect, scroll, lines); let zoom = (sh.zoom as f32 / 100.0).clamp(0.1, 4.0); g.z = zoom * DISPLAY_SCALE * grid.scale.clamp(0.1, 4.0); (g.fr, g.fc, g.frozen_w, g.frozen_h) = (0, 0, 0.0, 0.0); @@ -111,17 +148,17 @@ impl Geo { } } /// Scroll offset (in `scroll` units) at which `row` reaches the top of the scrolled area. - pub fn scroll_top_of(&self, sh: &Sheet, row: u32) -> f32 { + pub fn scroll_top_of(&self, _sh: &Sheet, row: u32) -> f32 { match &self.paper { Some(pg) => pg.row_y(row) as f32, - None => (sh.row_top(row) - sh.row_top(self.fr)) as f32, + None => (self.ri.start(row) - self.ri.start(self.fr)) as f32, } } /// Scroll offset (in `scroll` units) at which `col` reaches the left of the scrolled area. - pub fn scroll_left_of(&self, sh: &Sheet, col: u32) -> f32 { + pub fn scroll_left_of(&self, _sh: &Sheet, col: u32) -> f32 { match &self.paper { Some(pg) => pg.col_x(col) as f32, - None => (sh.col_left(col) - sh.col_left(self.fc)) as f32, + None => (self.ci.start(col) - self.ci.start(self.fc)) as f32, } } /// Screen x for a logical x (and back): identity unless the sheet is right-to-left. @@ -204,6 +241,16 @@ impl Geo { pub fn cell_at(&self, sh: &Sheet, p: Pos2) -> CellRef { CellRef::new(self.row_at(sh, p.y), self.col_at(sh, p.x)) } + /// The rows from `r0` to `r1` (inclusive) with a height above zero, in order. A run of + /// hidden rows costs one O(log n) step, so a filter that hides a million rows doesn't make + /// every frame walk them. + pub(super) fn shown_rows(&self, r0: u32, r1: u32) -> impl Iterator + '_ { + self.ri.visible(r0, r1) + } + /// The columns from `c0` to `c1` (inclusive) with a width above zero (see `shown_rows`). + pub(super) fn shown_cols(&self, c0: u32, c1: u32) -> impl Iterator + '_ { + self.ci.visible(c0, c1) + } /// Visible scrolled rows (from the first visible row after the frozen ones). pub(super) fn scroll_rows(&self, _sh: &Sheet) -> (u32, u32) { let top = self.ri.start(self.fr) + self.scroll.y as f64; diff --git a/crates/ui-egui/src/grid/quadrant.rs b/crates/ui-egui/src/grid/quadrant.rs index 7461c2204..e209e4f4a 100644 --- a/crates/ui-egui/src/grid/quadrant.rs +++ b/crates/ui-egui/src/grid/quadrant.rs @@ -150,12 +150,12 @@ fn paint_gridlines(q: &QuadrantPaint<'_>) { let bottom = geo.y_end(sh, r1); let left = geo.x(sh, c0); let right = geo.x_end(sh, c1); - let xs = (c0..=c1).filter(|&c| sh.col_width(c) > 0.0).map(|c| geo.x(sh, c)).chain([right]); + let xs = geo.shown_cols(c0, c1).map(|c| geo.x(sh, c)).chain([right]); for x in xs { let x = x.round() - 0.5; p.line_segment([pos2(x, top), pos2(x, bottom)], stroke); } - let ys = (r0..=r1).filter(|&r| sh.row_height(r) > 0.0).map(|r| geo.y(sh, r)).chain([bottom]); + let ys = geo.shown_rows(r0, r1).map(|r| geo.y(sh, r)).chain([bottom]); for y in ys { let y = y.round() - 0.5; p.line_segment([pos2(left, y), pos2(right, y)], stroke); @@ -164,21 +164,14 @@ fn paint_gridlines(q: &QuadrantPaint<'_>) { } fn visible_cells(q: &QuadrantPaint<'_>) -> Vec { - let sh = q.sheet; + let geo = q.geo; let (r0, r1) = q.rows; let (c0, c1) = q.cols; // Merged areas: blank them (no inner gridlines) and paint as one cell. + let cols: Vec = geo.shown_cols(c0, c1).collect(); let mut cells: Vec = Vec::new(); - for r in r0..=r1 { - if sh.row_height(r) == 0.0 { - continue; - } - for c in c0..=c1 { - if sh.col_width(c) == 0.0 { - continue; - } - cells.push(CellRef::new(r, c)); - } + for r in geo.shown_rows(r0, r1) { + cells.extend(cols.iter().map(|&c| CellRef::new(r, c))); } cells } @@ -225,10 +218,7 @@ fn outside_text_cells(q: &mut QuadrantPaint<'_>, merges: &[RangeRef]) -> Vec<(Ce // Text cells just outside the quadrant whose text may overflow into it: the nearest non-empty // cell left of `c0` and right of `c1` in each row (within the same frozen / scrolled pane). let mut outside: Vec<(CellRef, Rect, Look, Option)> = Vec::new(); - for r in r0..=r1 { - if sh.row_height(r) == 0.0 { - continue; - } + for r in geo.shown_rows(r0, r1) { let left = (c0.saturating_sub(20)..c0).rev().map(|col| CellRef::new(r, col)).find(|&n| !empty_ok(n)); let right = (c1.saturating_add(1)..=c1.saturating_add(20).min(MAX_COLS - 1)).map(|col| CellRef::new(r, col)).find(|&n| !empty_ok(n)); for n in [left, right].into_iter().flatten() { diff --git a/crates/ui-egui/src/grid/show.rs b/crates/ui-egui/src/grid/show.rs index c1c9011e7..34b76c982 100644 --- a/crates/ui-egui/src/grid/show.rs +++ b/crates/ui-egui/src/grid/show.rs @@ -15,6 +15,33 @@ use super::{ #[cfg(test)] use super::{CellRef, Pos2}; +/// The line positions and outlines of sheet `si`: the ones built for it before while it is the +/// same sheet, else built now and kept (see `GridState::lines`). +fn sheet_lines(app: &mut SheetApp, wb: &Workbook, si: usize) -> Option { + let sheet = wb.sheets.get(si)?; + if let Some((built_from, lines)) = &app.grid.lines + && is_sheet(built_from, wb, si) + { + return Some(lines.clone()); + } + let lines = super::geo::SheetLines::of(sheet); + app.grid.lines = Some((Arc::downgrade(sheet), lines.clone())); + Some(lines) +} + +/// Whether `built_from` still is sheet `si` of `wb`, unchanged (see `GridState::lines`). +fn is_sheet(built_from: &std::sync::Weak, wb: &Workbook, si: usize) -> bool { + wb.sheets.get(si).is_some_and(|s| built_from.upgrade().is_some_and(|b| Arc::ptr_eq(&b, s))) +} + +/// The conditional formatting cache kept for sheet `si` while it is unchanged, else a new one. +fn kept_cf(app: &mut SheetApp, wb: &Workbook, si: usize) -> CfCache { + match app.grid.cf.take() { + Some((built_from, cf)) if is_sheet(&built_from, wb, si) => cf, + _ => CfCache::new(), + } +} + pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); let available = ui.available_rect_before_wrap(); @@ -44,15 +71,16 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { } else { Rect::from_min_max(pos2((rect.left() + ruler).min(rect.right()), (rect.top() + ruler).min(rect.bottom())), rect.max) }; + let Some(lines) = sheet_lines(app, &wb, si) else { return }; let make_geo = |app: &mut SheetApp, scroll: egui::Vec2| -> Geo { if paper { - let probe = Geo::new(sh, grid_rect, scroll); + let probe = Geo::with_lines(sh, grid_rect, scroll, &lines); if let Some(g) = crate::paper_view::grid_for(app, &wb, si, sh, probe.cells.size(), scroll) { let g = crate::paper_view::with_dragged_margin(app, &wb, si, g); - return Geo::paged(sh, grid_rect, scroll, g); + return Geo::paged_with_lines(sh, grid_rect, scroll, g, &lines); } } - Geo::new(sh, rect, scroll) + Geo::with_lines(sh, rect, scroll, &lines) }; // Scrolling. let (scroll_delta, zoom_delta, modifiers, now) = ui.input(|i| (i.smooth_scroll_delta, i.zoom_delta(), i.modifiers, i.time)); @@ -115,7 +143,7 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { } update_zoom_glide(app, ui, place, sh.zoom, now); view.scroll = view.scroll.max(egui::Vec2::ZERO); - let (mut max_x, mut max_y) = (sh.col_left(MAX_COLS - 20) as f32, sh.row_top(MAX_ROWS - 40) as f32); + let (mut max_x, mut max_y) = (lines.ci.start(MAX_COLS - 20) as f32, lines.ri.start(MAX_ROWS - 40) as f32); if paper { // Page layout units run past sheet units (margins and gaps); the end is known once the // pages reach the last row / column. @@ -146,7 +174,10 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { painter.rect_filled(rect, 0.0, t.grid_bg); // Everything up to the headers is painted in logical (left-to-right) coordinates. let mirror_from = painter.add(egui::Shape::Noop); - let mut cf = CfCache::new(); + // A rule over a whole column scans it for its statistics: done once per change of the sheet, + // not every frame. + let cf_sheet = wb.sheets.get(si).map(Arc::downgrade); + let mut cf = kept_cf(app, &wb, si); let quads = if geo.paper.is_some() { crate::paper_view::paint_pages( crate::paper_view::PagePaintInput { @@ -168,6 +199,7 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { &mut cf, ) }; + app.grid.cf = cf_sheet.map(|s| (s, cf)); // Objects above cells. A hidden object can't stay selected: Delete or the Format pane would act // on something the user can't see. if crate::chartview::objects_hidden(&wb) { @@ -448,7 +480,7 @@ fn vertical_scrollbar(app: &mut SheetApp, ui: &mut egui::Ui, sh: &Sheet, geo: &G } let visible = track.height() / geo.z; let scroll = app.view().scroll.y; - let last = sh.used_range().map(|r| r.end.row.saturating_add(10)).unwrap_or(100).clamp(100, MAX_ROWS - 1); + let last = geo.used.map(|r| r.end.row.saturating_add(10)).unwrap_or(100).clamp(100, MAX_ROWS - 1); let content = geo.scroll_top_of(sh, last); let extent = (content - visible).max(scroll).max(0.0).min(limit); let id = ui.scope_id().with("vscroll"); @@ -501,7 +533,7 @@ pub(crate) fn horizontal_scrollbar(app: &mut SheetApp, ui: &mut egui::Ui, track: let Some(sh) = wb.active() else { return }; let visible = ((geo.cells.width() - geo.frozen_w) / geo.z).max(0.0); let scroll = app.view().scroll.x; - let last = sh.used_range().map(|r| r.end.col.saturating_add(10)).unwrap_or(26).clamp(26, MAX_COLS - 1); + let last = geo.used.map(|r| r.end.col.saturating_add(10)).unwrap_or(26).clamp(26, MAX_COLS - 1); let content = geo.scroll_left_of(sh, last); let extent = (content - visible).max(scroll).max(0.0); let id = ui.scope_id().with("hscroll"); @@ -574,7 +606,7 @@ pub(crate) fn paint_headers( for (rows, cols, clip) in quads { let pc = p.with_clip_rect(Rect::from_min_max(pos2(clip.left(), top.top()), pos2(clip.right(), top.bottom()))); if cols.0 <= cols.1 { - for c in cols.0..=cols.1 { + for c in geo.shown_cols(cols.0, cols.1) { if !done_cols.insert((c, clip.left() as i32)) { continue; } @@ -615,7 +647,7 @@ pub(crate) fn paint_headers( } let pr = p.with_clip_rect(Rect::from_min_max(pos2(left.left(), clip.top()), pos2(left.right(), clip.bottom()))); if rows.0 <= rows.1 { - for rr in rows.0..=rows.1 { + for rr in geo.shown_rows(rows.0, rows.1) { if !done_rows.insert((rr, clip.top() as i32)) { continue; } diff --git a/crates/ui-egui/src/grid/tests.rs b/crates/ui-egui/src/grid/tests.rs index 427c67629..be3ca6eb5 100644 --- a/crates/ui-egui/src/grid/tests.rs +++ b/crates/ui-egui/src/grid/tests.rs @@ -350,3 +350,86 @@ fn upward_text_with_middle_align_is_centred_in_a_merged_block() { assert!((bb.center().y - rect.center().y).abs() < 0.01, "{bb:?} not centred in {rect:?}"); assert!((bb.center().x - rect.center().x).abs() < 0.01, "{bb:?} not centred in {rect:?}"); } + +/// One grid frame of `app` in a headless context. +fn grid_frame(app: &mut crate::SheetApp) { + let ctx = egui::Context::default(); + let screen = egui::Rect::from_min_size(egui::pos2(0.0, 0.0), egui::vec2(1200.0, 800.0)); + let mut out = ctx.run_ui(egui::RawInput { screen_rect: Some(screen), ..Default::default() }, |ui| super::show(app, ui)); + out.textures_delta.clear(); // nothing consumes the uploads in a headless test +} + +/// The line positions, used range and conditional formatting statistics the grid keeps between +/// frames are rebuilt after every change of the sheet, whichever way it is changed, and match +/// what a fresh build gives. +#[test] +fn kept_sheet_geometry_and_cf_statistics_follow_every_change() { + use gridcraft_engine::cf::CfCache; + use gridcraft_engine::core::CellRef; + let mut session = gridcraft_engine::Session::new(); + session.new_workbook(); + let mut app = crate::SheetApp::new(session, Default::default()); + app.run("range.setValues", json!({"range": "A1:A3", "values": [[1], [2], [3]]})).unwrap(); + app.run("home.conditionalFormat", json!({"range": "A:A", "rule": {"type": "colorScale", "colors": ["#FFFFFF", "#FF0000"]}})).unwrap(); + let kept = |app: &crate::SheetApp| app.grid.lines.as_ref().map(|(_, l)| (Arc::clone(&l.ri), l.used)).unwrap(); + // The kept statistics give the look a fresh cache gives. + let check_cf = |app: &mut crate::SheetApp| { + let wb = app.session.active().unwrap().wb.clone(); + for row in 0..4 { + let c = CellRef::new(row, 0); + let kept = app.grid.cf.as_mut().map(|(_, cf)| cf.look(&wb, 0, c)); + assert_eq!(kept, Some(CfCache::new().look(&wb, 0, c)), "A{}", row + 1); + } + }; + grid_frame(&mut app); + let (ri, used) = kept(&app); + assert_eq!(used.map(|r| r.a1()).as_deref(), Some("A1:A3")); + check_cf(&mut app); + grid_frame(&mut app); + assert!(Arc::ptr_eq(&kept(&app).0, &ri), "an unchanged sheet keeps its line positions"); + + // A command: the new value moves the colour scale's maximum and the used range. + app.run("cell.set", json!({"cell": "A5", "input": "30"})).unwrap(); + grid_frame(&mut app); + assert_eq!(kept(&app).1.map(|r| r.a1()).as_deref(), Some("A1:A5")); + check_cf(&mut app); + // Hiding a row moves the rows under it. + app.run("selection.set", json!({"range": "A2"})).unwrap(); + app.run("home.hideRows", json!({})).unwrap(); + grid_frame(&mut app); + let geo = app.grid.geo.clone().unwrap(); + let sh = app.session.active().unwrap().wb.sheet(0).unwrap().clone(); + assert_eq!(geo.y(&sh, 2), geo.y(&sh, 1), "row 2 is hidden"); + assert!(!Arc::ptr_eq(&kept(&app).0, &ri)); + // Undo brings the row back. + app.run("edit.undo", json!({})).unwrap(); + grid_frame(&mut app); + let geo = app.grid.geo.clone().unwrap(); + let sh = app.session.active().unwrap().wb.sheet(0).unwrap().clone(); + assert!(geo.y(&sh, 2) > geo.y(&sh, 1), "row 2 is shown again"); + let fresh = super::geo::SheetLines::of(&sh); + for row in [0, 1, 2, 3, 100, 1_048_575] { + assert_eq!(kept(&app).0.start(row), fresh.ri.start(row), "row {row}"); + } + assert_eq!(kept(&app).1, fresh.used); + + // A change with nothing else holding the sheet (no undo step, no saved copy): the grid's weak + // reference makes `Arc::make_mut` move the sheet to a new allocation, and the grid notices. + let ri = kept(&app).0; + { + let d = app.session.active_mut().unwrap(); + d.undo.clear(); + d.redo.clear(); + d.saved = Arc::new(gridcraft_engine::model::Workbook::default()); + let sheet = Arc::make_mut(&mut d.wb).sheet_mut(0).unwrap(); + sheet.rows.entry(0).or_default().hidden = true; + sheet.rows.entry(1).or_default().hidden = true; + sheet.set_value(CellRef::new(2, 0), gridcraft_engine::core::Value::Number(300.0)); + } + grid_frame(&mut app); + let geo = app.grid.geo.clone().unwrap(); + let sh = app.session.active().unwrap().wb.sheet(0).unwrap().clone(); + assert_eq!(geo.y(&sh, 2), geo.y(&sh, 0), "rows 1 and 2 are hidden"); + assert!(!Arc::ptr_eq(&kept(&app).0, &ri)); + check_cf(&mut app); +} From 517a4b92567bda33fa66cb0f7bd74e6f17328399 Mon Sep 17 00:00:00 2001 From: Mouhand Kaddo Date: Mon, 9 Nov 2026 18:49:26 +0400 Subject: [PATCH 4/8] Sort: order Arabic text by its letters, not its vowel marks or digit script (#1940) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * M12.AR7: sort Arabic text by its letters, not its vowel marks or digit script * M12.AR7: order marked Arabic text the way Excel does Checked compare_text against Excel 16.0 build 20527 (en-US) via COM: Range.Sort, =SORT() and the < / = operators over ~300 strings. Fixes the mismatches inside this change's scope: - The tie-break compared raw code points, so an RLM-prefixed word sorted after every vowelled form of it, "‏12" after "١٢", and a tatweel form after a shadda form. Ignorable characters are now skipped at the tie-break too. - Marks are compared letter by letter (an unmarked letter first), so a mark on an earlier letter outranks the digit or letter after it, as in Excel ("ب١" < "بَ1", "بپ" < "بَپ"). - Marks rank in Excel's measured order (sukun before shadda), and the Quranic marks Excel skips (U+0610–061A, U+06D8–06ED except 06DD/06DE/ 06DF/06E0/06E5/06E6/06E9) are skipped by letter order. - Excel also ignores bidi embeddings/overrides/isolates, BOM, soft hyphen, CGJ and word joiner; they now are ignored too (ZWSP is not, as in Excel). Equality is unchanged. Regression tests use the orders Excel produced. Co-Authored-By: Claude Opus 5.5 * M12.AR7: compare the mark level of Arabic text without allocating --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- crates/core/src/value.rs | 268 ++++++++++++++++++++++++++- crates/engine/src/cmd/data/filter.rs | 6 +- crates/engine/src/cmd/data/tests.rs | 11 ++ crates/engine/src/pivot/items.rs | 2 +- crates/engine/src/tests.rs | 22 +++ docs/localization-parity.md | 3 +- 6 files changed, 299 insertions(+), 13 deletions(-) diff --git a/crates/core/src/value.rs b/crates/core/src/value.rs index 0000028e9..a0b4fcad6 100644 --- a/crates/core/src/value.rs +++ b/crates/core/src/value.rs @@ -346,19 +346,91 @@ pub fn compare_numbers(a: f64, b: f64) -> Ordering { round15(a).partial_cmp(&round15(b)).unwrap_or(Ordering::Equal) } -/// Case-insensitive text order (Unicode lowercase). +/// Case-insensitive text order in three levels, as Excel's collation sorts: first by letters and +/// digits alone (Arabic marks, tatweel and invisible format characters skipped, Arabic-Indic digits +/// read as digits); then, letter by letter, by the marks on each letter (none first, then in +/// Excel's mark order) and the digit script (ASCII, Arabic-Indic, Extended Arabic-Indic); then, to +/// break a remaining tie, by every character (Unicode lowercase). Only text that is equal at the +/// last level compares equal. pub fn compare_text(a: &str, b: &str) -> Ordering { - let mut ai = a.chars().flat_map(char::to_lowercase); - let mut bi = b.chars().flat_map(char::to_lowercase); - loop { - match (ai.next(), bi.next()) { - (None, None) => return Ordering::Equal, - (None, Some(_)) => return Ordering::Less, - (Some(_), None) => return Ordering::Greater, - (Some(x), Some(y)) if x != y => return x.cmp(&y), - _ => {} + if !(a.is_ascii() && b.is_ascii()) { + let primary = primary_chars(a).cmp(primary_chars(b)); + if primary != Ordering::Equal { + return primary; + } + let secondary = secondary_weights(a).cmp(secondary_weights(b)); + if secondary != Ordering::Equal { + return secondary; } } + lowercase_chars(a).cmp(lowercase_chars(b)) +} + +fn lowercase_chars(s: &str) -> impl Iterator + '_ { + s.chars().flat_map(char::to_lowercase) +} + +/// Characters Excel's text order skips entirely: tatweel and invisible format characters (soft +/// hyphen, combining grapheme joiner, zero-width (non-)joiner, bidi marks, embeddings, overrides +/// and isolates, word joiner, byte order mark). The zero-width space is not one. +fn ignorable(c: char) -> bool { + matches!( + c, + '\u{00AD}' + | '\u{034F}' + | '\u{061C}' + | '\u{0640}' + | '\u{200C}'..='\u{200F}' + | '\u{202A}'..='\u{202E}' + | '\u{2060}' + | '\u{2066}'..='\u{2069}' + | '\u{FEFF}' + ) +} + +/// Arabic combining marks in the order Excel sorts a letter carrying them; an unmarked letter +/// sorts before all of them. Measured in Excel 16.0 (build 20527, en-US) with one mark on a letter. +/// U+06D6 and U+06D7 sort as letters in Excel, so they are not listed. +const ARABIC_MARK_ORDER: [char; 48] = [ + '\u{06EC}', '\u{0611}', '\u{0612}', '\u{0613}', '\u{0614}', '\u{06DB}', '\u{0656}', '\u{0657}', '\u{0610}', '\u{06EB}', '\u{0615}', '\u{06D9}', + '\u{06E4}', '\u{06E1}', '\u{06DA}', '\u{06DC}', '\u{06EA}', '\u{06E3}', '\u{06D8}', '\u{0655}', '\u{065F}', '\u{0654}', '\u{06E7}', '\u{0653}', + '\u{06E8}', '\u{06ED}', '\u{06E2}', '\u{0658}', '\u{064B}', '\u{064C}', '\u{064D}', '\u{064E}', '\u{064F}', '\u{0650}', '\u{0652}', '\u{0651}', + '\u{0670}', '\u{0659}', '\u{065A}', '\u{065B}', '\u{065C}', '\u{065D}', '\u{065E}', '\u{0616}', '\u{0617}', '\u{0618}', '\u{0619}', '\u{061A}', +]; + +/// Rank of an Arabic mark that letter order skips (its index in `ARABIC_MARK_ORDER`). +fn arabic_mark_rank(c: char) -> Option { + ARABIC_MARK_ORDER.iter().position(|&m| m == c).and_then(|i| u16::try_from(i).ok()) +} + +/// Digit script of `c` (0 ASCII, 1 Arabic-Indic, 2 Extended Arabic-Indic) and its ASCII digit. +fn digit(c: char) -> Option<(u16, char)> { + let (script, zero) = match c { + '0'..='9' => return Some((0, c)), + '\u{0660}'..='\u{0669}' => (1, 0x0660), + '\u{06F0}'..='\u{06F9}' => (2, 0x06F0), + _ => return None, + }; + char::from_u32(u32::from(c) - zero + u32::from('0')).map(|d| (script, d)) +} + +/// The characters that decide text order before any tie-break. +fn primary_chars(s: &str) -> impl Iterator + '_ { + lowercase_chars(s).filter_map(|c| if ignorable(c) || arabic_mark_rank(c).is_some() { None } else { Some(digit(c).map_or(c, |(_, d)| d)) }) +} + +/// The second level, for text whose primary characters are equal: per letter, its digit script and +/// then its marks, the letters' lists closed by 0 so that an unmarked letter sorts first. Marks +/// before the first letter form a list of their own. +fn secondary_weights(s: &str) -> impl Iterator + '_ { + lowercase_chars(s) + .filter(|&c| !ignorable(c)) + .flat_map(|c| match arabic_mark_rank(c) { + Some(rank) => [Some(rank.saturating_add(1)), None], + None => [Some(0), Some(digit(c).map_or(0, |(script, _)| script))], + }) + .flatten() + .chain(std::iter::once(0)) } /// Sorting order used by Sort (ascending): numbers, text, booleans, errors, then blanks last. @@ -457,6 +529,182 @@ mod tests { assert_eq!(sort_compare(&Value::Empty, &Value::Number(1.0)), Ordering::Greater); } + #[test] + fn arabic_diacritics_sort_with_their_base_letter() { + assert_eq!(compare_text("بَاب", "بيت"), Ordering::Less); + assert_eq!(compare_text("بيت", "بَاب"), Ordering::Greater); + assert_eq!(compare_text("كِتاب", "كتب"), Ordering::Less); + assert_eq!(compare_text("رحمٰن", "رحمي"), Ordering::Less); + assert_eq!(compare(&Value::from("بَاب"), &Value::from("بيت")), Ordering::Less); + } + + #[test] + fn arabic_indic_digits_sort_with_digits() { + assert_eq!(compare_text("١٢", "ب"), Ordering::Less); + assert_eq!(compare_text("۱۲", "ب"), Ordering::Less); + assert_eq!(compare_text("١", "a"), Ordering::Less); + assert_eq!(compare_text("١٢", "13"), Ordering::Less); + assert_eq!(compare_text("۱۴", "13"), Ordering::Greater); + } + + #[test] + fn tatweel_and_bidi_marks_do_not_move_sort_order() { + assert_eq!(compare_text("بـاب", "بت"), Ordering::Less); + assert_eq!(compare_text("\u{200F}apple", "banana"), Ordering::Less); + assert_eq!(compare_text("\u{200E}zebra", "apple"), Ordering::Greater); + assert_eq!(compare_text("\u{061C}a", "b"), Ordering::Less); + assert_eq!(compare_text("a\u{200C}b", "ac"), Ordering::Less); + assert_eq!(compare_text("a\u{200D}b", "ac"), Ordering::Less); + } + + #[test] + fn primary_ties_keep_equality_unchanged() { + assert_eq!(compare_text("ABC", "abc"), Ordering::Equal); + assert_eq!(compare_text("باب", "بَاب"), Ordering::Less); + assert_eq!(compare_text("12", "١٢"), Ordering::Less); + assert_eq!(compare_text("a", "\u{200F}a"), Ordering::Less); + let samples = [ + "", + "abc", + "ABC", + "abd", + "ab", + "باب", + "بَاب", + "بِاب", + "بـاب", + "بيت", + "١٢", + "۱۲", + "12", + "\u{200F}12", + "\u{200F}a", + "a", + "A\u{200C}", + "a\u{200F}", + "b", + "Straße", + "STRASSE", + "ΣΑΣ", + "σας", + "İ", + "i̇", + ]; + let lowered = |s: &str| s.chars().flat_map(char::to_lowercase).collect::(); + for a in samples { + for b in samples { + let order = compare_text(a, b); + assert_eq!(order.is_eq(), lowered(a) == lowered(b), "{a:?} vs {b:?}"); + assert_eq!(order, compare_text(b, a).reverse(), "{a:?} vs {b:?}"); + for c in samples { + if order.is_le() && compare_text(b, c).is_le() { + assert!(compare_text(a, c).is_le(), "{a:?} <= {b:?} <= {c:?}"); + } + } + } + } + } + + /// Sorts `expected` from reversed and rotated starts and checks each comes back as `expected`. + #[track_caller] + fn assert_sorts_to(expected: &[&str]) { + let mut reversed: Vec<&str> = expected.iter().rev().copied().collect(); + reversed.sort_by(|a, b| compare_text(a, b)); + assert_eq!(reversed, expected); + let mut rotated: Vec<&str> = expected.to_vec(); + rotated.rotate_left(expected.len() / 2); + rotated.sort_by(|a, b| compare_text(a, b)); + assert_eq!(rotated, expected); + } + + // The expected orders below are Excel's, observed with Range.Sort, =SORT() and the `<` + // operator in Excel 16.0 (build 20527, en-US). Strings Excel ranks equal are left out. + + #[test] + fn excel_orders_a_letter_by_its_mark() { + assert_sorts_to(&[ + "باب", + "ب\u{0611}اب", + "ب\u{0610}اب", + "ب\u{0654}اب", + "ب\u{064B}اب", + "ب\u{064C}اب", + "ب\u{064D}اب", + "ب\u{064E}اب", + "ب\u{064F}اب", + "ب\u{0650}اب", + "ب\u{0652}اب", + "ب\u{0651}اب", + "ب\u{0670}اب", + "ب\u{065E}اب", + "ب\u{061A}اب", + ]); + // Quranic marks are skipped by letter order like harakat (hamza sorts before alef). + assert_eq!(compare_text("ب\u{0610}ء", "باب"), Ordering::Less); + assert_eq!(compare_text("ب\u{06E1}ء", "باب"), Ordering::Less); + } + + #[test] + fn excel_compares_marks_letter_by_letter() { + assert_sorts_to(&[ + "بت", + "بت\u{064E}", + "بت\u{064F}", + "بت\u{0650}", + "ب\u{064E}ت", + "ب\u{064E}ت\u{064E}", + "ب\u{064E}ت\u{064F}", + "ب\u{064F}ت", + "ب\u{064F}ت\u{064E}", + "ب\u{0650}ت\u{0650}", + ]); + assert_sorts_to(&["مـحـمـد", "محم\u{0651}د", "م\u{064E}حمد", "م\u{064F}ح\u{064E}م\u{064E}\u{0651}د", "محمدي"]); + } + + #[test] + fn excel_weighs_marks_and_digit_scripts_by_position() { + // A mark on an earlier letter outranks the digit script or letter after it, whatever the + // code point of that next character. + assert_sorts_to(&[ + "ب", + "ب\u{064E}", + "ب1", + "ب١", + "ب۱", + "ب\u{064E}1", + "ب\u{064E}١", + "ب\u{064E}۱", + "بa", + "ب\u{064E}a", + "با", + "ب\u{064E}ا", + "ب\u{067E}", + "ب\u{064E}\u{067E}", + "ب\u{06CC}", + "ب\u{064E}\u{06CC}", + ]); + // And an earlier digit script outranks a later mark. + assert_sorts_to(&["12", "1٢", "١2", "١٢", "1ب", "1ب\u{064E}", "1ب\u{0650}", "١ب", "١ب\u{064E}", "١ب\u{0650}", "۱ب", "۱ب\u{064E}"]); + assert_sorts_to(&["ب1ب", "ب1ب\u{064E}", "ب١ب", "ب\u{064E}ب", "بب1", "بب١", "ب\u{064E}ب1"]); + } + + #[test] + fn excel_skips_invisible_format_characters() { + for c in ['\u{00AD}', '\u{034F}', '\u{061C}', '\u{200C}', '\u{200D}', '\u{200E}', '\u{200F}', '\u{202A}', '\u{202B}'] + .into_iter() + .chain(['\u{202C}', '\u{202D}', '\u{202E}', '\u{2060}', '\u{2066}', '\u{2067}', '\u{2068}', '\u{2069}', '\u{FEFF}']) + { + let s = format!("{c}b"); + assert_eq!(compare_text(&s, "a"), Ordering::Greater, "{s:?}"); + assert_eq!(compare_text(&s, "ba"), Ordering::Less, "{s:?}"); + assert_eq!(compare_text(&format!("ب{c}\u{064E}اب"), "باب\u{064B}"), Ordering::Greater, "{c:?}"); + } + // Not even as a tie-break: a leading mark doesn't push text past marked or other-script forms. + assert_sorts_to(&["\u{200F}باب", "ب\u{064B}اب", "ب\u{064E}اب", "ب\u{0670}اب"]); + assert_sorts_to(&["\u{200F}12", "١٢", "۱۲"]); + assert_sorts_to(&["ب\u{200F}ب\u{064E}", "\u{200F}ب\u{064E}ب"]); + } + #[test] fn errors() { for e in CellError::ALL { diff --git a/crates/engine/src/cmd/data/filter.rs b/crates/engine/src/cmd/data/filter.rs index 437bd7887..cda2160bf 100644 --- a/crates/engine/src/cmd/data/filter.rs +++ b/crates/engine/src/cmd/data/filter.rs @@ -317,5 +317,9 @@ pub fn filter_values(wb: &Workbook, sheet: usize, col: u32) -> Vec<(String, bool let key = if blank { sort_key(&Value::Empty) } else { sort_key(&v) }; seen.entry((key, t)).or_insert(on); } - seen.into_iter().map(|((_, t), on)| (t, on)).collect() + let mut values: Vec<_> = seen.into_iter().collect(); + values.sort_by(|((a, ta), _), ((b, tb), _)| { + a.0.cmp(&b.0).then(a.1.cmp(&b.1)).then_with(|| gridcraft_core::compare_text(&a.2, &b.2)).then_with(|| ta.cmp(tb)) + }); + values.into_iter().map(|((_, t), on)| (t, on)).collect() } diff --git a/crates/engine/src/cmd/data/tests.rs b/crates/engine/src/cmd/data/tests.rs index 3c2782444..e073e23a5 100644 --- a/crates/engine/src/cmd/data/tests.rs +++ b/crates/engine/src/cmd/data/tests.rs @@ -53,6 +53,17 @@ fn subtotal_puts_summary_rows_below_their_detail() { assert_eq!(sh.value(CellRef::new(3, 0)).display(), "West"); } +#[test] +fn filter_list_orders_arabic_like_sort() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["H"], ["بج"], ["بُت"], ["\u{200F}بب"]]})).unwrap(); + s.execute("data.filter", json!({"range": "A1:A4"})).unwrap(); + let doc = s.doc().unwrap(); + let list: Vec = filter_values(&doc.wb, 0, 0).into_iter().map(|(t, _)| t).collect(); + assert_eq!(list, ["\u{200F}بب", "بُت", "بج"]); +} + #[test] fn filter_by_huge_numeric_column_is_rejected_not_overflowed() { let mut s = Session::new(); diff --git a/crates/engine/src/pivot/items.rs b/crates/engine/src/pivot/items.rs index 79b74da1a..6dee1dc5a 100644 --- a/crates/engine/src/pivot/items.rs +++ b/crates/engine/src/pivot/items.rs @@ -21,7 +21,7 @@ pub(super) struct FieldItems { } fn cmp_items(a: &Item, b: &Item) -> Ordering { - a.class.cmp(&b.class).then(a.num.total_cmp(&b.num)).then_with(|| a.text.cmp(&b.text)) + a.class.cmp(&b.class).then(a.num.total_cmp(&b.num)).then_with(|| gridcraft_core::compare_text(&a.text, &b.text)) } /// (identity key, item) for a source value under a field's date grouping. diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f97e1bc10..bf36bd15b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1459,6 +1459,28 @@ fn case_sensitive_sort_puts_lowercase_first() { assert_eq!(col, ["a", "A", "aB", "Ab", "b", "B"].map(Value::from)); } +#[test] +fn data_sort_orders_vowelled_arabic_by_letters() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["بيت"], ["بَاب"], ["تمر"], ["\u{200F}بحر"]]})).unwrap(); + s.execute("data.sort", json!({"range": "A1:A4", "header": false, "keys": [{"column": "A"}]})).unwrap(); + let col: Vec = (1..=4).map(|r| v(&s, &format!("A{r}"))).collect(); + assert_eq!(col, ["بَاب", "\u{200F}بحر", "بيت", "تمر"].map(Value::from)); +} + +#[test] +fn data_sort_orders_marks_and_digit_scripts_like_excel() { + // Excel 16.0 (build 20527, en-US) sorts this column, as text, into exactly this order. + let want = ["١٢", "۱۲", "ب\u{064E}1", "ب\u{064E}١", "بa", "\u{200F}باب", "ب\u{064B}اب", "ب\u{064E}اب", "ب\u{0652}اب", "ب\u{0651}اب"]; + let mut s = s(); + s.execute("home.numberFormat", json!({"range": "A1:A10", "code": "@"})).unwrap(); + let input: Vec> = want.iter().rev().map(|t| vec![*t]).collect(); + s.execute("range.setValues", json!({"range": "A1", "values": input})).unwrap(); + s.execute("data.sort", json!({"range": "A1:A10", "header": false, "keys": [{"column": "A"}]})).unwrap(); + let col: Vec = (1..=10).map(|r| v(&s, &format!("A{r}"))).collect(); + assert_eq!(col, want.map(Value::from)); +} + #[test] fn sort_with_a_sheet_prefix_sorts_the_named_sheet() { let col = |s: &Session, sheet: &str| -> Vec { diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 39a9b3f91..7b5dda2c0 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-23 · **Change:** minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-23 · **Change:** minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **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, @@ -117,6 +117,7 @@ language (none so far) and of localized function names. | 2026-10-20 | minor | Dutch (nl) interface catalog (`i18n/nl.rs`, 518 labels, ~24%): the shared key set every language must cover (ribbon, context menus, File view, dialog titles, Options page), matching the German, French and Czech catalogs; `nl-NL`, `nl-BE` locale tags resolve to Dutch | | 2026-10-20 | minor | XLSX: a theme's per-script typefaces (``, Hebr, Jpan…) are kept through open + save, so Arabic Excel's theme fonts for shapes, charts and theme-font cells survive a GridCraft save | | 2026-10-20 | trivial | XLSX: a workbook theme's major/minor East Asian (``) and complex-script (``) typefaces are kept through open + save, so a Japanese workbook keeps its theme's East Asian font; rendering does not resolve them yet (follow-up) | +| 2026-10-19 | minor | Text order (Sort, SORT/SORTBY, `<`/`>`, approximate MATCH/XLOOKUP, `" Date: Mon, 9 Nov 2026 18:52:55 +0400 Subject: [PATCH 5/8] AutoFit: estimate Arabic text width from measured glyph widths (#1933) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * M12.AR12: estimate Arabic text width from measured glyph widths in AutoFit * M12.AR12: treat U+0897–089F as marks and U+08E2 as visible; size the honorific ligatures (review) * M12.AR12: size the remaining honorific ligatures like their siblings (review) U+FD4C-FD4F (Unicode 14) and U+FDC8-FDCF (FDCF Unicode 14, FDC8-FDCE Unicode 17) are honorific word ligatures, not letters, but fell into the 0.48 em letter arm. Give them the 1.6 em of the measured U+FD40-FD4B honorifics and assert each range's endpoints. Co-Authored-By: Claude Opus 5.5 * M12.AR12: size Arabic-Indic digits like ASCII digits and the remaining honorific ligatures (review) Arabic-Indic (U+0660-0669) and Extended Arabic-Indic (U+06F0-06F9) digits measure 0.53-0.57 em in Noto Sans Arabic, Arial, Tahoma and Times New Roman, the same as each font's ASCII digits, so they get the ASCII digit estimate (0.56) instead of the letter estimate. Arabic punctuation measures 0.17-0.50 em, so it keeps 0.48 rather than falling back to 1 em. U+FBC3-FBD2 and U+FD90-FD91 are honorific word ligatures like U+FD40-FD4F and get the same 1.6 em. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: FredApps --- crates/engine/src/cmd/format.rs | 46 +++++++++++++++++++++++++++ crates/engine/src/cmd/format/sizes.rs | 45 +++++++++++++++++++++++++- docs/localization-parity.md | 3 +- 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 1066f48e2..d5eac5b5d 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -870,6 +870,52 @@ mod tests { assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["style"]["num_fmt"], "0.00"); } + #[test] + fn arabic_vowel_and_direction_marks_add_no_width() { + let plain = approx_text_width("محمد يدرس في المدرسة", 11.0, false); + assert_eq!(approx_text_width("مُحَمَّد يَدْرُسُ فِي الْمَدْرَسَةِ", 11.0, false), plain); + assert_eq!(approx_text_width("\u{200F}محمد يدرس في المدرسة\u{200C}", 11.0, false), plain); + assert_eq!(approx_text_width("ب\u{0897}\u{0898}\u{089F}", 11.0, false), approx_text_width("ب", 11.0, false)); + assert!(approx_text_width("\u{08E2}", 11.0, false) > 0.0, "the disputed end of ayah is a visible sign"); + } + + #[test] + fn arabic_text_fits_a_column_about_as_wide_as_latin_text_of_the_same_length() { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["مُحَمَّد يَدْرُسُ فِي الْمَدْرَسَةِ", "Hello there in school"]]})).unwrap(); + s.execute("home.autofitColumnWidth", json!({"cols": "A:B"})).unwrap(); + let doc = s.doc().unwrap(); + let width = |c: u32| doc.wb.active().and_then(|sh| sh.cols.get(&c)).and_then(|e| e.size).unwrap(); + let ratio = width(0) / width(1); + assert!((0.75..=1.15).contains(&ratio), "Arabic {} vs Latin {}", width(0), width(1)); + } + + #[test] + fn arabic_word_ligatures_are_wider_than_letters() { + let em = |t: &str| approx_text_width(t, 72.0, false) / 96.0; + assert!(em("\u{FDFD}") > 8.0); + assert!(em("\u{FDFA}") > 1.0); + let honorifics = ['\u{FD40}', '\u{FD4B}', '\u{FD4C}', '\u{FD4F}', '\u{FDC8}', '\u{FDCF}', '\u{FDFE}', '\u{FDFF}']; + for honorific in honorifics.into_iter().chain(['\u{FBC3}', '\u{FBD2}', '\u{FD90}', '\u{FD91}']) { + assert!(em(&honorific.to_string()) > 1.0, "U+{:04X}", u32::from(honorific)); + } + assert!(em("ب") < 0.6); + } + + #[test] + fn arabic_digits_are_as_wide_as_ascii_digits_and_punctuation_as_a_letter() { + let width = |t: &str| approx_text_width(t, 11.0, false); + assert_eq!(width("١٢٣"), width("123")); + assert_eq!(width("۴۵۶"), width("456")); + assert_eq!(width("،؟"), width("بب")); + } + + #[test] + fn cjk_text_keeps_a_full_em_per_character() { + assert_eq!(approx_text_width("中文", 12.0, false), 2.0 * 12.0 * 96.0 / 72.0); + } + #[test] fn width_units() { assert_eq!(chars_to_points(8.43), 64.0); diff --git a/crates/engine/src/cmd/format/sizes.rs b/crates/engine/src/cmd/format/sizes.rs index c79327678..8b304c889 100644 --- a/crates/engine/src/cmd/format/sizes.rs +++ b/crates/engine/src/cmd/format/sizes.rs @@ -136,12 +136,55 @@ pub fn approx_text_width(text: &str, size: f32, bold: bool) -> f32 { c if c.is_ascii_uppercase() => 0.65, c if c.is_ascii_digit() => 0.56, c if c.is_ascii() => 0.52, - _ => 1.0, + c if is_zero_width(c) => 0.0, + c => arabic_em(c).unwrap_or(1.0), }; } w * size * if bold { 1.07 } else { 1.0 } * 96.0 / 72.0 } +/// Arabic vowel and Quranic marks, which sit on the previous letter, and invisible format +/// characters such as direction marks and zero-width joiners. +fn is_zero_width(c: char) -> bool { + matches!( + c, + '\u{0610}'..='\u{061A}' + | '\u{061C}' + | '\u{064B}'..='\u{065F}' + | '\u{0670}' + | '\u{06D6}'..='\u{06DC}' + | '\u{06DF}'..='\u{06E4}' + | '\u{06E7}'..='\u{06E8}' + | '\u{06EA}'..='\u{06ED}' + | '\u{0897}'..='\u{089F}' + | '\u{08CA}'..='\u{08E1}' + | '\u{08E3}'..='\u{08FF}' + | '\u{200B}'..='\u{200F}' + | '\u{202A}'..='\u{202E}' + | '\u{2060}'..='\u{2069}' + | '\u{FE20}'..='\u{FE2F}' + | '\u{FEFF}' + ) +} + +/// Width in ems of an Arabic-script character, measured from shaped Noto Sans Arabic, Noto Naskh +/// Arabic and DejaVu Sans: joined letters average about half an em, word ligatures such as ﷺ and the +/// rial sign about 1.2 em, the honorific ligatures (U+FD40–FD4B) about 1.6 em, and the bismillah +/// ligature about 11 em. The later honorific ligatures (U+FBC3–FBD2, U+FD4C–FD4F, U+FD90–FD91, +/// U+FDC8–FDCF) are not measured and get the 1.6 em of their siblings rather than the letter width. +/// Arabic-Indic digits are as wide as ASCII digits (0.53–0.57 em in Noto Sans Arabic, Arial, Tahoma +/// and Times New Roman); Arabic punctuation (0.2–0.5 em in the same fonts) keeps the letter width. +fn arabic_em(c: char) -> Option { + match c { + '\u{FDFD}' => Some(11.0), + '\u{FBC3}'..='\u{FBD2}' | '\u{FD40}'..='\u{FD4F}' | '\u{FD90}'..='\u{FD91}' | '\u{FDC8}'..='\u{FDCF}' => Some(1.6), + '\u{FDF0}'..='\u{FDFC}' | '\u{FDFE}'..='\u{FDFF}' => Some(1.2), + '\u{0660}'..='\u{0669}' | '\u{06F0}'..='\u{06F9}' => Some(0.56), + '\u{0600}'..='\u{06FF}' | '\u{0750}'..='\u{077F}' | '\u{0870}'..='\u{08FF}' | '\u{FB50}'..='\u{FDFF}' | '\u{FE70}'..='\u{FEFC}' => Some(0.48), + _ => None, + } +} + /// Distinct texts AutoFit measures exactly; past this many, `Session::text_width` is asked for a /// quick measure so a huge column doesn't lay out every value. const EXACT_TEXT_WIDTHS: usize = 2_000; diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 7b5dda2c0..3f1dacb96 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-23 · **Change:** minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (AutoFit estimates Arabic text width from measured glyph widths); previously minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **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, @@ -110,6 +110,7 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | AutoFit column width, wrapped-row height and PivotTable column fit estimate Arabic letters at ~0.48 em (measured 0.36–0.53 em shaped, Noto Sans/Naskh Arabic and DejaVu Sans) instead of 1 em, Arabic-Indic digits as wide as ASCII digits, and give vowel marks and direction marks no width; vowelled Arabic no longer fits ~3.5× too wide | | 2026-10-23 | minor | Missing translations are reported, never a failure: `cargo xtask i18n-report` prints, per language, the strings other catalogs translate and it lacks (plus Spanish function help rows keyed by English the function catalog no longer has) and always passes; `cargo xtask ci` runs it first. Coverage and per-language parity tests and tests of one language's exact wording are gone; UI tests take the expected text from the catalog. Duplicate-key, placeholder, context-prefix and line-shape checks stay | | 2026-10-21 | minor | Romanian interface (`ro`, Română): 970 labels covering every key of the other catalogs (`crates/ui-egui/src/i18n/ro.rs`), `ro-*` system locales, the View-tab switcher and `app.language.set`; context keys show no prefix (File › Options test). English formulas and formats; AI-assisted draft pending native-speaker review | | 2026-10-20 | minor | Japanese: furigana (`rPh` phonetic runs and `` in `xl/sharedStrings.xml`) is no longer deleted by an open + save; it is kept on the workbook and written back (a `` without runs is kept too, while duplicate texts with different readings keep only the first, reported on open). Display of the readings is still missing (`crates/xlsx/src/tests/phonetic_tests.rs`) | From c35234e5f555201cf39669c7164a1fc9f5267cf8 Mon Sep 17 00:00:00 2001 From: Mouhand Kaddo Date: Mon, 9 Nov 2026 18:54:05 +0400 Subject: [PATCH 6/8] Number formats: honour the B1/B2 calendar prefixes (Hijri dates for B2) (#1918) * M12.AR6: honour the B1/B2 calendar prefixes in number formats (Hijri for B2) * M12.AR6: match Excel's B1/B2 behaviour measured in review Changes from a reviewer's Excel run (TEXT and Range.NumberFormat): - A [$-...] locale tag before B2 keeps the date Gregorian (prefix still hidden); B2[$-401] stays Hijri. - Any other unquoted B and digit (B3dd, dd/mm/yyyyB2, dd B2 mm), and B2 before a number format (B20.00), make the code invalid: TEXT gives #VALUE!, a cell shows General, as for a code over 255 characters. - Under B2, ddd and dddd show the Arabic weekday name (full name for both). - Serial 60 has no Hijri date: TEXT gives #VALUE!, a cell shows ####. - Serial 0 shows as serial 1 (29/08/1317). Also fixes a break from merging main: days_from_civil now returns 0 for day 0 and 29 February 1900, which sent serials 0 and 60 to 1970 (23/10/1389) under B2. The Hijri day count no longer passes those dates. Docs: localization-parity states the behaviour, including the one measured case not matched (mmmm next to dddd prints the weekday in Excel). Co-Authored-By: Claude Opus 5.5 * M12.AR6: Hijri leap year in cycle year 15, not 16, as Excel shows A reviewer compared Excel's B2yyyy-mm-dd on 43,512 serials with this formula: it disagreed by one day on 1,372, all in cycle years 15 and 16, and moving the leap year from 16 to 15 left no disagreement. The leap years are now 2, 5, 7, 10, 13, 15, 18, 21, 24, 26 and 29 (still 10,631 days a cycle). The count of leap years before a year becomes (4 + 11y) div 30; it differs from the old (3 + 11y) div 30 only when y = 16 (mod 30), which is exactly the move from 16 to 15. The inverse's year estimate can now be one year off at a boundary and is corrected by one step. Tests: Excel values for 2 March 2005 (1426-01-21) and 29 January 2006 (1426-12-29) plus the 1425/1426 year boundaries they imply, the reviewer's other anchors, and exhaustive checks: year, month and 30-year cycle lengths for Hijri years 1 to 10,000; day -> Hijri -> day round trip and day-by-day succession for every day from 1 Muharram 1 AH to one cycle past 31 December 9999; every 1900-system serial 1 to 2,958,465 (except 60). Co-Authored-By: Claude Opus 5.5 * M12.AR6: match B1/B2 details checked in Excel 16.0 build 20527 Compared GridCraft with Excel 16.0 (build 20527, en-US) through COM: TEXT with B2dd/mm/yyyy on every serial from 1 to 2,958,465 (no difference), and about 1,750 format/value pairs through both a cell's NumberFormat and TEXT. Fixed what differed and is in scope: - Two-word Hijri month names use a no-break space (U+00A0), as Excel does. - B1 before a number format is rejected like B2 (B10.00, B1#,##0). - Prefix runs: a repeated prefix is accepted (B2B2, B2b2, [Red]B2B2, B1B1); a mixed one (B1B2, B2B1), more digits (B12, B21, B02), a prefix after a tag or space (B2[Red]B2, B2 B2) and an escaped \B2 are rejected. - Serial 60 under B2 is #VALUE! for any date or clock part (hh:mm, ss, AM/PM), not only date parts; [h] and literal-only sections still show. Regression tests use the Excel values (all 12 month and 7 weekday names, the accepted and rejected codes, serial 60). Not changed, listed in the docs: Excel's weekday/month-name mix-ups when ddd(d) and mmm(m) share a B2 section, weekday-only B2 sections (real weekday for serials 0-59, #VALUE! for the last days of 9999), and Buddhist-year b/bb/bbbb tokens. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: FredApps --- crates/numfmt/src/calendar.rs | 206 ++++++++++++++++++++++++++++++ crates/numfmt/src/lib.rs | 22 +++- crates/numfmt/src/parse.rs | 69 +++++++++- crates/numfmt/src/render.rs | 34 +++-- crates/numfmt/src/tests.rs | 234 ++++++++++++++++++++++++++++++++++ docs/localization-parity.md | 19 ++- 6 files changed, 563 insertions(+), 21 deletions(-) create mode 100644 crates/numfmt/src/calendar.rs diff --git a/crates/numfmt/src/calendar.rs b/crates/numfmt/src/calendar.rs new file mode 100644 index 000000000..ab5bd4bbd --- /dev/null +++ b/crates/numfmt/src/calendar.rs @@ -0,0 +1,206 @@ +//! Calendars a format section can show dates in: the `B1` (Gregorian) and `B2` (Hijri) prefixes. + +use gridcraft_core::date::{DateTime, MONTHS, WEEKDAYS, days_from_civil, days_in_month}; + +/// The calendar a section's year, month and day parts count in. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub(crate) enum Calendar { + #[default] + Gregorian, + /// The tabular (arithmetic) Islamic calendar. + Hijri, +} + +/// Arabic names of the Hijri months, Muharram first. Two-word names are joined by a no-break +/// space (U+00A0), as Excel writes them. +const HIJRI_MONTHS: [&str; 12] = [ + "محرم", + "صفر", + "ربيع\u{a0}الأول", + "ربيع\u{a0}الثاني", + "جمادى\u{a0}الأولى", + "جمادى\u{a0}الثانية", + "رجب", + "شعبان", + "رمضان", + "شوال", + "ذو\u{a0}القعدة", + "ذو\u{a0}الحجة", +]; + +/// Arabic weekday names, Sunday first, shown for `ddd` and `dddd` under `B2`. +const ARABIC_WEEKDAYS: [&str; 7] = ["الأحد", "الإثنين", "الثلاثاء", "الأربعاء", "الخميس", "الجمعة", "السبت"]; + +/// Year, month and day of a date in a section's calendar, with the month and weekday names. +pub(crate) struct CalendarDate { + pub year: i64, + pub month: u32, + pub day: u32, + /// `mmmm`. + pub month_name: &'static str, + /// `mmm`. + pub month_short: &'static str, + /// `dddd`. + pub day_name: &'static str, + /// `ddd`. + pub day_short: &'static str, +} + +impl Calendar { + /// The date of a day in this calendar, `None` when it has none: under `B2`, the 1900 system's + /// fictitious 29 February 1900 (serial 60, which Excel shows as `#VALUE!`) and days before + /// 1 Muharram 1 AH. + pub(crate) fn date(self, dt: &DateTime) -> Option { + match self { + Calendar::Hijri => hijri_date(dt), + Calendar::Gregorian => Some(gregorian_date(dt)), + } + } +} + +fn gregorian_date(dt: &DateTime) -> CalendarDate { + let month_name = MONTHS.get((dt.month as usize).wrapping_sub(1)).copied().unwrap_or(""); + let day_name = WEEKDAYS.get(dt.weekday as usize).copied().unwrap_or(""); + CalendarDate { + year: dt.year.into(), + month: dt.month, + day: dt.day, + month_name, + month_short: month_name.get(..3).unwrap_or(month_name), + day_name, + day_short: day_name.get(..3).unwrap_or(day_name), + } +} + +/// Days from 1 Muharram 1 AH to the first of a Hijri month. Leap years (30 days in the last +/// month) are years 2, 5, 7, 10, 13, 15, 18, 21, 24, 26 and 29 of each 30-year cycle (10,631 +/// days), which is what Excel shows; odd months have 30 days and even months 29. +fn days_before_month(year: i64, month: i64) -> i64 { + // `(4 + 11 * year) / 30` is the number of leap years before `year`. + (year - 1) * 354 + (4 + 11 * year).div_euclid(30) + 29 * (month - 1) + month / 2 +} + +/// Hijri year, month and day of the day `days` after 1 Muharram 1 AH (`days >= 0`). +fn hijri_from_days(days: i64) -> Option<(i64, i64, i64)> { + if days < 0 { + return None; + } + // The estimate is the mean year length rounded; it can be one year off at a year boundary. + // Estimate and `days_before_month` both move by 30 years per 10,631 days, so one step is + // enough everywhere if it is within one cycle (`every_day_round_trips` checks many). + let mut year = (30 * days + 10646) / 10631; + if days < days_before_month(year, 1) { + year -= 1; + } else if days >= days_before_month(year + 1, 1) { + year += 1; + } + let month = ((11 * (days - days_before_month(year, 1)) + 330) / 325).clamp(1, 12); + let day = days - days_before_month(year, month) + 1; + Some((year, month, day)) +} + +/// 1 Muharram 1 AH is 18 July 622 (proleptic Gregorian), so that 1 January 2001 is 6 Shawwal 1421. +fn hijri_date(dt: &DateTime) -> Option { + // 29 February 1900 (serial 60) is no real day and has no Hijri date. The 1900 system's day 0 + // (`1900-01-00`, serial 0) shows as 1 January 1900, as Excel shows it. + if dt.day > days_in_month(dt.year, dt.month) { + return None; + } + let (year, month, day) = hijri_from_days(days_from_civil(dt.year, dt.month, dt.day.max(1)) - days_from_civil(622, 7, 18))?; + let month_name = HIJRI_MONTHS.get(usize::try_from(month - 1).ok()?).copied()?; + // Excel shows the full Arabic name for `ddd` as well as `dddd`, as it does for `mmm`. + let day_name = ARABIC_WEEKDAYS.get(dt.weekday as usize).copied()?; + Some(CalendarDate { + year, + month: u32::try_from(month).ok()?, + day: u32::try_from(day).ok()?, + month_name, + month_short: month_name, + day_name, + day_short: day_name, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use gridcraft_core::DateSystem; + use gridcraft_core::date::{datetime_from_serial, max_serial}; + + /// Leap years of the 30-year cycle (Excel's set, per the reviewer's 43,512-serial comparison). + const LEAP: [i64; 11] = [2, 5, 7, 10, 13, 15, 18, 21, 24, 26, 29]; + + fn is_leap(year: i64) -> bool { + LEAP.contains(&((year - 1).rem_euclid(30) + 1)) + } + + fn month_len(year: i64, month: i64) -> i64 { + if month == 12 { 29 + i64::from(is_leap(year)) } else { 30 - (month + 1) % 2 } + } + + /// Years 1 to 10,000 (every cycle up to and past 31 December 9999, which is 9666 AH): year, + /// month and cycle lengths follow the leap-year set. + #[test] + fn year_and_month_lengths_follow_the_leap_set() { + assert_eq!(days_before_month(1, 1), 0); + for year in 1..=10_000 { + let start = days_before_month(year, 1); + let len = days_before_month(year + 1, 1) - start; + assert_eq!(len, if is_leap(year) { 355 } else { 354 }, "year {year}"); + for month in 1..=12 { + let next = if month == 12 { days_before_month(year + 1, 1) } else { days_before_month(year, month + 1) }; + assert_eq!(next - days_before_month(year, month), month_len(year, month), "{year}-{month}"); + } + assert_eq!(days_before_month(year + 30, 1) - start, 10_631, "cycle from {year}"); + } + // One cycle: exactly the eleven leap years, 10,631 days. + let cycle: Vec = (1..=30).filter(|&y| days_before_month(y + 1, 1) - days_before_month(y, 1) == 355).collect(); + assert_eq!(cycle, LEAP); + } + + /// Every day from 1 Muharram 1 AH to one cycle past 31 December 9999: day -> Hijri -> day + /// round-trips, the day is within its month, and consecutive days follow each other. + #[test] + fn every_day_round_trips() { + let last = days_from_civil(9999, 12, 31) - days_from_civil(622, 7, 18) + 10_631; + let mut prev: Option<(i64, i64, i64)> = None; + for days in 0..=last { + let (y, m, d) = hijri_from_days(days).unwrap(); + assert!((1..=12).contains(&m) && (1..=month_len(y, m)).contains(&d), "{days}: {y}-{m}-{d}"); + assert_eq!(days_before_month(y, m) + d - 1, days, "{y}-{m}-{d}"); + if let Some((py, pm, pd)) = prev { + let expected = if pd < month_len(py, pm) { + (py, pm, pd + 1) + } else if pm < 12 { + (py, pm + 1, 1) + } else { + (py + 1, 1, 1) + }; + assert_eq!((y, m, d), expected, "after {py}-{pm}-{pd}"); + } + prev = Some((y, m, d)); + } + assert_eq!(hijri_from_days(-1), None); + } + + /// Every serial of the 1900 system: the Hijri date names the serial's day, one day apart. + #[test] + fn every_serial_has_the_right_hijri_day() { + let epoch = days_from_civil(622, 7, 18); + let mut prev = None; + for serial in 1..=max_serial(DateSystem::D1900) { + if serial == 60 { + continue; + } + let dt = datetime_from_serial(DateSystem::D1900, serial as f64).unwrap(); + let h = Calendar::Hijri.date(&dt).unwrap(); + assert!((1..=month_len(h.year, i64::from(h.month))).contains(&i64::from(h.day)), "serial {serial}"); + let days = days_before_month(h.year, i64::from(h.month)) + i64::from(h.day) - 1; + assert_eq!(days, days_from_civil(dt.year, dt.month, dt.day) - epoch, "serial {serial}"); + if let Some(p) = prev { + assert_eq!(days, p + 1, "serial {serial}"); + } + prev = Some(days); + } + } +} diff --git a/crates/numfmt/src/lib.rs b/crates/numfmt/src/lib.rs index cbb11bc31..57384cc89 100644 --- a/crates/numfmt/src/lib.rs +++ b/crates/numfmt/src/lib.rs @@ -4,6 +4,7 @@ #![forbid(unsafe_code)] mod builtin; +mod calendar; mod decimal; mod parse; mod render; @@ -96,6 +97,9 @@ struct Inner { num_sections: usize, /// Index of the section that applies to text, if any. text_section: Option, + /// A code Excel rejects (a misplaced `B1`/`B2` calendar prefix): shown as General, and + /// `TEXT` returns `#VALUE!`, as for a code that is too long. + invalid: bool, } /// A parsed number format code. Cheap to clone. @@ -130,7 +134,8 @@ fn too_long(code: &str) -> bool { impl NumberFormat { /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text, and - /// a code longer than [`MAX_FORMAT_CODE_CHARS`] characters degrades to General. + /// a code longer than [`MAX_FORMAT_CODE_CHARS`] characters, or one Excel rejects for a + /// misplaced `B1`/`B2` calendar prefix, degrades to General. pub fn parse(code: &str) -> NumberFormat { Self::parse_inner(code, false) } @@ -147,12 +152,16 @@ impl NumberFormat { let trimmed = code.trim(); // `checkbox` marks a checkbox cell (Insert › Checkbox): TRUE/FALSE draw as a box, any other // value displays as General. + let general = |invalid: bool| NumberFormat { + inner: Arc::new(Inner { code: code.to_string(), general: true, sections: Vec::new(), num_sections: 0, text_section: None, invalid }), + }; if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("general") || trimmed == "checkbox" || (!uncapped && too_long(code)) { - return NumberFormat { - inner: Arc::new(Inner { code: code.to_string(), general: true, sections: Vec::new(), num_sections: 0, text_section: None }), - }; + return general(false); } let sections = parse::parse_sections(code); + if sections.iter().any(|s| s.invalid) { + return general(true); + } let len = sections.len(); let last_is_text = sections.last().is_some_and(|s| s.kind == SecKind::Text); let text_section = if len >= 4 { @@ -169,7 +178,7 @@ impl NumberFormat { } else { len }; - NumberFormat { inner: Arc::new(Inner { code: code.to_string(), general: false, sections, num_sections, text_section }) } + NumberFormat { inner: Arc::new(Inner { code: code.to_string(), general: false, sections, num_sections, text_section, invalid: false }) } } pub fn general() -> NumberFormat { @@ -487,6 +496,9 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result return Err(e), Value::Empty => 0.0, diff --git a/crates/numfmt/src/parse.rs b/crates/numfmt/src/parse.rs index ac905d36b..c2b121940 100644 --- a/crates/numfmt/src/parse.rs +++ b/crates/numfmt/src/parse.rs @@ -1,6 +1,7 @@ //! Format code parsing: sections, bracket tags and tokens. use crate::FormatColor; +use crate::calendar::Calendar; /// At most this many sections are honoured (`pos;neg;zero;text`); extra ones are ignored. const MAX_SECTIONS: usize = 4; @@ -24,6 +25,11 @@ pub(crate) enum DatePart { } impl DatePart { + /// `[h]`, `[m]` or `[s]`. + pub(crate) fn is_elapsed(self) -> bool { + matches!(self, DatePart::ElapsedH(_) | DatePart::ElapsedM(_) | DatePart::ElapsedS(_)) + } + pub(crate) fn is_calendar(self) -> bool { matches!(self, DatePart::Year(_) | DatePart::Month(_) | DatePart::Day(_)) } @@ -122,6 +128,10 @@ pub(crate) struct Section { pub fraction: bool, pub currency: bool, pub fill: bool, + pub calendar: Calendar, + /// Excel rejects the whole code because of this section: a `B` and digit that is not a + /// calendar prefix, or a prefix before a number format (`B20.00`). + pub invalid: bool, } /// Splits a code into sections at unquoted, unescaped, unbracketed semicolons. @@ -251,17 +261,60 @@ struct Raw { color: Option, cond: Option, currency: bool, + calendar: Calendar, + /// The section's calendar prefix as written, `'1'` or `'2'` (whether or not a locale tag + /// cancels it). + prefix: Option, + /// An unquoted `B` and digit that is not a calendar prefix: Excel rejects the code. + invalid: bool, +} + +/// Where a section's body starts: after its leading `[...]` tags, read as `tokenize` reads them. +/// Also whether one of those tags is a `[$...]` locale tag. +fn after_leading_brackets(cs: &[char]) -> (usize, bool) { + let mut i = 0usize; + let mut locale = false; + while cs.get(i) == Some(&'[') { + match cs.get(i + 1..).and_then(|r| r.iter().position(|&d| d == ']')) { + Some(p) => { + locale |= cs.get(i + 1) == Some(&'$'); + i += p + 2; + } + None => break, + } + } + (i, locale) } fn tokenize(src: &str) -> Raw { let cs: Vec = src.chars().collect(); - let mut raw = Raw { toks: Vec::new(), color: None, cond: None, currency: false }; + let mut raw = Raw { toks: Vec::new(), color: None, cond: None, currency: false, calendar: Calendar::Gregorian, prefix: None, invalid: false }; let toks = &mut raw.toks; // With no `]` at or after a `[`, that `[` is a literal; checking this once keeps the scan linear. let last_close = cs.iter().rposition(|&c| c == ']'); + let (body_start, locale_first) = after_leading_brackets(&cs); + // Where the next calendar prefix may stand: the body's start, or right after another prefix. + let mut prefix_at = body_start; let mut i = 0usize; while let Some(&c) = cs.get(i) { match c { + // A calendar prefix, never shown: `B1` or `B2`, any case, which Excel accepts repeated + // (`B2B2`) but not mixed (`B2B1`) or with more digits (`B12`, `B02`). + 'B' | 'b' if i == prefix_at && cs.get(i + 1).is_some_and(char::is_ascii_digit) => { + let d = cs.get(i + 1).copied(); + let ok = matches!(d, Some('1' | '2')) && !cs.get(i + 2).is_some_and(char::is_ascii_digit) && raw.prefix.is_none_or(|p| Some(p) == d); + raw.invalid |= !ok; + raw.prefix = d; + i += 2; + prefix_at = i; + continue; + } + // Any other `B` and digit (`B3dd`, `dd/mm/yyyyB2`, `dd B2 mm`, `B2[Red]B2`) makes Excel + // reject the code. + 'B' | 'b' if cs.get(i + 1).is_some_and(char::is_ascii_digit) => { + raw.invalid = true; + push_lit(toks, c.encode_utf8(&mut [0; 4])); + } '"' => { let mut s = String::new(); i += 1; @@ -283,6 +336,8 @@ fn tokenize(src: &str) -> Raw { if is_currency_char(d) { raw.currency = true; } + // Excel rejects an escaped `\B2` as well. + raw.invalid |= matches!(d, 'B' | 'b') && cs.get(i + 2).is_some_and(char::is_ascii_digit); push_lit(toks, d.encode_utf8(&mut [0; 4])); } i += 2; @@ -383,6 +438,11 @@ fn tokenize(src: &str) -> Raw { } i += 1; } + // Excel keeps the date Gregorian when a `[$-401]`-style locale or currency tag comes before + // `B2` (`B2[$-401]` is still Hijri). + if raw.prefix == Some('2') && !locale_first { + raw.calendar = Calendar::Hijri; + } raw } @@ -438,7 +498,7 @@ fn date_part(t: &Tok) -> Option { } fn analyze(raw: Raw) -> Section { - let Raw { toks, color, cond, currency } = raw; + let Raw { toks, color, cond, currency, calendar, prefix, invalid } = raw; let is_date = toks.iter().any(|t| matches!(t, Tok::Date(_))); let has_general = toks.iter().any(|t| matches!(t, Tok::General)); let mut sec = Section { @@ -455,6 +515,8 @@ fn analyze(raw: Raw) -> Section { fraction: false, currency, fill: toks.iter().any(|t| matches!(t, Tok::Fill(_))), + calendar, + invalid, }; if is_date { sec.kind = SecKind::Date; @@ -469,6 +531,9 @@ fn analyze(raw: Raw) -> Section { sec.kind = SecKind::Text; return analyze_text(toks, sec); } + // Number section. Excel rejects a calendar prefix before digits (`B20.00`, `B1#,##0`); + // `B2General`, `B2@` and `B2"x"` show as without the prefix. + sec.invalid |= prefix.is_some() && sec.has_digits; analyze_number(toks, sec) } diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index bb7fd1514..bfd31e56c 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -1,8 +1,9 @@ //! Rendering a value with a parsed section. use gridcraft_core::DateSystem; -use gridcraft_core::date::{MONTHS, WEEKDAYS, datetime_from_serial, max_serial}; +use gridcraft_core::date::{datetime_from_serial, max_serial}; +use crate::calendar::Calendar; use crate::decimal::{decimal_digits, fixed, fixed_from_digits, format_general_fit}; use crate::parse::{DatePart, Section, Tok}; @@ -72,6 +73,15 @@ pub(crate) fn render_date(sec: &Section, v: f64, sys: DateSystem) -> Option day += 1; } let total_ms = day.saturating_mul(86_400_000).saturating_add(ms); + // Under `B2`, Excel shows the 1900 system's fictitious 29 February (serial 60) as `#VALUE!` + // whenever a clock or calendar part is shown, even for `hh:mm` (`[h]` alone still works). + if sec.calendar == Calendar::Hijri + && sys == DateSystem::D1900 + && day == 60 + && sec.toks.iter().any(|t| matches!(t, Tok::Date(d) if !d.is_elapsed())) + { + return None; + } let needs_date = sec.toks.iter().any(|t| matches!(t, Tok::Date(d) if d.is_calendar())); let dt = if needs_date { datetime_from_serial(sys, day as f64)? } else { datetime_from_serial(sys, 0.0).unwrap_or_default() }; let ampm = sec.toks.iter().any(|t| matches!(t, Tok::Date(DatePart::AmPm | DatePart::AP(..)))); @@ -80,8 +90,8 @@ pub(crate) fn render_date(sec: &Section, v: f64, sys: DateSystem) -> Option let second = (ms / 1000) % 60; let milli = ms % 1000; let pad = |n: i64, w: u8| if w >= 2 { format!("{:0w$}", n, w = w.min(20) as usize) } else { n.to_string() }; - let month_name = MONTHS.get((dt.month as usize).wrapping_sub(1)).copied().unwrap_or(""); - let day_name = WEEKDAYS.get(dt.weekday as usize).copied().unwrap_or(""); + // No date in the section's calendar (serial 60 under `B2`): shown like an out-of-range date. + let date = sec.calendar.date(&dt)?; let mut out = Out::default(); for t in &sec.toks { match t { @@ -89,21 +99,21 @@ pub(crate) fn render_date(sec: &Section, v: f64, sys: DateSystem) -> Option let s = match *d { DatePart::Year(n) => { if n <= 2 { - format!("{:02}", dt.year.rem_euclid(100)) + format!("{:02}", date.year.rem_euclid(100)) } else { - format!("{:04}", dt.year) + format!("{:04}", date.year) } } DatePart::Month(n) => match n { - 1 | 2 => pad(dt.month as i64, n), - 3 => month_name.chars().take(3).collect(), - 5 => month_name.chars().take(1).collect(), - _ => month_name.to_string(), + 1 | 2 => pad(date.month as i64, n), + 3 => date.month_short.to_string(), + 5 => date.month_name.chars().take(1).collect(), + _ => date.month_name.to_string(), }, DatePart::Day(n) => match n { - 1 | 2 => pad(dt.day as i64, n), - 3 => day_name.chars().take(3).collect(), - _ => day_name.to_string(), + 1 | 2 => pad(date.day as i64, n), + 3 => date.day_short.to_string(), + _ => date.day_name.to_string(), }, DatePart::Hour(n) => { let h = if ampm { diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index b2709b320..0e9a169a5 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -902,6 +902,240 @@ fn closed_brackets_still_hold_their_semicolons() { assert_eq!(f("\"[\"0;0", 5.0), "[5"); } +const JAN_1_2001: f64 = 36892.0; + +#[test] +fn b1_prefix_is_hidden_and_gregorian() { + assert_eq!(f("B1dd/mm/yyyy", 46314.0), "19/10/2026"); + assert_eq!(f("b1dddd, mmmm d, yyyy", 46314.0), "Monday, October 19, 2026"); + assert_eq!(f("B1yyyy-mm-dd hh:mm", 46314.5), "2026-10-19 12:00"); +} + +#[test] +fn b2_prefix_renders_hijri_date() { + assert_eq!(f("B2dd/mm/yyyy", JAN_1_2001), "06/10/1421"); + assert_eq!(f("b2d/m/yy", JAN_1_2001), "6/10/21"); + assert_eq!(f("B2dddd dd/mm/yyyy hh:mm", JAN_1_2001 + 0.75), "الإثنين 06/10/1421 18:00"); + assert_eq!(text_function(&Value::Number(46314.0), "B2dd/mm/yyyy", S).as_deref(), Ok("08/05/1448")); + assert_eq!(f("B2e", 46314.0), "1448"); + assert_eq!(f("B2ee", 46314.0), "1448"); + assert_eq!(f("B2d/m/yyyy;@", 46314.0), "8/5/1448"); +} + +/// Under `B2`, Excel shows Arabic weekday names, the full name for `ddd` as well as `dddd`. +#[test] +fn b2_prefix_names_weekdays_in_arabic() { + // Monday 19 and Tuesday 20 October 2026. + assert_eq!(f("B2dddd", 46314.0), "الإثنين"); + assert_eq!(f("B2ddd", 46314.0), "الإثنين"); + assert_eq!(f("B2dddd", 46315.0), "الثلاثاء"); + assert_eq!(f("B1dddd", 46314.0), "Monday"); + assert_eq!(f("B1ddd", 46315.0), "Tue"); + // Every weekday has a name. + for v in 46314..46321 { + assert!(!f("B2dddd", v as f64).is_empty()); + } +} + +/// `B2` on a General or text section changes nothing: Excel shows the plain serial. +#[test] +fn b2_prefix_on_general_and_text_sections() { + assert_eq!(f("B2General", 46314.0), "46314"); + assert_eq!(f("B2@", 46314.0), "46314"); +} + +#[test] +fn b2_prefix_names_hijri_months() { + assert_eq!(f("B2d mmmm yyyy", JAN_1_2001), "6 شوال 1421"); + assert_eq!(f("B2mmm", 37034.0), "ربيع\u{a0}الأول"); + assert_eq!(f("B2mmmm", JAN_1_2001 + 23.0), "شوال"); + assert_eq!(f("B2mmmm", JAN_1_2001 + 24.0), "ذو\u{a0}القعدة"); + assert_eq!(f("B2mmmmm", JAN_1_2001 + 24.0), "ذ"); +} + +/// All twelve month names and seven weekday names as Excel 16.0 (build 20527, en-US) shows them, +/// read with TEXT and a cell format: two-word month names are joined by a no-break space. +#[test] +fn b2_names_match_excel() { + let months = [ + (46219.0, "صفر"), + (46248.0, "ربيع\u{a0}الأول"), + (46278.0, "ربيع\u{a0}الثاني"), + (46307.0, "جمادى\u{a0}الأولى"), + (46337.0, "جمادى\u{a0}الثانية"), + (46366.0, "رجب"), + (46396.0, "شعبان"), + (46425.0, "رمضان"), + (46455.0, "شوال"), + (46484.0, "ذو\u{a0}القعدة"), + (46514.0, "ذو\u{a0}الحجة"), + (46543.0, "محرم"), + ]; + for (serial, name) in months { + assert_eq!(f("B2dd", serial), "01", "{serial}"); + assert_eq!(f("B2mmmm", serial), name, "{serial}"); + assert_eq!(f("B2mmm", serial), name, "{serial}"); + assert_eq!(f("B2mmmmmm", serial), name, "{serial}"); + } + // Sunday 18 to Saturday 24 October 2026. + let days = ["الأحد", "الإثنين", "الثلاثاء", "الأربعاء", "الخميس", "الجمعة", "السبت"]; + for (k, name) in days.iter().enumerate() { + let serial = 46313.0 + k as f64; + assert_eq!(f("B2dddd", serial), *name, "{serial}"); + assert_eq!(f("B2ddd", serial), *name, "{serial}"); + } + assert_eq!(f("B2yy", 46314.0), "48"); + assert_eq!(f("B2y", 46314.0), "48"); + assert_eq!(f("B2yyy", 46314.0), "1448"); + assert_eq!(f("B2yyyyy", 46314.0), "1448"); +} + +#[test] +fn b2_month_and_year_rollover() { + assert_eq!(f("B2dd/mm/yyyy", 36915.0), "29/10/1421"); + assert_eq!(f("B2dd/mm/yyyy", 36916.0), "01/11/1421"); + assert_eq!(f("B2dd/mm/yyyy", 36974.0), "29/12/1421"); + assert_eq!(f("B2dd/mm/yyyy", 36975.0), "01/01/1422"); + assert_eq!(f("B2dd/mm/yyyy", 37683.0), "30/12/1423"); + assert_eq!(f("B2dd/mm/yyyy", 37684.0), "01/01/1424"); +} + +/// Cycle years 15 (1425, leap) and 16 (1426, not leap): Excel's leap year is 15, not 16. The +/// 2 March 2005 and 29 January 2006 values were read from Excel by a reviewer; the year +/// boundaries follow from them and the 354/355-day years. The same comparison covered every +/// serial from 36,000 to 49,000 with no disagreement. +#[test] +fn b2_leap_year_is_cycle_year_15() { + assert_eq!(f("B2yyyy-mm-dd", 38413.0), "1426-01-21"); // 2005-03-02, from Excel + assert_eq!(f("B2yyyy-mm-dd", 38746.0), "1426-12-29"); // 2006-01-29, from Excel + assert_eq!(f("B2dd/mm/yyyy", 38037.0), "29/12/1424"); // 2004-02-20 + assert_eq!(f("B2dd/mm/yyyy", 38038.0), "01/01/1425"); // 2004-02-21 + assert_eq!(f("B2dd/mm/yyyy", 38392.0), "30/12/1425"); // 2005-02-09 + assert_eq!(f("B2dd/mm/yyyy", 38393.0), "01/01/1426"); // 2005-02-10 + assert_eq!(f("B2dd/mm/yyyy", 38747.0), "01/01/1427"); // 2006-01-30 + // Anchors the reviewer read from Excel. + assert_eq!(f("B2dd/mm/yyyy", 46314.0), "08/05/1448"); // 2026-10-19 + assert_eq!(f("B2dd/mm/yyyy", 46313.0), "07/05/1448"); // 2026-10-18 + assert_eq!(f("B2dd/mm/yyyy", JAN_1_2001), "06/10/1421"); + assert_eq!(f("B2dd/mm/yyyy", 61.0), "29/10/1317"); // 1900-03-01 + assert_eq!(f("B2dd/mm/yyyy", 64970.0), "01/01/1501"); // 2077-11-16 + assert_eq!(f("B2dd/mm/yyyy", 64971.0), "02/01/1501"); // 2077-11-17 + assert_eq!(f("B2dd/mm/yyyy", 0.0), "29/08/1317"); + assert_eq!(f("B2dd/mm/yyyy", 2958465.0), "03/04/9666"); // 9999-12-31 +} + +/// Excel honours `B2` after a colour tag, but a `[$-...]` locale tag before it keeps the date +/// Gregorian (the prefix is still hidden); `B2[$-401]` is Hijri. +#[test] +fn calendar_prefix_and_bracket_tags() { + assert_eq!(f("[Red]B2dd/mm/yyyy", 46314.0), "08/05/1448"); + assert_eq!(f("[<0]0;B2dd/mm/yyyy", JAN_1_2001), "06/10/1421"); + assert_eq!(f("B2[$-401]dd/mm/yyyy", 46314.0), "08/05/1448"); + for code in ["[$-401]B2dd/mm/yyyy", "[$-409]B2dd/mm/yyyy", "[$-1010401]B2dd/mm/yyyy", "[$-ar-SA]B2dd/mm/yyyy"] { + assert_eq!(f(code, 46314.0), "19/10/2026", "{code}"); + assert_eq!(text_function(&Value::Number(46314.0), code, S).as_deref(), Ok("19/10/2026"), "{code}"); + } + assert_eq!(f("[Red][$-401]B1dd/mm/yyyy", JAN_1_2001), "01/01/2001"); + // Checked in Excel 16.0 (build 20527, en-US): a currency tag cancels `B2` like a locale tag, + // a tag after the prefix does not, and a repeated prefix is accepted. + assert_eq!(f("[$€-407]B2dd/mm/yyyy", 46314.0), "€19/10/2026"); + assert_eq!(f("[$$-409]B2dd/mm/yyyy", 46314.0), "$19/10/2026"); + assert_eq!(f("[$-401][Red]B2dd/mm/yyyy", 46314.0), "19/10/2026"); + assert_eq!(f("B2[Red]dd/mm/yyyy", 46314.0), "08/05/1448"); + for code in ["B2B2dd/mm/yyyy", "B2B2B2dd/mm/yyyy", "B2b2dd/mm/yyyy", "[Red]B2B2dd/mm/yyyy"] { + assert_eq!(f(code, 46314.0), "08/05/1448", "{code}"); + } + assert_eq!(f("B1B1dd/mm/yyyy", 46314.0), "19/10/2026"); + assert_eq!(f("B2 dd/mm/yyyy", 46314.0), " 08/05/1448"); + assert_eq!(f("B2\"x\"dd", 46314.0), "x08"); + for code in ["B1\"x\"", "B2\"x\""] { + assert_eq!(f(code, 46314.0), "x", "{code}"); + } + for code in ["B1General", "B1@", "0;0;0;B2@", "0;B2dd/mm/yyyy"] { + assert_eq!(f(code, 46314.0), "46314", "{code}"); + } +} + +/// A quoted `B2` is literal text; any other `B` and digit makes Excel reject the code (`TEXT` is +/// `#VALUE!`), as does a prefix before a number format. A rejected code shows as General. All +/// checked in Excel 16.0 (build 20527, en-US), with TEXT and with a cell format. +#[test] +fn misplaced_calendar_prefix_is_rejected() { + assert_eq!(f("\"B2\"dd/mm/yyyy", 46314.0), "B219/10/2026"); + assert_eq!(f("dd/\"B2\"/yyyy", 46314.0), "19/B2/2026"); + for code in [ + "B0dd/mm/yyyy", + "B3dd/mm/yyyy", + "B4dd/mm/yyyy", + "B9dd/mm/yyyy", + "B02dd", + "B12dd", + "B21dd", + "B1B2dd/mm/yyyy", + "B2B1dd/mm/yyyy", + "B2[Red]B2dd/mm/yyyy", + "B2 B2dd", + "dd/mm/yyyyB2", + "dd/mm/yyyyB1", + "dd B2 mm", + "dd b2 mm", + "dd \\B2 yyyy", + "B2\\B2dd", + "B20.00", + "B2#,##0", + "B10.00", + "B1#,##0", + "0;B3dd", + ] { + assert_eq!(text_function(&Value::Number(46314.0), code, S), Err(CellError::Value), "{code}"); + assert_eq!(f(code, 46314.0), "46314", "{code}"); + assert!(NumberFormat::parse(code).is_general(), "{code}"); + } +} + +#[test] +fn b2_edges_of_the_date_range() { + // Serial 60 is the 1900 system's fictitious 29 February: Excel has no Hijri date for it. + assert_eq!(text_function(&Value::Number(60.0), "B2dd/mm/yyyy", S), Err(CellError::Value)); + assert_eq!(f("B2dd/mm/yyyy", 60.0), "########"); + assert_eq!(f("B1dd/mm/yyyy", 60.0), "29/02/1900"); + assert_eq!(f("B2dd/mm/yyyy", 59.0), "28/10/1317"); + assert_eq!(f("B2dd/mm/yyyy", 61.0), "29/10/1317"); + // So does any clock part (Excel 16.0, build 20527, en-US), but not an elapsed `[h]`, a literal, + // or a locale tag that cancels `B2`. + for code in ["B2hh:mm", "B2hh", "B2ss", "B2am/pm", "B2hh:mm AM/PM", "B2[$-401]hh:mm", "B2dd/mm/yyyy;\"x\""] { + assert_eq!(text_function(&Value::Number(60.0), code, S), Err(CellError::Value), "{code}"); + assert_eq!(text_function(&Value::Number(60.5), code, S), Err(CellError::Value), "{code}"); + } + assert_eq!(f("B2[h]", 60.0), "1440"); + assert_eq!(f("B2\"x\"", 60.0), "x"); + assert_eq!(f("[$-401]B2hh:mm", 60.0), "00:00"); + assert_eq!(f("B1hh:mm", 60.0), "00:00"); + assert_eq!(f("B2hh:mm", 61.0), "00:00"); + // Serial 0 shows as serial 1. + assert_eq!(f("B2dd/mm/yyyy", 0.0), "29/08/1317"); + assert_eq!(f("B2dd/mm/yyyy", 1.0), "29/08/1317"); + // No upper limit short of 31 December 9999: 16 and 17 November 2077, 31 December 9999. + assert_eq!(f("B2dd/mm/yyyy", 64970.0), "01/01/1501"); + assert_eq!(f("B2dd/mm/yyyy", 64971.0), "02/01/1501"); + assert_eq!(f("B2dd/mm/yyyy", 2958465.0), "03/04/9666"); + // In the 1904 system, serial 60 is a real day. + assert_ne!(format_value(&Value::Number(60.0), &NumberFormat::parse("B2dd/mm/yyyy"), DateSystem::D1904).text, "########"); +} + +#[test] +fn b2_out_of_range_does_not_panic() { + for v in [-1.0, 0.0, 60.0, 60.5, 2958465.0, 2958466.0, 1e300, f64::NAN] { + let _ = f("B2dd/mm/yyyy mmmm dddd", v); + let _ = text_function(&Value::Number(v), "B2dd/mm/yyyy mmmm dddd", S); + } + assert_eq!(f("B2dd/mm/yyyy", -1.0), f("dd/mm/yyyy", -1.0)); + assert_eq!(f("B2dd/mm/yyyy", 2958466.0), f("dd/mm/yyyy", 2958466.0)); + // Before 1 Muharram 1 AH there is no Hijri date (no serial reaches it). + let before_epoch = gridcraft_core::date::DateTime { year: 600, month: 3, day: 9, ..Default::default() }; + assert!(calendar::Calendar::Hijri.date(&before_epoch).is_none()); +} + /// "Set precision as displayed": the value each format stores. Expected values observed in Excel /// (COM, `Workbook.PrecisionAsDisplayed = True`) with synthetic values. #[test] diff --git a/docs/localization-parity.md b/docs/localization-parity.md index 3f1dacb96..d504c19ea 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-27 · **Change:** minor (AutoFit estimates Arabic text width from measured glyph widths); previously minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (number formats honour the `B1`/`B2` calendar prefixes, checked against Excel; earlier: missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks); previously minor (AutoFit estimates Arabic text width from measured glyph widths); previously minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **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, @@ -40,7 +40,21 @@ right-to-left sheets and full complex-script support. Part of - **en-US only formulas and formats.** Function names, `,` argument separator, `.` decimal separator, en-US number and date formats (`crates/numfmt` says "en-US only"). Users ask for localized function names and `;` separators (#45). German with localized numbers and formulas - is in PR #163 (open). + is in PR #163 (open). The `B1` (Gregorian) and `B2` (Hijri) calendar prefixes that Arabic + Excel writes at the start of a format section are hidden; under `B2` the year, month number, + day, month names and weekday names (Arabic; `mmm` and `ddd` show the full name) count in the + tabular Hijri calendar (leap years 2, 5, 7, 10, 13, 15, 18, 21, 24, 26 and 29 of each 30-year + cycle). Checked against Excel 16.0 (build 20527, en-US) on every serial from 1 to 2,958,465 + and about 1,700 format/value pairs: a `[$...]` locale or currency tag before `B2` keeps the + date Gregorian; a repeated prefix (`B2B2`) is accepted; any other `B` and digit (`B3`, + `B12`, `B1B2`, `dd B2`, `\B2`), or a prefix before a number format (`B10.00`), is + rejected (`TEXT` gives `#VALUE!`, a cell shows General); serial 60 shows `#VALUE!` under `B2` + for any date or clock part; two-word month names use a no-break space. Differs from Excel: + in a `B2` section with a weekday and a month name, Excel prints the weekday for + `mmmm`/`mmmmm` after `ddd`/`dddd`, and the weekday numbered by the month (Saturday from + month 7) for `ddd`/`dddd` after `mmm`/`mmmm`; a `B2` section showing only the weekday uses + the real weekday for serials 0–59 and gives `#VALUE!` for the last days of 9999. Excel's + Buddhist-year `b`/`bb`/`bbbb` tokens are not supported. - **Script support.** System-font fallback for CJK, Thai and Hebrew landed in #38 (closes #32; #142 is still open pending confirmation). Native CJK fallback also probes Flatpak's read-only `/run/host/fonts`, `/run/host/local-fonts` and `/run/host/user-fonts` layouts, so installed @@ -110,6 +124,7 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Number formats honour the `B1`/`B2` calendar prefixes: never shown; `B2` renders year, month, day and Arabic month and weekday names in the tabular Hijri calendar (1 Jan 2001 = 6 Shawwal 1421; leap years 2, 5, 7, 10, 13, 15, 18, 21, 24, 26 and 29 of the 30-year cycle, which is what Excel shows). Checked against Excel 16.0 (build 20527, en-US) on every serial and about 1,700 format/value pairs: a locale or currency tag before `B2` keeps the date Gregorian, a misplaced `B` and digit or a prefix before a number format is rejected, serial 60 has no Hijri date, serial 0 shows as serial 1, two-word month names use a no-break space. Not matched: Excel's weekday/month-name mix-ups in one `B2` section and its weekday-only quirks | | 2026-10-27 | minor | AutoFit column width, wrapped-row height and PivotTable column fit estimate Arabic letters at ~0.48 em (measured 0.36–0.53 em shaped, Noto Sans/Naskh Arabic and DejaVu Sans) instead of 1 em, Arabic-Indic digits as wide as ASCII digits, and give vowel marks and direction marks no width; vowelled Arabic no longer fits ~3.5× too wide | | 2026-10-23 | minor | Missing translations are reported, never a failure: `cargo xtask i18n-report` prints, per language, the strings other catalogs translate and it lacks (plus Spanish function help rows keyed by English the function catalog no longer has) and always passes; `cargo xtask ci` runs it first. Coverage and per-language parity tests and tests of one language's exact wording are gone; UI tests take the expected text from the catalog. Duplicate-key, placeholder, context-prefix and line-shape checks stay | | 2026-10-21 | minor | Romanian interface (`ro`, Română): 970 labels covering every key of the other catalogs (`crates/ui-egui/src/i18n/ro.rs`), `ro-*` system locales, the View-tab switcher and `app.language.set`; context keys show no prefix (File › Options test). English formulas and formats; AI-assisted draft pending native-speaker review | From cda767b49b1b4fdf691cb4cc4b95bdd40c9262d0 Mon Sep 17 00:00:00 2001 From: Mouhand Kaddo Date: Mon, 9 Nov 2026 18:54:55 +0400 Subject: [PATCH 7/8] Arabic font fallback in cells, the formula bar and the web (#931) * M12.AR2: Arabic font fallback in cells and the interface (#229) * M12.AR2: reserve the preferred Arabic file only once it has loaded load_fallback treated fontconfig's Arabic file as already loaded even when load_preferred_arabic could not load it, so a system Arabic face from the same file was skipped and the chains could end up with no Arabic system face. The file is now reserved only when the preferred face is in the font set; the candidate loop moves to load_faces so a test can drive it with a fixture file. Co-Authored-By: Claude Opus 5.5 * M12.AR2: say where fontconfig's Arabic choice applies fontconfig's Arabic face leads the Arabic fallbacks, so it draws Arabic where the UI or cell font has none. A UI font with Arabic of its own, such as DejaVu Sans in the Flatpak, still draws interface Arabic (#229), which is left for a follow-up. The docs and the test's doc comment no longer say the choice comes first in every chain. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: FredApps --- ROADMAP.md | 2 + crates/ui-egui/src/craft_fonts_policy.rs | 3 + crates/ui-egui/src/fonts.rs | 29 +++-- crates/ui-egui/src/theme.rs | 9 +- crates/ui-egui/src/theme/font_fallback.rs | 95 ++++++++++++++- crates/ui-egui/src/theme/font_setup.rs | 37 ++++-- crates/ui-egui/src/theme/tests.rs | 141 +++++++++++++++++++++- docs/gaps.md | 5 +- docs/localization-parity.md | 7 +- 9 files changed, 296 insertions(+), 32 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 340bb2d06..f12cd112a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -149,6 +149,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | 2026-10-31 | CSV and text open detects each column's date order: a value that reads only day first (`31/05/2026`) makes the column day first, a value that reads only month first (`12/31/2026`) makes it month first, an all-ambiguous column keeps month first with a warning, and a column with both kinds stays text with a warning (#2765) | | 2026-10-31 | Recently Used lists the functions most recently entered in a formula from any source (ribbon, control channel, MCP, scripts), kept with the preferences and returned by `formulas.recentlyUsed`; the Insert Function dialog takes Excel's title and a working category list (Most Recently Used, All, each category) | | 2026-10-31 | Formulas › Function Library as a big Insert Function button, the AutoSum split button and three columns of labelled small buttons, each category book in its own colour; category menus end with Insert Function… on that category; More Functions cascades into the remaining categories; Home › Editing shares the AutoSum menu; the MR key tip opens Recently Used | +| 2026-10-26 | Arabic text shows in cells and the interface through a system, fontconfig or craft-fonts Arabic face; workbook fonts keep their script fallbacks | | 2026-10-25 | AutoSum over a selection does what Excel does: a selected row gets one total to its right (in its last cell if that is empty), not a one-cell total under each cell; a block whose last column is empty gets each row's total there, one whose last row is empty each column's, and with both the corner gets the total of the block; otherwise totals go under each column as before | | 2026-10-24 | Macro interface ([macros.md](docs/macros.md#in-the-window)): an Automate › Macros group (Record Macro, which turns into Stop Recording, Use Relative References, Run Macro with recent macros); "Recording · N steps" with Stop in the status bar; a Record Macro dialog (checked name, free Ctrl+Shift shortcut, description); the Macros dialog lists macros beside Automate scripts and a VBA project, with Run, Step Into, Edit, Create, Delete and Options; Ctrl+Shift+letter macro shortcuts; questions that offer Run, Overwrite, Shift Down or Shift Right; a step editor with a breakpoint gutter, drag to reorder, a command picker, reference boxes that switch between relative tokens and absolute references, Inputs and Output tabs, a debug bar (Check, Run, Step, Continue, Run to Here, Stop with keep or undo) and grid overlays for inputs, writes, conflicts and the current step | | 2026-10-24 | Go To lists the named ranges of every sheet (`Sheet2!Name` for a name scoped to another sheet; hidden names, constants and formulas left out) and the references entered before, a double-click goes to an entry, and the Reference box takes several names or references separated by commas (`Price, Type`, `B14:C22,F19:G30`) | @@ -276,6 +277,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | 2026-10-31 | minor | File-page text fields keep the view open when Esc cancels an edit (#3035) | | 2026-10-31 | minor | Progress log: Recently Used and Insert Function categories | | 2026-10-31 | minor | Progress log: Function Library | +| 2026-10-26 | minor | Progress log: Arabic font fallback | | 2026-10-25 | minor | Progress log: AutoSum over a selected row, and a selection's empty last row and column | | 2026-10-24 | minor | Macro interface on the Automate tab; progress log row | | 2026-10-24 | minor | Macro engine and `macro.*` commands; catalog 275 / 291 → 276 / 292 (View › Macros › Record Macro); progress log row | diff --git a/crates/ui-egui/src/craft_fonts_policy.rs b/crates/ui-egui/src/craft_fonts_policy.rs index d88fa6a75..1a9109406 100644 --- a/crates/ui-egui/src/craft_fonts_policy.rs +++ b/crates/ui-egui/src/craft_fonts_policy.rs @@ -24,6 +24,7 @@ pub const REQUIRED_FACES: &[(&str, &str, &[&str])] = &[ ("NanumGothic", "Regular", &["Hang", "Kore"]), ("Noto Sans Symbols", "Regular", &["Zsym"]), ("Noto Sans Symbols 2", "Regular", &["Zsym"]), + ("Noto Sans Arabic", "Regular", &["Arab"]), ]; /// The native-only Simplified Chinese face from craft-fonts' separate @@ -41,6 +42,7 @@ pub const WEB_ALLOWED: &[(&str, &str)] = &[ ("NanumGothic", "Regular"), ("Noto Sans Symbols", "Regular"), ("Noto Sans Symbols 2", "Regular"), + ("Noto Sans Arabic", "Regular"), ]; /// One validated `fonts/manifest.txt` row. Only the fields GridCraft consumes are kept. @@ -172,6 +174,7 @@ mod tests { ("NanumGothic", "Regular"), ("Noto Sans Symbols", "Regular"), ("Noto Sans Symbols 2", "Regular"), + ("Noto Sans Arabic", "Regular"), ] ); for (family, _) in WEB_ALLOWED { diff --git a/crates/ui-egui/src/fonts.rs b/crates/ui-egui/src/fonts.rs index 699806eba..da7dd57ec 100644 --- a/crates/ui-egui/src/fonts.rs +++ b/crates/ui-egui/src/fonts.rs @@ -19,6 +19,24 @@ use std::sync::OnceLock; #[cfg(not(target_arch = "wasm32"))] const MAX_FONT_FILE: u64 = 64 << 20; +/// Reads a font file of at most [`MAX_FONT_FILE`] bytes, even when it grows after it was found. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn read_font(path: &std::path::Path) -> Option> { + use std::io::Read as _; + let file = std::fs::File::open(path).ok()?; + if file.metadata().ok()?.len() > MAX_FONT_FILE { + return None; + } + let mut bytes = Vec::new(); + file.take(MAX_FONT_FILE + 1).read_to_end(&mut bytes).ok()?; + (bytes.len() as u64 <= MAX_FONT_FILE).then_some(bytes) +} + +#[cfg(target_arch = "wasm32")] +pub(crate) fn read_font(_path: &std::path::Path) -> Option> { + None +} + /// A face keeps at most this many family names, so a hostile `name` table can't grow the catalog. #[cfg(not(target_arch = "wasm32"))] const MAX_FAMILY_NAMES: usize = 16; @@ -172,7 +190,7 @@ pub fn scan_dirs(dirs: &[PathBuf]) -> Vec { if !is_font || std::fs::metadata(&path).map(|m| m.len() > MAX_FONT_FILE).unwrap_or(true) { continue; } - let Ok(data) = std::fs::read(&path) else { continue }; + let Some(data) = read_font(&path) else { continue }; for (index, family, names, style) in file_faces(&data) { out.push(Face { family, names, path: path.clone(), index, weight: style_weight(&style), italic: style_italic(&style) }); } @@ -335,7 +353,7 @@ pub fn bind_faces(fonts: &mut egui::FontDefinitions, family: &str, faces: &[Face continue; } if !fonts.font_data.contains_key(&key) { - let Ok(bytes) = std::fs::read(&face.path) else { continue }; + let Some(bytes) = read_font(&face.path) else { continue }; let mut fd = egui::FontData::from_owned(bytes); fd.index = face.index; fonts.font_data.insert(key.clone(), std::sync::Arc::new(fd)); @@ -353,11 +371,6 @@ pub fn bind_faces(fonts: &mut egui::FontDefinitions, family: &str, faces: &[Face bound } -/// Bind every installed face of `family`, found by any of its names (see [`bind_faces`]). -pub fn bind_family(fonts: &mut egui::FontDefinitions, family: &str) -> Vec { - bind_faces(fonts, family, &faces_of(family)) -} - /// The role families currently bound to font data. `cell_family` consults this, so it never names a /// family egui hasn't been given. static BOUND: OnceLock>> = OnceLock::new(); @@ -472,6 +485,8 @@ mod tests { } // A family with no faces binds nothing and never touches the map. assert!(bind_faces(&mut fonts, "No Such Font", &[]).is_empty()); + std::fs::File::create(&face.path).expect("sparse file").set_len(MAX_FONT_FILE + 1).expect("oversized file"); + assert!(read_font(&face.path).is_none(), "an oversized font is not read"); let _ = std::fs::remove_dir_all(&dir); } diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 343fd79f4..9ebf19944 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -25,7 +25,12 @@ mod font_setup; pub use font_fallback::symbol_face_installed; #[cfg(test)] -use font_fallback::{cjk_han_japanese, cjk_han_simplified, cjk_hangul, fallback_faces, hebrew, lao, symbols_more, thai}; +use font_fallback::{ + PREFERRED_ARABIC, arabic, cjk_han_japanese, cjk_han_simplified, cjk_hangul, fallback_faces, hebrew, lao, load_faces, preferred_file, + symbols_more, thai, +}; +#[cfg(all(test, unix, not(target_os = "macos")))] +use font_fallback::{covers_arabic, fontconfig_arabic, parse_fc_match}; use font_setup::pick; pub use font_setup::{ CELL, CELL_BOLD, CELL_BOLD_ITALIC, CELL_ITALIC, CraftFont, HanOrder, MATH, MATH_BOLD, MATH_ITALIC, MONO, SERIF, UI, UI_BOLD, available_text, @@ -33,7 +38,7 @@ pub use font_setup::{ font_definitions, font_definitions_for_language, font_definitions_with, pinned_font_definitions, symbol_fallback, ui_bold, ui_font, }; #[cfg(test)] -use font_setup::{candidates, craft_font_faces, try_load}; +use font_setup::{candidates, craft_font_faces, font_definitions_impl, try_load}; #[cfg(test)] pub(crate) use font_setup::{drop_unparseable_fonts, font_parses}; diff --git a/crates/ui-egui/src/theme/font_fallback.rs b/crates/ui-egui/src/theme/font_fallback.rs index 1f5d70150..65479d017 100644 --- a/crates/ui-egui/src/theme/font_fallback.rs +++ b/crates/ui-egui/src/theme/font_fallback.rs @@ -12,12 +12,25 @@ use super::font_setup::{in_dirs, try_load}; /// is bundled, and a machine without the face keeps the old behaviour. #[cfg(not(target_arch = "wasm32"))] pub(super) fn load_fallback(fonts: &mut FontDefinitions, han: HanOrder) -> Vec { + let loaded = preferred_file(fonts, fontconfig_arabic()); + load_faces(fonts, fallback_faces(han), loaded) +} + +/// The preferred Arabic file, once [`load_preferred_arabic`] has loaded it: a file that failed to +/// load must not keep the system Arabic faces out of the chains. +#[cfg(not(target_arch = "wasm32"))] +pub(super) fn preferred_file(fonts: &FontDefinitions, preferred: Option<(String, u32)>) -> Vec { + preferred.filter(|_| fonts.font_data.contains_key(PREFERRED_ARABIC)).map(|(path, _)| path).into_iter().collect() +} + +/// Loads the first candidate of each fallback face that loads and returns their keys. `loaded` +/// holds the files already loaded as a fallback face. A file serves at most one key: the +/// region-merged Noto Sans CJK .ttc found for Han also carries Hangul (and Tahoma carries both Thai +/// and Hebrew), so loading it again for a later script would only duplicate tens of MB in memory. +#[cfg(not(target_arch = "wasm32"))] +pub(super) fn load_faces(fonts: &mut FontDefinitions, faces: Vec<(&'static str, Vec<(String, u32)>)>, mut loaded: Vec) -> Vec { let mut keys = Vec::new(); - // Files already loaded as a fallback face. A file serves at most one key: the region-merged - // Noto Sans CJK .ttc found for Han also carries Hangul (and Tahoma carries both Thai and - // Hebrew), so loading it again for a later script would only duplicate tens of MB in memory. - let mut loaded: Vec = Vec::new(); - for (key, cands) in fallback_faces(han) { + for (key, cands) in faces { for (path, index) in &cands { if loaded.contains(path) { break; // a face loaded for an earlier script already covers this one @@ -38,6 +51,60 @@ pub(super) fn load_fallback(_fonts: &mut FontDefinitions, _han: HanOrder) -> Vec Vec::new() } +/// The face fontconfig prefers for Arabic leads the script faces (#229), ahead of the system and +/// bundled Arabic faces. +#[cfg(not(target_arch = "wasm32"))] +pub(super) fn load_preferred_arabic(fonts: &mut FontDefinitions) -> Vec { + let Some((path, index)) = fontconfig_arabic() else { return Vec::new() }; + let Some(fd) = try_load(&[(path.as_str(), index)]) else { return Vec::new() }; + fonts.font_data.insert(PREFERRED_ARABIC.into(), Arc::new(fd)); + vec![PREFERRED_ARABIC.into()] +} + +#[cfg(target_arch = "wasm32")] +pub(super) fn load_preferred_arabic(_fonts: &mut FontDefinitions) -> Vec { + Vec::new() +} + +#[cfg(not(target_arch = "wasm32"))] +pub(super) const PREFERRED_ARABIC: &str = "sys-arabic-preferred"; + +/// The face fontconfig prefers for Arabic, such as a Vazirmatn the user set as their Arabic font +/// (#229), kept only when it has Arabic letters. Asked once per process. +#[cfg(all(unix, not(target_os = "macos")))] +pub(super) fn fontconfig_arabic() -> Option<(String, u32)> { + static PREFERRED: std::sync::OnceLock> = std::sync::OnceLock::new(); + PREFERRED + .get_or_init(|| { + let out = std::process::Command::new("fc-match").args(["--format=%{file}\n%{index}\n", ":lang=ar"]).output().ok()?; + let (file, index) = parse_fc_match(&String::from_utf8_lossy(&out.stdout)).filter(|_| out.status.success())?; + let bytes = crate::fonts::read_font(std::path::Path::new(&file))?; + covers_arabic(&bytes, index).then_some((file, index)) + }) + .clone() +} + +#[cfg(all(not(target_arch = "wasm32"), not(all(unix, not(target_os = "macos")))))] +pub(super) fn fontconfig_arabic() -> Option<(String, u32)> { + None +} + +/// The file and face index from `fc-match --format='%{file}\n%{index}\n'`. fontconfig packs a +/// variable font's named instance into the upper 16 bits of the index; only the face index is kept. +#[cfg(all(unix, not(target_os = "macos")))] +pub(super) fn parse_fc_match(out: &str) -> Option<(String, u32)> { + let mut lines = out.lines().map(str::trim); + let file = lines.next().filter(|file| !file.is_empty())?; + let index = lines.next().and_then(|index| index.parse::().ok()).map_or(0, |packed| packed & 0xFFFF); + Some((file.to_owned(), index)) +} + +#[cfg(all(unix, not(target_os = "macos")))] +pub(super) fn covers_arabic(bytes: &[u8], index: u32) -> bool { + use skrifa::MetadataProvider; + skrifa::FontRef::from_index(bytes, index).is_ok_and(|font| "العربية".chars().all(|ch| font.charmap().map(ch).is_some())) +} + /// Every fallback face we look for, in chain order: `(key, candidate files)`. One file per script /// family is enough — epaint picks the first face with the glyph, and a script's glyphs live in one /// file. Order only matters between faces that both cover a glyph (Japanese vs Chinese kanji), so @@ -57,6 +124,7 @@ pub(super) fn fallback_faces(han: HanOrder) -> Vec<(&'static str, Vec<(String, u ("sys-thai", thai()), ("sys-lao", lao()), ("sys-hebrew", hebrew()), + ("sys-arabic", arabic()), ("sys-symbols", symbols()), ("sys-symbols-2", symbols_more()), ] @@ -243,6 +311,23 @@ pub(super) fn hebrew() -> Vec<(String, u32)> { ) } +/// Arabic faces. Windows has Tahoma, Arial and Segoe UI with Arabic; macOS has Geeza Pro; Linux +/// usually has Noto Sans Arabic or Noto Naskh Arabic. Installed Noto and Amiri faces found by the +/// font scan come last, for layouts the paths miss. +#[cfg(not(target_arch = "wasm32"))] +pub(super) fn arabic() -> Vec<(String, u32)> { + let mut v = script_fonts( + &["tahoma.ttf", "arial.ttf", "segoeui.ttf"], + &["/System/Library/Fonts/GeezaPro.ttc", "/System/Library/Fonts/Supplemental/GeezaPro.ttc"], + &["NotoSansArabic-Regular.ttf", "NotoNaskhArabic-Regular.ttf", "noto/NotoSansArabic-Regular.ttf", "noto/NotoNaskhArabic-Regular.ttf"], + "/usr/share/fonts/google-noto-vf/NotoSansArabic[wght].ttf", + ); + for family in ["Noto Sans Arabic", "Noto Naskh Arabic", "Amiri"] { + v.extend(crate::fonts::faces_of(family).into_iter().map(|face| (face.path.to_string_lossy().into_owned(), face.index))); + } + v +} + /// Symbol faces behind the UI font. Interface fonts such as Noto Sans and Cantarell (and egui's /// built-in Ubuntu) lack arrows, check marks, geometric shapes and keyboard symbols (▾ ✓ ✕ ▲ ↳ ⇧), /// which then render as empty boxes (#197, #54). DejaVu Sans covers most of them; the paths cover diff --git a/crates/ui-egui/src/theme/font_setup.rs b/crates/ui-egui/src/theme/font_setup.rs index 3489b1443..da995c489 100644 --- a/crates/ui-egui/src/theme/font_setup.rs +++ b/crates/ui-egui/src/theme/font_setup.rs @@ -1,6 +1,6 @@ #[cfg(not(target_arch = "wasm32"))] use super::Blob; -use super::font_fallback::load_fallback; +use super::font_fallback::{load_fallback, load_preferred_arabic}; use super::{Arc, Color32, FontData, FontDefinitions, FontFamily, FontId}; /// A font from the optional craft-fonts build input (`CRAFT_FONTS_DIR` at build time; see @@ -152,7 +152,7 @@ fn font_file(path: &str, index: u32) -> Option { let bytes: &[u8] = (**blob).as_ref(); return font_parses(bytes, index).then(|| std::sync::Arc::clone(blob)); } - let bytes = std::fs::read(path).ok()?; + let bytes = crate::fonts::read_font(std::path::Path::new(path))?; if !font_parses(&bytes, index) { log::warn!("skipping font {path} (face {index}): not a font file egui can read"); return None; @@ -466,37 +466,45 @@ pub fn font_definitions_for_language(language: crate::i18n::Language) -> FontDef /// machine see the same font set. Craft-fonts remains a pinned build input rather than a repository /// asset; without it only egui's bundled faces are available, which is the documented no-font mode. pub fn pinned_font_definitions() -> FontDefinitions { - font_definitions_impl(crate::i18n::Language::En, &[], false) + font_definitions_impl(crate::i18n::Language::En, Vec::new(), false) } /// [`font_definitions_for_language`] that also binds the workbook fonts in `used` that are /// installed, so cells set in them render with them; the rest fall back to the roles. The bound /// families are recorded in [`crate::fonts`] so [`cell_family`] only names a family egui has. pub fn font_definitions_with(language: crate::i18n::Language, used: &[String]) -> FontDefinitions { - font_definitions_impl(language, used, true) + let installed = crate::fonts::installed_among(used).into_iter().map(|family| { + let faces = crate::fonts::faces_of(&family); + (family, faces) + }); + font_definitions_impl(language, installed.collect(), true) } /// Builds the role font chains, script fallbacks, installed workbook fonts and optional craft-font faces, drops unparseable data and records which families were bound. -fn font_definitions_impl(language: crate::i18n::Language, used: &[String], system: bool) -> FontDefinitions { +pub(super) fn font_definitions_impl( + language: crate::i18n::Language, + installed: Vec<(String, Vec)>, + system: bool, +) -> FontDefinitions { let mut fonts = FontDefinitions::default(); let base_prop: Vec = fonts.families.get(&FontFamily::Proportional).cloned().unwrap_or_default(); let base_mono: Vec = fonts.families.get(&FontFamily::Monospace).cloned().unwrap_or_default(); let mut bound: Vec = Vec::new(); // The non-Latin fallback faces have to sit at the end of each chain: a Latin font owns Latin, // and epaint picks the first face that has the glyph, so they only catch their own scripts. - let fallback = if system { load_fallback(&mut fonts, HanOrder::of(language)) } else { Vec::new() }; + let fallback = if system { + let mut fallback = load_preferred_arabic(&mut fonts); + fallback.extend(load_fallback(&mut fonts, HanOrder::of(language))); + fallback + } else { + Vec::new() + }; let hack = fonts.font_data.contains_key(HACK).then_some(HACK); for role in [UI, UI_BOLD, CELL, CELL_BOLD, CELL_ITALIC, CELL_BOLD_ITALIC, SERIF, MONO, MATH, MATH_ITALIC, MATH_BOLD] { let chain = build_role_chain(&mut fonts, role, &base_prop, &base_mono, &fallback, hack, system); fonts.families.insert(FontFamily::Name(role.into()), chain); bound.push(role.to_string()); } - // Installed workbook fonts, so a cell's chosen font actually renders in it. - if system { - for family in crate::fonts::installed_among(used) { - bound.extend(crate::fonts::bind_family(&mut fonts, &family)); - } - } // Interface text (formula bar, name box, dialogs) falls back to the cell font before the // script faces: whatever a cell can draw, the formula bar can draw too, even where no script // fallback face was found (#349: macOS UI font without Hebrew, cells in Arial with it). @@ -505,6 +513,11 @@ fn font_definitions_impl(language: crate::i18n::Language, used: &[String], syste if let Some(ui) = fonts.families.get(&FontFamily::Name(UI.into())).cloned() { fonts.families.insert(FontFamily::Proportional, ui); } + // Installed workbook fonts, so a cell's chosen font actually renders in it. They are bound + // after the proportional chain has the script faces, which their chains end with. + for (family, faces) in installed { + bound.extend(crate::fonts::bind_faces(&mut fonts, &family, &faces)); + } // The optional craft-fonts faces go at the very end of every family: a system or bundled face // that has the script keeps its glyph shapes, while the permitted OFL faces cover scripts and // symbols the host image lacks (Japanese, Korean, Thai, Arabic, Georgian and the UI symbols). diff --git a/crates/ui-egui/src/theme/tests.rs b/crates/ui-egui/src/theme/tests.rs index 31dc653ca..359336ee8 100644 --- a/crates/ui-egui/src/theme/tests.rs +++ b/crates/ui-egui/src/theme/tests.rs @@ -76,6 +76,7 @@ fn the_required_craft_faces_keep_their_exact_scripts() { ("NanumGothic", "Regular", ["Hang", "Kore"].as_slice()), ("Noto Sans Symbols", "Regular", ["Zsym"].as_slice()), ("Noto Sans Symbols 2", "Regular", ["Zsym"].as_slice()), + ("Noto Sans Arabic", "Regular", ["Arab"].as_slice()), ] { assert!( faces.iter().any(|(f, s, declared)| *f == family && *s == style && *declared == scripts), @@ -121,11 +122,11 @@ fn language_reorders_only_the_han_faces() { let keys = |han| fallback_faces(han).into_iter().map(|(k, _)| k).collect::>(); assert_eq!( keys(HanOrder::Japanese), - ["sys-cjk-han-ja", "sys-cjk-han-zh", "sys-cjk-hangul", "sys-thai", "sys-lao", "sys-hebrew", "sys-symbols", "sys-symbols-2"] + ["sys-cjk-han-ja", "sys-cjk-han-zh", "sys-cjk-hangul", "sys-thai", "sys-lao", "sys-hebrew", "sys-arabic", "sys-symbols", "sys-symbols-2"] ); assert_eq!( keys(HanOrder::Chinese), - ["sys-cjk-han-zh", "sys-cjk-han-ja", "sys-cjk-hangul", "sys-thai", "sys-lao", "sys-hebrew", "sys-symbols", "sys-symbols-2"] + ["sys-cjk-han-zh", "sys-cjk-han-ja", "sys-cjk-hangul", "sys-thai", "sys-lao", "sys-hebrew", "sys-arabic", "sys-symbols", "sys-symbols-2"] ); assert_eq!(HanOrder::of(Language::Ja), HanOrder::Japanese); for l in [Language::En, Language::Zh, Language::Ko, Language::Ru, Language::PtBr, Language::De, Language::Cs, Language::Fr, Language::Tr] { @@ -184,7 +185,7 @@ fn fallback_covers_its_script() { // face or the optional craft-fonts input provides one; the printed skip is a recorded // gate, and a present face that is not wired in fails. type Case = (&'static str, Vec<(String, u32)>, &'static str); - let cases: [Case; 10] = [ + let cases: [Case; 11] = [ ("sys-cjk-han-ja", cjk_han_japanese(), "カテゴリ"), // Chinese uses a system Han face or the native-only permitted LXGW craft face; the // barred Noto CJK collection never supplies it. @@ -193,6 +194,7 @@ fn fallback_covers_its_script() { ("sys-thai", thai(), "ทดสอบ"), ("sys-lao", lao(), "ພາສາລາວ"), ("sys-hebrew", hebrew(), "עברית"), + ("sys-arabic", arabic(), "العربية١٢٣"), ("craft-noto-sans-arabic", Vec::new(), "مرحبا"), ("craft-noto-sans-georgian", Vec::new(), "ქართული"), ("ui", candidates(UI), "Русский"), @@ -221,6 +223,138 @@ fn fallback_covers_its_script() { } } +#[test] +/// A preferred Arabic file keeps the system Arabic faces from loading it again only once it is +/// loaded: one that failed to load leaves them free to load it. +fn an_unloaded_preferred_arabic_file_does_not_block_the_system_face() { + let dir = std::env::temp_dir().join(format!("gridcraft-theme-preferred-arabic-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("temp dir"); + let path = dir.join("Arabic.ttf"); + std::fs::write(&path, epaint_default_fonts::UBUNTU_LIGHT).expect("font fixture"); + let path = path.to_string_lossy().into_owned(); + let preferred = || Some((path.clone(), 0)); + let faces = || vec![("sys-arabic", vec![(path.clone(), 0)])]; + + let mut fonts = FontDefinitions::default(); + let loaded = preferred_file(&fonts, preferred()); + assert!(loaded.is_empty(), "a preferred face that did not load reserves no file"); + assert_eq!(load_faces(&mut fonts, faces(), loaded), ["sys-arabic"]); + + let mut fonts = FontDefinitions::default(); + fonts.font_data.insert(PREFERRED_ARABIC.into(), Arc::new(FontData::from_static(epaint_default_fonts::UBUNTU_LIGHT))); + let loaded = preferred_file(&fonts, preferred()); + assert_eq!(loaded, std::slice::from_ref(&path)); + assert!(load_faces(&mut fonts, faces(), loaded).is_empty(), "a loaded preferred file is not loaded a second time"); + assert!(!fonts.font_data.contains_key("sys-arabic")); + let _ = std::fs::remove_dir_all(&dir); +} + +fn with_installed_latin_font() -> std::io::Result { + let dir = std::env::temp_dir().join(format!("gridcraft-theme-latin-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir)?; + let path = dir.join("Ubuntu-Light.ttf"); + std::fs::write(&path, epaint_default_fonts::UBUNTU_LIGHT)?; + let face = crate::fonts::Face { family: "Ubuntu".into(), names: vec!["Ubuntu".into()], path, index: 0, weight: 300.0, italic: false }; + let fonts = font_definitions_impl(Language::En, vec![("Ubuntu".into(), vec![face])], true); + std::fs::remove_dir_all(&dir)?; + Ok(fonts) +} + +fn installed_latin_chains(fonts: &FontDefinitions) -> Vec<(FontFamily, Option<&Vec>)> { + [(false, false), (true, false), (false, true), (true, true)] + .into_iter() + .map(|(bold, italic)| { + let family = FontFamily::Name(crate::fonts::role_key("Ubuntu", bold, italic).into()); + let chain = fonts.families.get(&family); + (family, chain) + }) + .collect() +} + +fn chain_covers(fonts: &FontDefinitions, chain: &[String], ch: char) -> bool { + use skrifa::MetadataProvider; + chain + .iter() + .filter_map(|key| fonts.font_data.get(key)) + .any(|data| skrifa::FontRef::from_index((*data.font).as_ref(), data.index).is_ok_and(|font| font.charmap().map(ch).is_some())) +} + +#[test] +/// A workbook font with no Arabic or Hangul still shows them through the system script faces. +fn installed_latin_fonts_keep_system_script_fallbacks() { + let _g = crate::fonts::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let fonts = with_installed_latin_font().expect("Latin-only fixture"); + let loaded: Vec<&str> = + fallback_faces(HanOrder::Chinese).into_iter().map(|(key, _)| key).filter(|key| fonts.font_data.contains_key(*key)).collect(); + for (family, chain) in installed_latin_chains(&fonts) { + let chain = chain.expect("bound fixture family"); + for key in &loaded { + assert!(chain.iter().any(|k| k == key), "{family:?} lost the {key} fallback"); + } + if fonts.font_data.contains_key("sys-arabic") { + assert!(chain_covers(&fonts, chain, 'ع'), "{family:?} has no glyph for ع"); + } + } +} + +#[test] +/// The craft Arabic face reaches workbook fonts and the pinned web set, without help from system faces. +fn the_craft_arabic_face_reaches_every_chain() { + assert!(crate::craft_fonts_policy::web_allowed("Noto Sans Arabic", "Regular")); + if !craft_fonts_cover("العربية١٢٣") { + assert!(!craft_fonts_required(), "CRAFT_FONTS_REQUIRED=1 but no craft-fonts face covers Arabic"); + return; + } + let _g = crate::fonts::TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let mut fonts = with_installed_latin_font().expect("Latin-only fixture"); + fonts.font_data.retain(|key, _| !key.starts_with("sys-")); + for (family, chain) in installed_latin_chains(&fonts) { + let chain = chain.expect("bound fixture family"); + assert!("العربية١٢٣".chars().all(|ch| chain_covers(&fonts, chain, ch)), "{family:?} has no Arabic glyphs"); + } + let pinned = pinned_font_definitions(); + for (family, chain) in &pinned.families { + assert!("العربية".chars().all(|ch| chain_covers(&pinned, chain, ch)), "pinned {family:?} has no Arabic glyphs"); + } +} + +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +/// fontconfig's Arabic face, when it has Arabic letters, comes first among the Arabic fallbacks in every chain (#229). A +/// UI or cell face with Arabic of its own sits ahead of every fallback and still draws Arabic first. +fn the_fontconfig_arabic_choice_comes_first() { + assert_eq!( + parse_fc_match("/home/u/.local/share/fonts/Vazirmatn-Regular.ttf\n0\n"), + Some(("/home/u/.local/share/fonts/Vazirmatn-Regular.ttf".into(), 0)) + ); + assert_eq!( + parse_fc_match("/usr/share/fonts/noto-cjk/NotoSansCJK-Regular.ttc\n2\n"), + Some(("/usr/share/fonts/noto-cjk/NotoSansCJK-Regular.ttc".into(), 2)) + ); + assert_eq!(parse_fc_match("/usr/share/fonts/a.ttf"), Some(("/usr/share/fonts/a.ttf".into(), 0))); + assert_eq!( + parse_fc_match("/usr/share/fonts/vazirmatn/Vazirmatn[wght].ttf\n262145\n"), + Some(("/usr/share/fonts/vazirmatn/Vazirmatn[wght].ttf".into(), 1)), + "a named instance's bits are not part of the face index" + ); + assert_eq!(parse_fc_match(""), None); + assert!(!covers_arabic(epaint_default_fonts::UBUNTU_LIGHT, 0), "a Latin-only face is not an Arabic choice"); + if fontconfig_arabic().is_some() { + let fonts = font_definitions(); + for (family, chain) in fonts.families.iter().filter(|(_, chain)| chain.iter().any(|key| key.starts_with("sys-"))) { + let preferred = chain.iter().position(|key| key == PREFERRED_ARABIC); + let other_arabic = chain.iter().position(|key| key == "sys-arabic" || key.starts_with("craft-")); + assert!(preferred.is_some(), "{family:?} lacks the preferred Arabic face"); + assert!(other_arabic.is_none_or(|other| preferred < Some(other)), "{family:?} puts another Arabic face first"); + } + if let (Some(preferred), Some(system)) = (fonts.font_data.get(PREFERRED_ARABIC), fonts.font_data.get("sys-arabic")) { + assert!((*preferred.font).as_ref() != (*system.font).as_ref(), "the system Arabic fallback is a different file from the preferred face"); + } + } +} + #[test] fn chromeos_font_layout_is_probed() { // #1762: Crostini mounts the host's fonts under /usr/share/fonts/chromeos, one folder per @@ -229,6 +363,7 @@ fn chromeos_font_layout_is_probed() { 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(arabic(), &format!("{cros}/noto/NotoSansArabic-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"))); diff --git a/docs/gaps.md b/docs/gaps.md index 8879e967d..3fda49d2b 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -55,7 +55,9 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- ### 5. Text in non-Latin scripts doesn't render or export (L) - **Missing:** CJK font fallback, complex shaping (Arabic, Devanagari, Thai), bidi, non-Latin PDF. -- **Evidence:** system-font fallback for CJK, Thai and Hebrew landed in #38 (closed #32); #142 +- **Evidence:** system-font fallback for CJK, Thai and Hebrew landed in #38 (closed #32), and for + Arabic (system, fontconfig and craft-fonts faces); a UI font with Arabic of its own (DejaVu Sans + in the Flatpak) still draws interface Arabic ahead of fontconfig's choice (#229); #142 (Chinese text) awaits confirmation; #54 (symbols as boxes on Fedora); no shaping for Arabic or Devanagari; PDF writer uses 14 standard fonts only. - **Impact:** blocks most of the world's users even with an English UI. @@ -193,6 +195,7 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- | 2026-11-01 | minor | §4: chart data tables implemented for column, bar, line, area and compatible combo charts, with legend keys and border settings, live source values, XLSX round-trip and print/PDF; other chart gaps remain. | | 2026-10-31 | minor | §13: Solver Options (Max Evaluations, Integer Optimality and the Evolutionary options), saved with the model | | 2026-10-31 | minor | §13: Solver Load/Save Model (`data.solverSaveModel`, `data.solverLoadModel`) and Reset All (`data.solverReset`), with buttons in the Solver Parameters dialog | +| 2026-10-26 | minor | §5: Arabic text renders in cells and the interface: a system Arabic face (Tahoma, Arial or Segoe UI on Windows, Geeza Pro on macOS, Noto Sans or Naskh Arabic on Linux) joins the script fallbacks, fontconfig's Arabic choice, when it has Arabic letters, leads the Arabic fallbacks and draws Arabic wherever the UI or cell font has none, such as the default cell fonts (part of #229; a UI font with Arabic of its own, such as DejaVu Sans in the Flatpak, still draws interface Arabic, left for a follow-up), and craft-fonts' Noto Sans Arabic is a required face allowed on the web. Workbook fonts without Arabic keep every script fallback. Shaping and bidirectional layout remain separate work | | 2026-10-24 | minor | §8: PivotTable Enable show details: `pivot.options` `enableShowDetails` turns Show Details off for a PivotTable, read and written in XLSX as `enableDrill` | | 2026-10-23 | minor | §8: PivotTable Show Details: double-clicking a value cell lists its source records on a new sheet (#2364) | | 2026-10-22 | minor | §20: a native window per workbook window, New Window on the same workbook and Arrange All on the desktop; left: Arrange All across monitors, the web's one window, pen pressure (estimate 10–14 h → 5–8 h) | diff --git a/docs/localization-parity.md b/docs/localization-parity.md index d504c19ea..6bc96187c 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-27 · **Change:** minor (number formats honour the `B1`/`B2` calendar prefixes, checked against Excel; earlier: missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks); previously minor (AutoFit estimates Arabic text width from measured glyph widths); previously minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (Arabic font fallback; missing translations are listed by `cargo xtask i18n-report` and never fail a check); previously minor (number formats honour the `B1`/`B2` calendar prefixes, checked against Excel; earlier: missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks); previously minor (AutoFit estimates Arabic text width from measured glyph widths); previously minor (missing translations are listed by `cargo xtask i18n-report` and never fail a check; Arabic text sorts by its letters, not its vowel marks) · **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, @@ -60,7 +60,9 @@ right-to-left sheets and full complex-script support. Part of `/run/host/fonts`, `/run/host/local-fonts` and `/run/host/user-fonts` layouts, so installed Noto CJK and WenQuanYi faces can render Chinese without a broad filesystem grant. This does not change the installed-font catalogue or add bundled fonts. Lao falls back to Lao Sangam MN / Lao MN (macOS), - Leelawadee UI / DokChampa (Windows) or Noto Sans Lao / Phetsarath OT (Linux). There is no complex-text shaping: Arabic and + Leelawadee UI / DokChampa (Windows) or Noto Sans Lao / Phetsarath OT (Linux). Arabic falls back to a system Arabic face, fontconfig's + Arabic choice or craft-fonts' Noto Sans Arabic; fontconfig's choice leads these fallbacks, but a UI font with Arabic of its + own (DejaVu Sans in the Flatpak) still draws interface Arabic (#229). 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`, `.xls` WINDOW2 fRightToLeft) mirrors the grid (#146): column A and the row headers on the right, the vertical scrollbar on the left; hit-testing, @@ -124,6 +126,7 @@ language (none so far) and of localized function names. | Date | Change | Summary | |---|---|---| +| 2026-10-26 | minor | Arabic text renders in cells and the interface: a system Arabic face (Tahoma, Arial or Segoe UI on Windows, Geeza Pro on macOS, Noto Sans or Naskh Arabic on Linux) joins the script fallbacks, fontconfig's Arabic choice, when it has Arabic letters, leads the Arabic fallbacks and draws Arabic wherever the UI or cell font has none, such as the default cell fonts (part of #229; a UI font with Arabic of its own, such as DejaVu Sans in the Flatpak, still draws interface Arabic, left for a follow-up), and craft-fonts' Noto Sans Arabic is a required face allowed on the web. Workbook fonts without Arabic keep every script fallback. Shaping and bidirectional layout remain separate work | | 2026-10-27 | minor | Number formats honour the `B1`/`B2` calendar prefixes: never shown; `B2` renders year, month, day and Arabic month and weekday names in the tabular Hijri calendar (1 Jan 2001 = 6 Shawwal 1421; leap years 2, 5, 7, 10, 13, 15, 18, 21, 24, 26 and 29 of the 30-year cycle, which is what Excel shows). Checked against Excel 16.0 (build 20527, en-US) on every serial and about 1,700 format/value pairs: a locale or currency tag before `B2` keeps the date Gregorian, a misplaced `B` and digit or a prefix before a number format is rejected, serial 60 has no Hijri date, serial 0 shows as serial 1, two-word month names use a no-break space. Not matched: Excel's weekday/month-name mix-ups in one `B2` section and its weekday-only quirks | | 2026-10-27 | minor | AutoFit column width, wrapped-row height and PivotTable column fit estimate Arabic letters at ~0.48 em (measured 0.36–0.53 em shaped, Noto Sans/Naskh Arabic and DejaVu Sans) instead of 1 em, Arabic-Indic digits as wide as ASCII digits, and give vowel marks and direction marks no width; vowelled Arabic no longer fits ~3.5× too wide | | 2026-10-23 | minor | Missing translations are reported, never a failure: `cargo xtask i18n-report` prints, per language, the strings other catalogs translate and it lacks (plus Spanish function help rows keyed by English the function catalog no longer has) and always passes; `cargo xtask ci` runs it first. Coverage and per-language parity tests and tests of one language's exact wording are gone; UI tests take the expected text from the catalog. Duplicate-key, placeholder, context-prefix and line-shape checks stay | From e9addf52fe68847e6c1310fcfa5d42b7aa8b9650 Mon Sep 17 00:00:00 2001 From: Mouhand Kaddo Date: Mon, 9 Nov 2026 18:55:43 +0400 Subject: [PATCH 8/8] Arabic bidi layout and cell reading direction in the grid and editors (#947) * M12.AR4: preserve Arabic logical text in bidi display and editing * M12.AR4: drop the stale "no bidi" note from the RTL sheet status * M12.AR4: open the Arabic editors and place the caret through key and pointer events in tests * M12.AR4: skip the conditional-format lookup in the formula bar unless a rule sets reading order * M12.AR4: test that the caret after typed Arabic is drawn at the text's left end (#1070) * M12.AR4: pass the theme body font in the reading-order style test * M12.AR4: adapt the Arabic UI tests to egui 0.37 (row direction, editor ids, Ctrl+End to the logical end) * M12.AR4: read the active sheet through Workbook::active_sheet() in the formula bar main made `Workbook::active_sheet` an `ActiveSheet` (shared between windows on one workbook); the formula bar's reading-order lookup takes the index from the accessor. Co-Authored-By: Claude Opus 5.5 * M12.AR4: keep explicit Left on wrapped right-to-left cells and trailing spaces in the editor Wrapped cell text lays out every row from the left when the painted alignment is Left, instead of aligning right-to-left rows to their start. The cell and formula bar editor keep trailing whitespace in their layout so a right-to-left entry ending in a space keeps the caret inside the text. * M12.AR4: keep Right and General on right-to-left sheets aligned right in wrapped cells * M12.AR4: keep right-aligned Arabic with a marked first letter inside its cell --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: FredApps --- Cargo.lock | 1 + ROADMAP.md | 7 +- crates/engine/src/cf.rs | 25 +++ crates/engine/src/cmd/format.rs | 2 +- crates/engine/src/tests.rs | 38 ++++ crates/model/src/features.rs | 24 +++ crates/model/src/lib.rs | 4 +- crates/model/src/style.rs | 30 +++ crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/src/dialogs/format_cells.rs | 13 +- crates/ui-egui/src/formula_bar.rs | 33 ++- crates/ui-egui/src/grid/cell_paint.rs | 3 + crates/ui-egui/src/grid/quadrant.rs | 29 ++- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/text_direction.rs | 32 +++ crates/ui-egui/tests/arabic_bidi.rs | 228 +++++++++++++++++++++ crates/ui-egui/tests/arabic_editing.rs | 131 ++++++++++++ crates/xlsx/src/styles.rs | 62 +++++- crates/xlsx/src/tests/roundtrip.rs | 10 +- crates/xlsx/src/xls/styles.rs | 1 + docs/file-format-parity.md | 3 + docs/gaps.md | 6 + docs/localization-parity.md | 13 +- 23 files changed, 673 insertions(+), 24 deletions(-) create mode 100644 crates/ui-egui/src/text_direction.rs create mode 100644 crates/ui-egui/tests/arabic_bidi.rs create mode 100644 crates/ui-egui/tests/arabic_editing.rs diff --git a/Cargo.lock b/Cargo.lock index cb783e546..543cddfe1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2197,6 +2197,7 @@ dependencies = [ "skrifa", "sys-locale", "toml", + "unicode-bidi", "web-sys", "web-time", "zbus", diff --git a/ROADMAP.md b/ROADMAP.md index f12cd112a..5906fda99 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -43,8 +43,7 @@ long `.xls` track, so about half parallelizes (≈30–50 wall-clock hours with Why alpha: all six core workflows in the [alpha gate](docs/roadmap.md#alpha-gate) pass end to end on macOS with save and reopen, and the core workflow works end to end (open an XLSX, enter formulas and data, format, sort and filter, chart, pivot, print to PDF, save XLSX), but users still hit blocking gaps on real -files: 100 MB workbooks don't open (#175), there's no `.xls` (#121), Arabic and Devanagari text -isn't shaped (Chinese text still reported in #142), charts lose their type and options on round trip, and +files: 100 MB workbooks don't open (#175), there's no `.xls` (#121), Devanagari text needs shaping verification (Chinese text still reported in #142), charts lose their type and options on round trip, and nobody has yet confirmed that Excel opens our output without a repair prompt across a real corpus. Beta needs those closed (see [beta gates](docs/roadmap.md#beta-gates)). @@ -105,7 +104,7 @@ Localized German formulas and numbers are in PR #163. Detail: [localization-pari | Simplified Chinese | zh-Hans | menus only | ~12% | | Spanish | es | none | 0% | | Hindi | hi | none | 0% | -| Arabic | ar | none (no RTL layout, no shaping) | 0% | +| Arabic | ar | catalog pending; shared shaping/bidi works | 0% | | French | fr | none | 0% | | Portuguese | pt-BR | menus, File view, dialogs | ~43% | | Indonesian | id | none | 0% | @@ -149,6 +148,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | 2026-10-31 | CSV and text open detects each column's date order: a value that reads only day first (`31/05/2026`) makes the column day first, a value that reads only month first (`12/31/2026`) makes it month first, an all-ambiguous column keeps month first with a warning, and a column with both kinds stays text with a warning (#2765) | | 2026-10-31 | Recently Used lists the functions most recently entered in a formula from any source (ribbon, control channel, MCP, scripts), kept with the preferences and returned by `formulas.recentlyUsed`; the Insert Function dialog takes Excel's title and a working category list (Most Recently Used, All, each category) | | 2026-10-31 | Formulas › Function Library as a big Insert Function button, the AutoSum split button and three columns of labelled small buttons, each category book in its own colour; category menus end with Insert Function… on that category; More Functions cascades into the remaining categories; Home › Editing shares the AutoSum menu; the MR key tip opens Recently Used | +| 2026-10-26 | Arabic bidi display/editing preserve logical source text; per-cell reading direction is editable and survives undo, XLSX and conditional-format priority | | 2026-10-26 | Arabic text shows in cells and the interface through a system, fontconfig or craft-fonts Arabic face; workbook fonts keep their script fallbacks | | 2026-10-25 | AutoSum over a selection does what Excel does: a selected row gets one total to its right (in its last cell if that is empty), not a one-cell total under each cell; a block whose last column is empty gets each row's total there, one whose last row is empty each column's, and with both the corner gets the total of the block; otherwise totals go under each column as before | | 2026-10-24 | Macro interface ([macros.md](docs/macros.md#in-the-window)): an Automate › Macros group (Record Macro, which turns into Stop Recording, Use Relative References, Run Macro with recent macros); "Recording · N steps" with Stop in the status bar; a Record Macro dialog (checked name, free Ctrl+Shift shortcut, description); the Macros dialog lists macros beside Automate scripts and a VBA project, with Run, Step Into, Edit, Create, Delete and Options; Ctrl+Shift+letter macro shortcuts; questions that offer Run, Overwrite, Shift Down or Shift Right; a step editor with a breakpoint gutter, drag to reorder, a command picker, reference boxes that switch between relative tokens and absolute references, Inputs and Output tabs, a debug bar (Check, Run, Step, Continue, Run to Here, Stop with keep or undo) and grid overlays for inputs, writes, conflicts and the current step | @@ -277,6 +277,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | 2026-10-31 | minor | File-page text fields keep the view open when Esc cancels an edit (#3035) | | 2026-10-31 | minor | Progress log: Recently Used and Insert Function categories | | 2026-10-31 | minor | Progress log: Function Library | +| 2026-10-26 | minor | Progress log: Arabic bidi display and editing | | 2026-10-26 | minor | Progress log: Arabic font fallback | | 2026-10-25 | minor | Progress log: AutoSum over a selected row, and a selection's empty last row and column | | 2026-10-24 | minor | Macro interface on the Automate tab; progress log row | diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs index 38f5bb1fe..0af7a9df5 100644 --- a/crates/engine/src/cf.rs +++ b/crates/engine/src/cf.rs @@ -368,6 +368,9 @@ impl CfCache { /// Adds the parts `lower` sets that `style` leaves unset (differential styles: default parts /// are unset). fn layer_under(style: &mut Style, lower: &Style) { + if style.align.reading_order.is_none() { + style.align.reading_order = lower.align.reading_order; + } if style.fill.pattern == PatternType::None { style.fill = lower.fill; } @@ -417,6 +420,28 @@ mod tests { use super::*; + #[test] + fn reading_order_respects_conditional_rule_priority_and_explicit_context() { + for (higher, expected) in [ + (json!({"font": {"bold": true}}), "RightToLeft"), + (json!({"align": {"reading_order": "Context"}}), "Context"), + (json!({"align": {"reading_order": "LeftToRight"}}), "LeftToRight"), + ] { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "مرحبا 123 ABC"})).unwrap(); + for style in [json!({"align": {"reading_order": "RightToLeft"}}), higher] { + s.execute("home.conditionalFormat", json!({"range": "A1", "rule": {"type": "noBlanks", "style": style}})).unwrap(); + } + let wb = &s.doc().unwrap().wb; + let (reopened, _) = gridcraft_xlsx::read_xlsx(&gridcraft_xlsx::write_xlsx(wb).unwrap()).unwrap(); + for wb in [wb, &reopened] { + let style = CfCache::new().look(wb, 0, CellRef::new(0, 0)).unwrap().style.unwrap(); + assert_eq!(serde_json::to_value(style.align).unwrap()["reading_order"], expected); + } + } + } + #[test] fn every_matching_rule_applies() { let mut s = crate::Session::new(); diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index d5eac5b5d..aa2638801 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -85,7 +85,7 @@ pub fn specs() -> Vec { "Format Cells…", ["Home", "Cells", "Format"], Some("Cmd+1"), - "{range?, style: {font?, fill?, borders?, align?, numFmt?, protection?}} (partial Style JSON merged in)", + "{range?, style: {font?, fill?, borders?, align?: {reading_order?: Context|LeftToRight|RightToLeft|null, ...}, numFmt?, protection?}} (partial Style JSON merged in; null clears reading_order)", has_doc, format_cells ), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index bf36bd15b..fcaf19c36 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -18,6 +18,44 @@ fn v(s: &Session, a: &str) -> Value { s.doc().unwrap().wb.active().unwrap().value(CellRef::parse(a).unwrap()) } +#[test] +fn format_cells_reading_order_survives_undo_and_xlsx() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "مرحبا 123 ABC"})).unwrap(); + s.execute("home.bold", json!({"range": "A1"})).unwrap(); + s.execute("home.formatCells", json!({"range": "A1", "style": {"align": {"reading_order": "RightToLeft"}}})).unwrap(); + let alignment = |s: &Session| { + let d = s.doc().unwrap(); + let style = d.wb.styles.get(d.wb.active().unwrap().style_id(CellRef::new(0, 0))); + assert!(style.font.bold); + serde_json::to_value(style.align).unwrap() + }; + assert_eq!(alignment(&s)["reading_order"], "RightToLeft"); + s.execute("edit.undo", json!({})).unwrap(); + assert!(alignment(&s)["reading_order"].is_null()); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(alignment(&s)["reading_order"], "RightToLeft"); + for order in [json!("invalid"), json!(2)] { + assert!(s.execute("home.formatCells", json!({"range": "A1", "style": {"align": {"reading_order": order}}})).is_err()); + assert_eq!(alignment(&s)["reading_order"], "RightToLeft"); + } + let wb = &s.doc().unwrap().wb; + let (reopened, _) = gridcraft_xlsx::read_xlsx(&gridcraft_xlsx::write_xlsx(wb).unwrap()).unwrap(); + let cell = CellRef::new(0, 0); + assert_eq!(reopened.styles.get(reopened.active().unwrap().style_id(cell)), wb.styles.get(wb.active().unwrap().style_id(cell))); + assert_eq!(reopened.active().unwrap().value(cell), Value::text("مرحبا 123 ABC")); + for order in ["LeftToRight", "Context"] { + s.execute("home.formatCells", json!({"range": "A1", "style": {"align": {"reading_order": order}}})).unwrap(); + assert_eq!(alignment(&s)["reading_order"], order); + } + s.execute("home.formatCells", json!({"range": "A1", "style": {"align": {"reading_order": null}}})).unwrap(); + assert!(alignment(&s)["reading_order"].is_null()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(alignment(&s)["reading_order"], "Context"); + s.execute("edit.redo", json!({})).unwrap(); + assert!(alignment(&s)["reading_order"].is_null()); +} + #[test] fn entering_empty_cells_preserves_whole_sheet_formatting() { let mut s = s(); diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs index 06e68839f..8ab74606a 100644 --- a/crates/model/src/features.rs +++ b/crates/model/src/features.rs @@ -230,6 +230,30 @@ pub enum CfRule { }, } +impl CfRule { + /// The differential style a matching rule applies; colour scales, data bars and icon sets have none. + pub fn style(&self) -> Option<&Style> { + match self { + CfRule::CellIs { style, .. } + | CfRule::Expression { style, .. } + | CfRule::ContainsText { style, .. } + | CfRule::NotContainsText { style, .. } + | CfRule::BeginsWith { style, .. } + | CfRule::EndsWith { style, .. } + | CfRule::Blanks { style } + | CfRule::NoBlanks { style } + | CfRule::Errors { style } + | CfRule::NoErrors { style } + | CfRule::Duplicate { style } + | CfRule::Unique { style } + | CfRule::Top10 { style, .. } + | CfRule::AboveAverage { style, .. } + | CfRule::TimePeriod { style, .. } => Some(style), + CfRule::ColorScale { .. } | CfRule::DataBar { .. } | CfRule::IconSet { .. } => None, + } + } +} + /// Number of icons in an icon set (`3Arrows` → 3, `5Quarters` → 5), always 3 to 5. pub fn icon_count(set: &str) -> usize { set.chars().next().and_then(|c| c.to_digit(10)).unwrap_or(3).clamp(3, 5) as usize diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index 35cfa315f..b1f875b3c 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -26,8 +26,8 @@ pub use python::{PyCode, PyOutput}; pub use sheet::{DEFAULT_COL_WIDTH, DEFAULT_ROW_HEIGHT, LineIndex, Sheet, SheetUid, SheetViewMode, Visibility, points_to_chars}; pub use store::CellStore; pub use style::{ - Alignment, BorderLine, BorderStyle, Borders, Color, Fill, Font, HAlign, NumFmt, PatternType, Style, StyleId, StyleTable, Theme, Underline, - VAlign, VertAlign, + Alignment, BorderLine, BorderStyle, Borders, Color, Fill, Font, HAlign, NumFmt, PatternType, ReadingOrder, Style, StyleId, StyleTable, Theme, + Underline, VAlign, VertAlign, }; pub use workbook::{ CalcMode, CalcSettings, DefinedName, DocProps, EQUATION_BUDGET_MESSAGE, MAX_EQUATION_LAYOUT_BYTES, PhoneticProps, PhoneticRun, PhoneticString, diff --git a/crates/model/src/style.rs b/crates/model/src/style.rs index d2f10780e..9ad61abec 100644 --- a/crates/model/src/style.rs +++ b/crates/model/src/style.rs @@ -350,6 +350,14 @@ pub enum VAlign { Distributed, } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum ReadingOrder { + #[default] + Context, + LeftToRight, + RightToLeft, +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct Alignment { pub h: HAlign, @@ -359,6 +367,9 @@ pub struct Alignment { pub indent: u8, /// Degrees -90..=90, or 255 for vertical stacked text. pub rotation: i16, + /// Unspecified inherits in differential styles and uses context for ordinary cells. + #[serde(default)] + pub reading_order: Option, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -488,6 +499,25 @@ static DEFAULT_STYLE: std::sync::LazyLock