diff --git a/crates/ui-egui/src/file_dialog.rs b/crates/ui-egui/src/file_dialog.rs index b314aba3..1b794b57 100644 --- a/crates/ui-egui/src/file_dialog.rs +++ b/crates/ui-egui/src/file_dialog.rs @@ -192,8 +192,10 @@ impl PhotocraftApp { }) } - /// Opens each picked file's contents as its own document, reporting each failure. - fn open_contents(&mut self, files: Vec<(String, Vec)>) { + /// Opens each picked file's contents as its own document, in name order (like + /// [`Self::open_paths`]), reporting each failure. + fn open_contents(&mut self, mut files: Vec<(String, Vec)>) { + files.sort_by(|a, b| crate::file_open::natural_cmp(&a.0, &b.0)); for (name, bytes) in files { if let Err(e) = self.open_bytes(&name, &bytes) { self.open_failed(&name, &e); diff --git a/crates/ui-egui/src/file_open.rs b/crates/ui-egui/src/file_open.rs index 8c5f81c4..9b052dda 100644 --- a/crates/ui-egui/src/file_open.rs +++ b/crates/ui-egui/src/file_open.rs @@ -103,17 +103,31 @@ impl PhotocraftApp { } /// Open several files, reporting each failure to the user instead of stopping. Relative paths - /// are resolved against the working directory so the remembered path stays valid. Returns how - /// many opened. + /// are resolved against the working directory so the remembered path stays valid. The files + /// open in name order ([`natural_cmp`]: `File_2` before `File_10`), and their tabs keep that + /// order however long each takes to open in the background (#3248). Returns how many opened. pub fn open_paths(&mut self, paths: &[String]) -> usize { + let mut paths: Vec = paths.iter().map(|p| absolute(p)).collect(); + paths.sort_by(|a, b| natural_cmp(a, b)); + let batch = crate::jobs_ui::begin_batch(self); let mut opened = 0; - for p in paths { - let abs = absolute(p); - match self.open_path(&abs) { - Ok(_) => opened += 1, - Err(e) => self.open_failed(&display_name(p), &e), + for (seq, abs) in paths.iter().enumerate() { + let order = crate::jobs_ui::OpenOrder { batch, seq }; + let pending = self.jobs.opens.len(); + match self.open_path(abs) { + Ok(_) => { + opened += 1; + // A background open's tab takes its place when it finishes; a document opened + // at once is in place now. + match self.jobs.opens.get_mut(pending) { + Some(tab) => tab.order = Some(order), + None => crate::jobs_ui::opened_in_order(&mut self.jobs, order), + } + } + Err(e) => self.open_failed(&display_name(abs), &e), } } + crate::jobs_ui::end_batch(&mut self.jobs); opened } @@ -230,6 +244,47 @@ impl PhotocraftApp { } } +/// Order names like a file browser does: runs of digits by their value (`File_2` before +/// `File_10`), everything else case-insensitively; names equal that way fall back to the exact +/// text so the order is total. +pub(crate) fn natural_cmp(a: &str, b: &str) -> std::cmp::Ordering { + use std::cmp::Ordering; + let (mut x, mut y) = (a.chars().peekable(), b.chars().peekable()); + loop { + match (x.peek().copied(), y.peek().copied()) { + (None, None) => return a.cmp(b), + (None, Some(_)) => return Ordering::Less, + (Some(_), None) => return Ordering::Greater, + (Some(c), Some(d)) if c.is_ascii_digit() && d.is_ascii_digit() => { + let (m, n) = (digit_run(&mut x), digit_run(&mut y)); + let (m, n) = (m.trim_start_matches('0'), n.trim_start_matches('0')); + match m.len().cmp(&n.len()).then_with(|| m.cmp(n)) { + Ordering::Equal => {} + o => return o, + } + } + (Some(c), Some(d)) => { + x.next(); + y.next(); + match c.to_lowercase().cmp(d.to_lowercase()) { + Ordering::Equal => {} + o => return o, + } + } + } + } +} + +/// Take the run of ASCII digits at the front of `it`. +fn digit_run(it: &mut std::iter::Peekable) -> String { + let mut run = String::new(); + while let Some(c) = it.peek().copied().filter(char::is_ascii_digit) { + run.push(c); + it.next(); + } + run +} + #[cfg(not(target_arch = "wasm32"))] fn absolute(p: &str) -> String { std::path::absolute(p).map(|a| a.to_string_lossy().to_string()).unwrap_or_else(|_| p.to_string()) diff --git a/crates/ui-egui/src/file_open/tests.rs b/crates/ui-egui/src/file_open/tests.rs index 995eef15..3e6bde28 100644 --- a/crates/ui-egui/src/file_open/tests.rs +++ b/crates/ui-egui/src/file_open/tests.rs @@ -705,3 +705,73 @@ fn open_documents_as_tabs_preference_controls_floating_windows() { app.open_bytes("b.psd", b"x").unwrap(); assert_eq!(app.ui.windows.len(), 2); } + +#[test] +fn natural_order_counts_numbers_by_value() { + use std::cmp::Ordering; + assert_eq!(natural_cmp("File_2.psd", "File_10.psd"), Ordering::Less); + assert_eq!(natural_cmp("file_1.psd", "File_2.psd"), Ordering::Less, "case-insensitive"); + assert_eq!(natural_cmp("File_03.psd", "File_10.psd"), Ordering::Less, "leading zeros"); + assert_eq!(natural_cmp("a.psd", "a.psd"), Ordering::Equal); + assert_eq!(natural_cmp("a", "ab"), Ordering::Less); + assert_eq!(natural_cmp("File_02.psd", "File_2.psd").then(Ordering::Equal), "File_02.psd".cmp("File_2.psd"), "equal by value: by text"); + let mut names = vec!["File_10.psd", "File_9.psd", "file_1.psd", "File_100.psd", "b.psd", "A.psd", "File_03.psd"]; + names.sort_by(|a, b| natural_cmp(a, b)); + assert_eq!(names, ["A.psd", "b.psd", "file_1.psd", "File_03.psd", "File_9.psd", "File_10.psd", "File_100.psd"]); +} + +/// #3248: several files opened at once (File › Open, the command line, Finder) open in name order. +#[test] +fn open_paths_opens_a_set_in_name_order() { + let (dir, paths) = files("sorted", &["File_10.psd", "File_2.psd", "file_1.psd", "File_03.psd"], b"x"); + let (mut app, _) = app_with(Vec::new()); + assert_eq!(app.open_paths(&paths), 4); + assert_eq!(doc_names(&app), ["file_1.psd", "File_2.psd", "File_03.psd", "File_10.psd"]); + assert_eq!(app.session.active().and_then(|s| s.path.clone()).as_deref(), Some(paths[0].as_str()), "the last in order is active"); + assert!(app.jobs.batches.is_empty(), "nothing left opening"); + // The next set goes after. + let (dir2, more) = files("sorted-2", &["b.psd", "a.psd"], b"x"); + assert_eq!(app.open_paths(&more), 2); + assert_eq!(doc_names(&app), ["file_1.psd", "File_2.psd", "File_03.psd", "File_10.psd", "a.psd", "b.psd"]); + let _ = std::fs::remove_dir_all(&dir); + let _ = std::fs::remove_dir_all(&dir2); +} + +/// #3248: a set opening in the background keeps its order however the opens finish (a small file +/// decodes before a big one named before it). +#[test] +fn a_set_of_background_opens_keeps_its_order_however_they_finish() { + use crate::jobs_ui::{self, OpenOrder, OpenTab}; + use photocraft_engine::jobs::JobId; + let (mut app, _) = app_with(Vec::new()); + app.open_bytes("existing.psd", b"x").unwrap(); + let batch = jobs_ui::begin_batch(&mut app); + let tab = |seq: usize| OpenTab { job: JobId(100 + seq as u64), name: format!("f{seq}.psd"), path: None, slot: None, order: Some(OpenOrder { batch, seq }) }; + app.jobs.opens.extend((0..4).map(tab)); + // f1 opened at once (a preset file, say, or background jobs off for it). + app.jobs.opens.remove(1); + let index = app.session.add_document(Document::new("f1.psd", Size::new(4, 4), ColorMode::Rgb, SampleType::U8), None); + assert_eq!(index, 1); + jobs_ui::opened_in_order(&mut app.jobs, OpenOrder { batch, seq: 1 }); + jobs_ui::end_batch(&mut app.jobs); + assert_eq!(app.jobs.batches.len(), 1, "kept while members are opening"); + // The rest finish out of order: last, first, third. + for seq in [3usize, 0, 2] { + let i = app.jobs.opens.iter().position(|o| o.order.is_some_and(|x| x.seq == seq)).unwrap(); + let tab = app.jobs.opens.remove(i); + let index = app.session.add_document(Document::new(&tab.name, Size::new(4, 4), ColorMode::Rgb, SampleType::U8), None); + jobs_ui::finish_open_tab(&mut app, &tab, &json!({"document": index})).unwrap(); + } + assert_eq!(doc_names(&app), ["existing.psd", "f0.psd", "f1.psd", "f2.psd", "f3.psd"]); + assert_eq!(app.session.active().map(|d| d.doc.name.as_str()), Some("f2.psd"), "the one that just opened is active"); + assert_eq!(app.ui.views.len(), 5, "views follow their documents"); + assert!(app.jobs.batches.is_empty(), "forgotten once every member opened"); + // A tab with a slot (dropped on the tabs) keeps it; one from a forgotten set goes to the end. + let dropped = OpenTab { job: JobId(200), name: "dropped.psd".into(), path: None, slot: Some(0), order: None }; + let index = app.session.add_document(Document::new("dropped.psd", Size::new(4, 4), ColorMode::Rgb, SampleType::U8), None); + jobs_ui::finish_open_tab(&mut app, &dropped, &json!({"document": index})).unwrap(); + let late = OpenTab { job: JobId(201), name: "late.psd".into(), path: None, slot: None, order: Some(OpenOrder { batch: 999, seq: 0 }) }; + let index = app.session.add_document(Document::new("late.psd", Size::new(4, 4), ColorMode::Rgb, SampleType::U8), None); + jobs_ui::finish_open_tab(&mut app, &late, &json!({"document": index})).unwrap(); + assert_eq!(doc_names(&app), ["dropped.psd", "existing.psd", "f0.psd", "f1.psd", "f2.psd", "f3.psd", "late.psd"]); +} diff --git a/crates/ui-egui/src/jobs_ui.rs b/crates/ui-egui/src/jobs_ui.rs index 80944af8..d3049f21 100644 --- a/crates/ui-egui/src/jobs_ui.rs +++ b/crates/ui-egui/src/jobs_ui.rs @@ -40,6 +40,30 @@ pub struct OpenTab { pub path: Option, /// The tab position it goes to when it opens (a file dropped on the tabs); `None`: the end. pub slot: Option, + /// Its place in a set of files opened together, when it is one of several (see [`OpenOrder`]). + pub order: Option, +} + +/// A file's place in a set opened together (File › Open with several files, paths on the command +/// line, the OS's "open these documents"): the set's tabs end up in its order whatever order the +/// background opens finish in (#3248), small files first otherwise. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct OpenOrder { + /// The set, from [`begin_batch`]. + pub batch: u64, + /// The file's position in the set. + pub seq: usize, +} + +/// A set of files opened together: where its tabs start and which members have opened. +#[derive(Clone, Debug)] +pub(crate) struct OpenBatch { + id: u64, + /// The tab position of the set's first document: after the documents and opens there when it + /// started. + base: usize, + /// The `seq` of every member whose document is in place. + opened: Vec, } /// The job command of a background save. @@ -132,6 +156,10 @@ pub struct JobsUi { pub(crate) waiters: Vec<(JobId, Sender)>, /// Background opens, in tab order. pub opens: Vec, + /// Sets of files opened together that still have members opening (see [`OpenOrder`]). + pub(crate) batches: Vec, + /// The id of the next set. + next_batch: u64, /// Background saves. pub saves: Vec, /// The window was asked to close while a save was writing: close it once none is left. @@ -169,7 +197,7 @@ pub fn start_open(app: &mut PhotocraftApp, name: &str, path: Option, sou // Inline (wasm): finish now, like a background open that ended at once. Started::Done(v) => finish_open(app, name, path.as_deref(), &v, None), Started::Job(job) => { - app.jobs.opens.push(OpenTab { job, name: name.to_string(), path, slot: None }); + app.jobs.opens.push(OpenTab { job, name: name.to_string(), path, slot: None, order: None }); app.jobs.focus = Some(job); app.ui.chrome.home = None; app.ui.status = crate::i18n::fmt(tl!("Opening {name}…"), &[("name", name)]); @@ -179,6 +207,48 @@ pub fn start_open(app: &mut PhotocraftApp, name: &str, path: Option, sou } } +/// Start a set of files opened together: the `batch` of their [`OpenOrder`]s. Its tabs follow the +/// documents and opens already there. Call [`end_batch`] once every member has been started. +pub(crate) fn begin_batch(app: &mut PhotocraftApp) -> u64 { + let id = app.jobs.next_batch; + app.jobs.next_batch = app.jobs.next_batch.wrapping_add(1); + let base = app.session.documents().len() + app.jobs.opens.len(); + app.jobs.batches.push(OpenBatch { id, base, opened: Vec::new() }); + id +} + +/// A member of a set opened at once (not in the background): its document is in place at the +/// end, so later members go after it. +pub(crate) fn opened_in_order(jobs: &mut JobsUi, order: OpenOrder) { + if let Some(batch) = jobs.batches.iter_mut().find(|b| b.id == order.batch) { + batch.opened.push(order.seq); + } +} + +/// Every member of the set has been started: forget it unless some are still opening. +pub(crate) fn end_batch(jobs: &mut JobsUi) { + let opens = &jobs.opens; + jobs.batches.retain(|b| opens.iter().any(|o| o.order.is_some_and(|x| x.batch == b.id))); +} + +/// The tab position for a member of a set that just opened: after the members before it that +/// have opened, so the set reads in order whichever order the opens finish in. +fn batch_slot(jobs: &mut JobsUi, order: OpenOrder) -> Option { + let batch = jobs.batches.iter_mut().find(|b| b.id == order.batch)?; + let slot = batch.base + batch.opened.iter().filter(|s| **s < order.seq).count(); + batch.opened.push(order.seq); + Some(slot) +} + +/// A background open's result (`{document, warnings, color}`) for `tab`, already taken out of +/// [`JobsUi::opens`]: the document goes to the tab's slot, or to its place in its set. +pub(crate) fn finish_open_tab(app: &mut PhotocraftApp, tab: &OpenTab, v: &Value) -> Result<(), String> { + let slot = tab.slot.or_else(|| tab.order.and_then(|o| batch_slot(&mut app.jobs, o))); + let r = finish_open(app, &tab.name, tab.path.as_deref(), v, slot); + end_batch(&mut app.jobs); + r +} + /// Save the active document to `path` in the background: `save` encodes and writes it on a /// worker while the window keeps drawing and shows the progress dialog. The document is locked /// until the job ends, which [`tick`] records like a synchronous save. `Ok(None)`: started; @@ -389,7 +459,7 @@ fn on_event(app: &mut PhotocraftApp, ctx: &egui::Context, e: JobEvent) { let tab = app.jobs.opens.remove(i); match e.outcome { JobOutcome::Done(v) => { - if let Err(err) = finish_open(app, &tab.name, tab.path.as_deref(), &v, tab.slot) { + if let Err(err) = finish_open_tab(app, &tab, &v) { app.open_failed(&tab.name, &err); } } @@ -399,6 +469,7 @@ fn on_event(app: &mut PhotocraftApp, ctx: &egui::Context, e: JobEvent) { app.ui.status_error = false; } } + end_batch(&mut app.jobs); return; } match e.outcome { diff --git a/crates/ui-egui/src/jobs_ui_tests.rs b/crates/ui-egui/src/jobs_ui_tests.rs index b1f7412f..f0c62910 100644 --- a/crates/ui-egui/src/jobs_ui_tests.rs +++ b/crates/ui-egui/src/jobs_ui_tests.rs @@ -298,7 +298,7 @@ fn the_opening_card_keeps_a_long_name_and_cancel_inside() { let long = "a7rv-test_2024_02_17_shot_30_3705_with_an_even_longer_suffix_that_never_ends.psb"; for name in ["short.psd", long] { h.state_mut().jobs.opens.clear(); - h.state_mut().jobs.opens.push(crate::jobs_ui::OpenTab { job, name: name.to_string(), path: None, slot: None }); + h.state_mut().jobs.opens.push(crate::jobs_ui::OpenTab { job, name: name.to_string(), path: None, slot: None, order: None }); h.state_mut().jobs.focus = Some(job); h.run_steps(4); let r = crate::jobs_ui::open_card_rects(&h.ctx).expect("the opening card was drawn");