dc-fix-1519: address print preview review

apps/deckcraft/src/main.rs

@@ -60,6 +60,8 @@ impl eframe::App for App {
         // nothing after it is needed: the preferences are saved and the log file is unbuffered.
         // `on_exit` only runs once the app exists, never on a failed start. Same as GridCraft #117.
         #[cfg(target_os = "macos")]
+        self.0.cleanup_print_previews();
+        #[cfg(target_os = "macos")]
         std::process::exit(0);
     }
 }

crates/ui-egui/src/app.rs

@@ -30,6 +30,11 @@ pub use keys::key_name;
 const STATUS_LINGER_MS: f64 = 6000.0;
 
 impl SlideApp {
+    /// Remove temporary PDFs created for printing.
+    pub fn cleanup_print_previews(&mut self) {
+        self.print_previews.cleanup();
+    }
+
     /// Build application UI state, taking ownership of audio output and initialising media, dialogs,
     /// synthetic input, and timing fields.
     pub fn new(session: Session, mut services: Services) -> Self {

crates/ui-egui/src/app/files.rs

@@ -24,44 +24,17 @@ impl PrintPreviews {
             self.0.retain(|owned| owned != dir);
         }
     }
-}
 
-#[cfg(test)]
-mod print_preview_tests {
-    use super::PrintPreviews;
-    use std::time::{SystemTime, UNIX_EPOCH};
-
-    #[test]
-    fn same_title_gets_distinct_private_paths_and_drop_cleans_them() {
-        let stamp = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_nanos()).unwrap_or_default();
-        let base = std::env::temp_dir().join(format!("deckcraft-print-test-{}-{stamp}", std::process::id()));
-        std::fs::create_dir(&base).unwrap();
-        let (first, second) = {
-            let mut previews = PrintPreviews::default();
-            let first = previews.create_in(&base, "Same title").unwrap();
-            let second = previews.create_in(&base, "Same title").unwrap();
-            assert_ne!(first, second);
-            assert_eq!(first.file_name(), second.file_name());
-            assert!(first.parent().is_some_and(|dir| dir.is_dir()));
-            #[cfg(unix)]
-            {
-                use std::os::unix::fs::PermissionsExt;
-                let mode = std::fs::metadata(first.parent().unwrap()).unwrap().permissions().mode();
-                assert_eq!(mode & 0o777, 0o700);
-            }
-            (first, second)
-        };
-        assert!(!first.parent().is_some_and(|dir| dir.exists()));
-        assert!(!second.parent().is_some_and(|dir| dir.exists()));
-        std::fs::remove_dir(base).unwrap();
+    pub(crate) fn cleanup(&mut self) {
+        for dir in self.0.drain(..) {
+            let _ = std::fs::remove_dir_all(dir);
+        }
     }
 }
 
 impl Drop for PrintPreviews {
     fn drop(&mut self) {
-        for dir in &self.0 {
-            let _ = std::fs::remove_dir_all(dir);
-        }
+        self.cleanup();
     }
 }
 
@@ -761,3 +734,49 @@ pub(crate) fn web_image_files(
         })
         .collect()
 }
+
+#[cfg(test)]
+mod print_preview_tests {
+    use super::PrintPreviews;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    #[test]
+    fn same_title_gets_distinct_private_paths_and_drop_cleans_them() {
+        let stamp = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_nanos()).unwrap_or_default();
+        let base = std::env::temp_dir().join(format!("deckcraft-print-test-{}-{stamp}", std::process::id()));
+        std::fs::create_dir(&base).unwrap();
+        let (first, second) = {
+            let mut previews = PrintPreviews::default();
+            let first = previews.create_in(&base, "Same title").unwrap();
+            let second = previews.create_in(&base, "Same title").unwrap();
+            assert_ne!(first, second);
+            assert_eq!(first.file_name(), second.file_name());
+            assert!(first.parent().is_some_and(|dir| dir.is_dir()));
+            #[cfg(unix)]
+            {
+                use std::os::unix::fs::PermissionsExt;
+                let mode = std::fs::metadata(first.parent().unwrap()).unwrap().permissions().mode();
+                assert_eq!(mode & 0o777, 0o700);
+            }
+            (first, second)
+        };
+        assert!(!first.parent().is_some_and(|dir| dir.exists()));
+        assert!(!second.parent().is_some_and(|dir| dir.exists()));
+        std::fs::remove_dir(base).unwrap();
+    }
+
+    #[test]
+    fn explicit_cleanup_removes_previews() {
+        let stamp = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_nanos()).unwrap_or_default();
+        let base = std::env::temp_dir().join(format!("deckcraft-print-cleanup-test-{}-{stamp}", std::process::id()));
+        std::fs::create_dir(&base).unwrap();
+        let mut previews = PrintPreviews::default();
+        let path = previews.create_in(&base, "Explicit cleanup").unwrap();
+        let dir = path.parent().unwrap().to_path_buf();
+
+        previews.cleanup();
+
+        assert!(!dir.exists());
+        std::fs::remove_dir(base).unwrap();
+    }
+}

crates/ui-egui/src/backstage/tests.rs

@@ -634,21 +634,24 @@ fn print_previews_pages_and_prints_a_pdf() {
     h.state_mut().backstage.as_mut().unwrap().print_range = "1-2".into();
     h.run_steps(2);
     click_on_page(&h, "Print");
-    h.run_steps(2);
-    let url = h.state().pending_urls.first().cloned().unwrap_or_default();
-    // The URL was handed to egui at the end of the frame; check what it pointed at.
+    h.step();
+    let url = h
+        .output()
+        .platform_output
+        .commands
+        .iter()
+        .find_map(|command| match command {
+            egui::OutputCommand::OpenUrl(open) => Some(open.url.as_str()),
+            _ => None,
+        })
+        .expect("Print emits an egui OpenUrl command");
     assert!(url.contains("deckcraft-print-"), "print preview uses its private directory: {url}");
-    let title = h.state().session.active().unwrap().title();
-    let safe: String = title.chars().map(|c| if c.is_alphanumeric() || c == ' ' || c == '-' { c } else { '_' }).collect();
-    let file_name = format!("{safe} (print).pdf");
-    let printed = std::fs::read_dir(std::env::temp_dir())
-        .unwrap()
-        .filter_map(Result::ok)
-        .filter(|entry| entry.file_name().to_string_lossy().starts_with("deckcraft-print-"))
-        .map(|entry| entry.path().join(&file_name))
-        .find(|path| path.is_file())
-        .expect("the PDF to print was written in its private directory");
-    assert!(std::fs::read(&printed).unwrap().starts_with(b"%PDF"));
+    let path = url.strip_prefix("file://").expect("print preview URL is a file URL");
+    #[cfg(windows)]
+    let path = path.strip_prefix('/').unwrap_or(path);
+    let printed = std::path::Path::new(path);
+    assert!(printed.is_file(), "the PDF at the emitted URL was written: {url}");
+    assert!(std::fs::read(printed).unwrap().starts_with(b"%PDF"));
     let _ = std::fs::remove_dir_all(printed.parent().unwrap());
     let _ = std::fs::remove_dir_all(&dir);
 }