Open several files in name order, keeping it for background opens (#3248) (#3293)

Files opened together (File > Open with several files, paths on the
command line, the OS's "open these documents") appeared in random tab
order: each one opens in the background and its tab was added when it
finished decoding, so small files came first. The paths were also used
in the order the picker returned them.

`open_paths` now sorts the paths naturally (`File_2` before `File_10`,
case-insensitively) and opens them as a set: every background open
carries its position (`OpenOrder`), and when it finishes its document is
moved after the members before it that have already opened, so the set
reads in order however long each file takes. Members opened at once
(preset files) take their place immediately. The web build sorts the
picked files' contents the same way.

Tests: `natural_cmp`, a set opened synchronously, and a set whose
background opens finish out of order, mixed with a tab dropped at a
slot and an open from a forgotten set.

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<u8>)>) {
+    /// 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<u8>)>) {
+        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);

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<String> = 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<std::str::Chars>) -> 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())

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"]);
+}

crates/ui-egui/src/jobs_ui.rs

@@ -40,6 +40,30 @@ pub struct OpenTab {
     pub path: Option<String>,
     /// The tab position it goes to when it opens (a file dropped on the tabs); `None`: the end.
     pub slot: Option<usize>,
+    /// Its place in a set of files opened together, when it is one of several (see [`OpenOrder`]).
+    pub order: Option<OpenOrder>,
+}
+
+/// 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<usize>,
 }
 
 /// The job command of a background save.
@@ -132,6 +156,10 @@ pub struct JobsUi {
     pub(crate) waiters: Vec<(JobId, Sender<ControlResponse>)>,
     /// Background opens, in tab order.
     pub opens: Vec<OpenTab>,
+    /// Sets of files opened together that still have members opening (see [`OpenOrder`]).
+    pub(crate) batches: Vec<OpenBatch>,
+    /// The id of the next set.
+    next_batch: u64,
     /// Background saves.
     pub saves: Vec<SaveJob>,
     /// 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<String>, 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<String>, 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<usize> {
+    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 {

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");