From 4d2e40155adc9ba6772a686f66650fd377772ce9 Mon Sep 17 00:00:00 2001 From: Nick Nyssen <314538108+amoisttowe1@users.noreply.github.com> Date: Tue, 20 Oct 2026 03:00:33 -0700 Subject: [PATCH] M14: correct two drifted comments and cite why candidates are optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pre-publish review caught both comment errors, and Apple's own documentation settles the rest of it: - installed_faces_switched_off said "both loader tests". It now guards three sites — two loader tests plus the macOS oracle — ever since 21dc64e added the third call without touching this sentence. - MACOS_FONT_DIRS claimed the Telugu, Sinhala and Tamil lists "still name one folder each". They do not: Telugu names /System/Library/Fonts, Supplemental/ and /Library/Fonts (four paths), Tamil three, Sinhala two. The accurate statement is that no family in those lists is looked up in both system folders, which is the property Devanagari now has and they do not. Their first choices all exist on macOS 26.6, verified again here. - The oracle's doc comment now records the authority for treating listed faces as optional: Apple's "Fonts included with macOS Tahoe" page (support.apple.com/122869) is titled "Installed or downloadable fonts" and states that most install automatically while others download via Font Book — without marking which entries are which. Absence of a face is therefore expected on some Macs, which is exactly what the reported-not-failed behaviour accommodates. Comment-only change: fmt --all clean, clippy -D warnings quiet, system_fonts 13 passed with and without PDFCRAFT_SYSTEM_FONTS=0. --- crates/ui-egui/src/system_fonts.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/system_fonts.rs b/crates/ui-egui/src/system_fonts.rs index a9c71ccecd..add591f05c 100644 --- a/crates/ui-egui/src/system_fonts.rs +++ b/crates/ui-egui/src/system_fonts.rs @@ -119,9 +119,10 @@ fn script_fallback(candidates: &[PathBuf], probe: char) -> Option> /// below. On that machine each family below lives in exactly one of the two folders, so their /// order is a deterministic choice rather than a tie-break that was ever exercised. /// -/// Only [`macos_devanagari_candidates`] uses this today; the Telugu, Sinhala and Tamil lists still -/// name one folder each. Their first choices all exist on macOS 26.6, so nothing is dropping there -/// now — the same expansion would fit them, and is left for a change that tests it. +/// Only [`macos_devanagari_candidates`] uses this today: in the Telugu, Sinhala and Tamil lists each +/// family is named once, in a single folder (some of those lists also name `/Library/Fonts` for the +/// Unicode face). Their first choices all exist on macOS 26.6, so nothing is dropping there now — the +/// same two-folder expansion would fit them, and is left for a change that tests it. const MACOS_FONT_DIRS: [&str; 2] = ["/System/Library/Fonts", "/System/Library/Fonts/Supplemental"]; /// The Devanagari families this list tries, best first. Spelled as the files are on disk — @@ -430,8 +431,8 @@ mod tests { } /// `script_fallback` honours the documented installed-font switch, under which it returns - /// nothing before order is ever consulted — so both loader tests skip when it is off, the way - /// `tests/fonts.rs` skips for the same reason. Without this the suite goes red in exactly the + /// nothing before order is ever consulted — so both loader tests and the macOS oracle skip when + /// it is off, the way `tests/fonts.rs` skips for the same reason. Without this the suite goes red in exactly the /// mode `cargo xtask screenshots` uses. fn installed_faces_switched_off() -> bool { std::env::var_os("PDFCRAFT_SYSTEM_FONTS").is_some_and(|v| v == "0") @@ -551,7 +552,11 @@ mod tests { /// reachable from the candidate list, and something in the list covers Devanagari. A family /// Apple no longer ships is reported rather than failed — this list exists precisely for the case /// where one of them is missing, with one exception: if *nothing* here covers Devanagari the test - /// fails loudly, because a machine with no face to check cannot validate the list at all. CI has no macOS runner (`.github/workflows/ci.yml` + /// fails loudly, because a machine with no face to check cannot validate the list at all. That + /// leniency is not a guess: Apple's "Fonts included with macOS Tahoe" page (support.apple.com/122869) + /// titles its list "Installed or downloadable fonts" and says most install automatically while + /// others download on demand via Font Book — without marking which is which, so no entry in these + /// lists can be assumed present on a given Mac. CI has no macOS runner (`.github/workflows/ci.yml` /// keeps the org's Mac slots for release builds), so this runs on contributors' and /// maintainers' machines. #[cfg(target_os = "macos")]