From 1d7c8f93ab975c5860548f483999c1fe4541dcf3 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 20:35:58 -0500 Subject: [PATCH] Export for Screens: name patterns and taken names take linear work The file name pattern searched the rest of the pattern for a closing bracket at every opening bracket, so a pattern of many unmatched brackets took quadratic time to render, in the export and its live preview. A token's name never holds a bracket, so the search now stops at the next opening one. Output naming tried -2, -3 and so on from the start for every file given a taken name, so one name pattern over many artboards rebuilt and looked up every number used before for each file. It now goes on from the last number given for that name, and remembers which file got a name from it. --- crates/engine/src/cmd/fileio/naming.rs | 16 +++++++++-- crates/engine/src/cmd/fileio/screens.rs | 36 +++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/fileio/naming.rs b/crates/engine/src/cmd/fileio/naming.rs index e701a03d05..12cf26ca0b 100644 --- a/crates/engine/src/cmd/fileio/naming.rs +++ b/crates/engine/src/cmd/fileio/naming.rs @@ -70,9 +70,11 @@ pub fn render(template: &str, p: &NameParts<'_>) -> String { let mut out = String::with_capacity(t.len() + 16); let mut rest = t; while let Some(c) = rest.chars().next() { - // Bracketed tokens; an unknown one stays as written, with `-` for its brackets. + // Bracketed tokens; an unknown one stays as written, with `-` for its brackets. A token's + // name has no `<`, so the search for its `>` stops at the next `<`: each part of the + // pattern is searched once, not once for every `<` before it. if c == '<' - && let Some(end) = rest.find('>') + && let Some(end) = rest.get(1..).and_then(|r| r.find(['<', '>'])).map(|i| i + 1).filter(|&i| rest.as_bytes().get(i) == Some(&b'>')) && let Some(value) = rest.get(1..end).and_then(|name| token(name, p)) { out.push_str(&value); @@ -155,6 +157,16 @@ mod tests { assert_eq!(render("#", &NameParts { counter: 12345, ..parts() }), "12345"); } + /// Many `<` without a `>`, or before one far away, are each looked at once. + #[test] + fn unmatched_brackets_are_read_in_one_pass() { + let p = parts(); + let n = 200_000; + assert_eq!(render(&"<".repeat(n), &p), "-".repeat(n)); + assert_eq!(render(&format!("{}", "<".repeat(n)), &p), format!("{}Dark", "-".repeat(n))); + assert_eq!(render(">", &p), "-aDark-"); + } + #[test] fn dates_are_strict_about_hyphens_and_days() { for bad in ["2026-1-011", "2-0-2-6-1-0-1-6", "2026--10--16", "2026-02-31", "2026-10-16-", "202610-16"] { diff --git a/crates/engine/src/cmd/fileio/screens.rs b/crates/engine/src/cmd/fileio/screens.rs index 384f731aa3..19381709ab 100644 --- a/crates/engine/src/cmd/fileio/screens.rs +++ b/crates/engine/src/cmd/fileio/screens.rs @@ -521,6 +521,11 @@ struct Output { folders: std::collections::HashMap, /// Sub-folders already made, by their [`fold`]ed names. made: HashSet, + /// For each [`fold`]ed name asked for, the number to try next: many files given one name + /// would otherwise try `-2`, `-3`… from the start each time. + next: std::collections::HashMap, + /// Each [`fold`]ed name asked for with the file (`key`) that got a name from it. + claimed: HashSet<(String, String)>, /// `(relative name, bytes)` kept to return or zip (empty when written to a folder). kept: Vec<(String, Vec)>, /// The paths written. @@ -540,8 +545,14 @@ impl Output { } None => name.to_string(), }; + let asked = (fold(&name), key.to_string()); + // The same file under the same name again: it has its name already. + if self.claimed.contains(&asked) { + return None; + } let (base, ext) = name.rsplit_once('.').map_or((name.as_str(), ""), |(b, e)| (b, e)); - for n in 1.. { + let start = self.next.get(&asked.0).copied().unwrap_or(1); + for n in start.. { let candidate = if n == 1 { name.to_string() } else if ext.is_empty() { @@ -554,6 +565,8 @@ impl Output { Some(_) => continue, None => { self.taken.insert(fold(&candidate), key.to_string()); + self.next.insert(asked.0.clone(), n + 1); + self.claimed.insert(asked); return Some(candidate); } } @@ -1280,7 +1293,26 @@ pub(super) fn export_settings(s: &mut Session, _: &Value) -> Result { #[cfg(test)] mod tests { - use super::{ScreenSize, safe_file_name}; + use super::{Output, ScreenSize, safe_file_name}; + + /// Files given one name are numbered in turn, each numbering going on from the last one (so + /// thousands of them don't each try every number used before), and a file asking again for + /// the name it got is told it has it. + #[test] + fn taken_names_are_numbered_from_the_last_number() { + let mut out = Output::default(); + assert_eq!(out.claim("a.png", "1").as_deref(), Some("a.png")); + assert_eq!(out.claim("A.png", "2").as_deref(), Some("A-2.png")); + assert_eq!(out.claim("a-3.png", "x").as_deref(), Some("a-3.png")); + assert_eq!(out.claim("a.png", "3").as_deref(), Some("a-4.png")); + assert_eq!(out.claim("a.png", "2"), None); + assert_eq!(out.claim("a.png", "1"), None); + assert_eq!(out.claim("sub/a.png", "1").as_deref(), Some("sub/a.png")); + for k in 0..20_000 { + assert!(out.claim("same.png", &k.to_string()).is_some()); + } + assert_eq!(out.claim("same.png", "last").as_deref(), Some("same-20001.png")); + } #[test] fn size_parse_refuses_signs_and_exponents() {