From fdba8c679a543b6e9f45c14de855b50550920835 Mon Sep 17 00:00:00 2001 From: Hal Goff Date: Thu, 8 Oct 2026 13:26:42 +0100 Subject: [PATCH] =?UTF-8?q?macOS:=20no=20blocking=20file=20picker=20left:?= =?UTF-8?q?=20Save=20As,=20exports,=20folders,=20Browse=E2=80=A6=20(#145?= =?UTF-8?q?=20follow-up)=20(#262)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every remaining rfd::FileDialog (26 call sites) ran -[NSSavePanel runModal] inside winit's event handler, which aborts the app when the window gets an event while the panel is open. They all go through pickers::ask now: an rfd::AsyncFileDialog, with the follow-up run as a closure on a later frame. - One picker at a time, held until its closure starts, so a closure that asks again (merge data, run action) gets its picker; refusals are told. - Closures run inside pdfcraft_engine::guard, as commands do. - Targets drop a pick if the document is no longer active or was edited. - Save-on-close closes only once written; CloseRequest::Tab holds a DocId. - Extract+delete deletes only after writing, and only if unedited. - Signing happens once the path is chosen, on an unchanged document. - Browse… results land only in the same showing of the dialog. - Sign, print-to-PDF and attachment saves write atomically. Co-authored-by: Hal Goff Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- crates/ui-egui/src/actions_ui.rs | 59 +++--- crates/ui-egui/src/comment_props.rs | 28 ++- crates/ui-egui/src/content_ui.rs | 114 ++++------- crates/ui-egui/src/create_ui.rs | 51 ++--- crates/ui-egui/src/dialogs.rs | 2 +- crates/ui-egui/src/editing.rs | 158 ++++++++++------ crates/ui-egui/src/export_ui.rs | 47 +++-- crates/ui-egui/src/files.rs | 220 ++++++++++++--------- crates/ui-egui/src/lib.rs | 28 ++- crates/ui-egui/src/marks_ui.rs | 42 ++-- crates/ui-egui/src/measure_ui.rs | 17 +- crates/ui-egui/src/ocr_ui.rs | 27 ++- crates/ui-egui/src/pickers.rs | 284 ++++++++++++++++++++++------ crates/ui-egui/src/print_ui.rs | 35 ++-- crates/ui-egui/src/sign_ui.rs | 74 ++++++-- crates/ui-egui/src/stamps_ui.rs | 25 +-- crates/ui-egui/tests/editing.rs | 2 +- crates/ui-egui/tests/pickers.rs | 242 +++++++++++++++++++++++- 18 files changed, 1015 insertions(+), 440 deletions(-) diff --git a/crates/ui-egui/src/actions_ui.rs b/crates/ui-egui/src/actions_ui.rs index 01c958e691..a60f1e8d51 100644 --- a/crates/ui-egui/src/actions_ui.rs +++ b/crates/ui-egui/src/actions_ui.rs @@ -44,12 +44,24 @@ impl PdfCraftApp { return; } #[cfg(not(target_arch = "wasm32"))] - let dir = match &self.export_dir_override { - Some(d) => Some(std::path::PathBuf::from(d)), - None => rfd::FileDialog::new().set_title(tl!("Choose a folder for the results").to_string()).pick_folder(), - }; - #[cfg(not(target_arch = "wasm32"))] - let Some(dir) = dir else { return }; + match self.export_dir_override.clone() { + Some(d) => self.run_action_into(action, files, d.into()), + None => { + let dialog = rfd::AsyncFileDialog::new().set_title(tl!("Choose a folder for the results").to_string()); + self.ask_one(crate::pickers::Ask::Folder(dialog), None, move |app, dir| app.run_action_into(action, files, dir)); + } + } + #[cfg(target_arch = "wasm32")] + self.run_action_into(action, files); + } + + /// [`Self::run_action_on`] once the folder for the results is known. + fn run_action_into(&mut self, action: Action, files: Vec<(String, Vec)>, #[cfg(not(target_arch = "wasm32"))] dir: std::path::PathBuf) { + // Another run may have started while the folder picker was open. + if self.action_run.is_some() { + self.notify_tr("An action is already running"); + return; + } let progress = Arc::new(Mutex::new(RunProgress { total: files.len(), ..Default::default() })); let p = progress.clone(); // The summary is written on the worker thread: draw it in the UI's language. @@ -126,24 +138,27 @@ impl PdfCraftApp { let Some(action) = self.wizard.selected.as_ref().and_then(|n| self.all_actions().into_iter().find(|a| &a.name == n)) else { return }; #[cfg(not(target_arch = "wasm32"))] { - let paths = match &self.action_files_override { - Some(p) => p.iter().map(std::path::PathBuf::from).collect(), - None => rfd::FileDialog::new() - .set_title(crate::i18n::fmt(tl!("Files for {name}"), &[("name", &action.name)])) - .add_filter("PDF", &["pdf"]) - .pick_files() - .unwrap_or_default(), + let title = crate::i18n::fmt(tl!("Files for {name}"), &[("name", &action.name)]); + let run = move |app: &mut Self, paths: Vec| { + let mut files = Vec::new(); + for p in paths { + let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + match std::fs::read(&p) { + Ok(b) => files.push((name, b)), + Err(e) => return app.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), + } + } + if !files.is_empty() { + // Asks for the results folder: a second picker, now that this one has closed. + app.run_action_on(action, files); + } }; - let mut files = Vec::new(); - for p in paths { - let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - match std::fs::read(&p) { - Ok(b) => files.push((name, b)), - Err(e) => return self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), + match self.action_files_override.clone() { + Some(p) => run(self, p.into_iter().map(std::path::PathBuf::from).collect()), + None => { + let dialog = rfd::AsyncFileDialog::new().set_title(title).add_filter("PDF", &["pdf"]); + self.ask(crate::pickers::Ask::Files(dialog), None, run); } - } - if !files.is_empty() { - self.run_action_on(action, files); } } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/src/comment_props.rs b/crates/ui-egui/src/comment_props.rs index 6d535eb3be..96eb5ef331 100644 --- a/crates/ui-egui/src/comment_props.rs +++ b/crates/ui-egui/src/comment_props.rs @@ -31,17 +31,27 @@ const ICONS: [NoteIcon; 7] = impl PdfCraftApp { /// Attach file: ask for a file (or take `attach_override`) and attach it at `at`. pub fn attach_file_comment(&mut self, page: usize, at: [f64; 2]) { - let picked = match self.attach_override.take() { - Some(f) => Some(f), + match self.attach_override.take() { + Some((file, data)) => self.add_attachment_comment(page, at, file, data), #[cfg(not(target_arch = "wasm32"))] - None => rfd::FileDialog::new().set_title(tl!("Attach a file")).pick_file().and_then(|p| { - let data = std::fs::read(&p).ok()?; - Some((p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), data)) - }), + None => { + // Attached on a later frame, and only to the document it was started on. + let target = self.active_ids().map(|(_, id)| id); + let dialog = rfd::AsyncFileDialog::new().set_title(tl!("Attach a file")); + self.ask_one(crate::pickers::Ask::File(dialog), target, move |app, p| match std::fs::read(&p) { + Ok(data) => { + app.add_attachment_comment(page, at, p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), data) + } + Err(e) => app.notify_fmt("Couldn't read {name}: {e}", &[("name", &p.display().to_string()), ("e", &e.to_string())]), + }); + } #[cfg(target_arch = "wasm32")] - None => None, - }; - let Some((file, data)) = picked else { return }; + None => {} + } + } + + /// Add a file attachment comment at `at` on `page` of the active document. + fn add_attachment_comment(&mut self, page: usize, at: [f64; 2], file: String, data: Vec) { let tool = crate::comments::CommentTool::Attach; let shape = pdfcraft_engine::Shape::Attachment { at, icon: pdfcraft_engine::AttachIcon::PushPin, file, data }; let edit = Edit::AddAnnotation(pdfcraft_engine::NewAnnotation { diff --git a/crates/ui-egui/src/content_ui.rs b/crates/ui-egui/src/content_ui.rs index b008a89169..fcebab3484 100644 --- a/crates/ui-egui/src/content_ui.rs +++ b/crates/ui-egui/src/content_ui.rs @@ -404,28 +404,35 @@ pub(crate) fn hint(ui: &mut egui::Ui, t: &Tokens) { } impl crate::PdfCraftApp { + /// Pick an image file for the active document, read it, and call `then` with its name and + /// bytes: now when `save_override` answers (tests and automation never see a native dialog), + /// otherwise on a later frame, and only if that document is still the active one. + #[cfg(not(target_arch = "wasm32"))] + fn pick_image(&mut self, title: &str, then: impl FnOnce(&mut Self, String, Vec) + Send + 'static) { + let read = |app: &mut Self, path: std::path::PathBuf| match std::fs::read(&path) { + Ok(bytes) => { + let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + then(app, name, bytes); + } + Err(e) => app.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + }; + match self.save_override.clone() { + Some(p) if p.ends_with(".png") || p.ends_with(".jpg") => read(self, p.into()), + Some(_) => {} + None => { + let dialog = rfd::AsyncFileDialog::new() + .add_filter(tl!("Images"), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) + .set_title(title); + let target = self.active_ids().map(|(_, id)| id); + self.ask_one(crate::pickers::Ask::File(dialog), target, read); + } + } + } + /// Edit a PDF ▸ Add content ▸ Image: pick a file and place it in the middle of the current page. pub fn add_image_dialog(&mut self) { #[cfg(not(target_arch = "wasm32"))] - { - let picked = match self.save_override.clone() { - Some(p) if p.ends_with(".png") || p.ends_with(".jpg") => Some(std::path::PathBuf::from(p)), - // Tests and automation set `save_override` and never see a native dialog. - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("Images").to_string(), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Choose an image")) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { - Ok(bytes) => { - let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - self.add_image(name, bytes); - } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), - } - } + self.pick_image(tl!("Choose an image"), |app, name, bytes| app.add_image(name, bytes)); #[cfg(target_arch = "wasm32")] self.notify_tr("Adding images arrives on the web with file pickers for images"); } @@ -433,28 +440,13 @@ impl crate::PdfCraftApp { /// Edit text & images ▸ Replace Image: pick a file to draw in a page image's place. pub(crate) fn replace_page_image_dialog(&mut self, page: usize, index: usize) { #[cfg(not(target_arch = "wasm32"))] - { - let picked = match self.save_override.clone() { - Some(p) if p.ends_with(".png") || p.ends_with(".jpg") => Some(std::path::PathBuf::from(p)), - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("Images"), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Replace image")) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { - Ok(bytes) => { - let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - self.apply_edit(Edit::EditPageImage { - page, - index, - change: pdfcraft_engine::ImageEdit::Replace { name, bytes: std::sync::Arc::new(bytes) }, - }); - } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), - } - } + self.pick_image(tl!("Replace image"), move |app, name, bytes| { + app.apply_edit(Edit::EditPageImage { + page, + index, + change: pdfcraft_engine::ImageEdit::Replace { name, bytes: std::sync::Arc::new(bytes) }, + }); + }); #[cfg(target_arch = "wasm32")] self.notify_fmt( "Replacing images on page {p} arrives on the web with image pickers ({i})", @@ -479,21 +471,10 @@ impl crate::PdfCraftApp { pub fn choose_field_image(&mut self, name: &str) { #[cfg(not(target_arch = "wasm32"))] { - let picked = match self.save_override.clone() { - Some(p) if p.ends_with(".png") || p.ends_with(".jpg") => Some(std::path::PathBuf::from(p)), - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("Images"), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Select Icon")) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { - Ok(bytes) => { - self.apply_edit(Edit::SetFieldImage { name: name.to_string(), image: std::sync::Arc::new(bytes) }); - } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), - } + let name = name.to_string(); + self.pick_image(tl!("Select Icon"), move |app, _, bytes| { + app.apply_edit(Edit::SetFieldImage { name, image: std::sync::Arc::new(bytes) }); + }); } #[cfg(target_arch = "wasm32")] self.notify_fmt("{name}: choosing images arrives on the web with file pickers for images", &[("name", &name)]); @@ -502,24 +483,9 @@ impl crate::PdfCraftApp { /// Edit image ▸ Replace: pick a file for the selected image. pub fn replace_image_dialog(&mut self, page: usize, index: usize) { #[cfg(not(target_arch = "wasm32"))] - { - let picked = match self.save_override.clone() { - Some(p) if p.ends_with(".png") || p.ends_with(".jpg") => Some(std::path::PathBuf::from(p)), - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("Images"), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Replace image")) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { - Ok(bytes) => { - let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - self.apply_edit(Edit::ReplaceImage { page, index, name, bytes: std::sync::Arc::new(bytes) }); - } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), - } - } + self.pick_image(tl!("Replace image"), move |app, name, bytes| { + app.apply_edit(Edit::ReplaceImage { page, index, name, bytes: std::sync::Arc::new(bytes) }); + }); #[cfg(target_arch = "wasm32")] { let _ = (page, index); diff --git a/crates/ui-egui/src/create_ui.rs b/crates/ui-egui/src/create_ui.rs index 7e0c094241..17c4be2998 100644 --- a/crates/ui-egui/src/create_ui.rs +++ b/crates/ui-egui/src/create_ui.rs @@ -166,24 +166,22 @@ impl PdfCraftApp { pub(crate) fn create_from_images_dialog(&mut self) { #[cfg(not(target_arch = "wasm32"))] { - let Some(files) = rfd::FileDialog::new() + let dialog = rfd::AsyncFileDialog::new() .add_filter(tl!("Images"), &["png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Choose images")) - .pick_files() - else { - return; - }; - let mut images = Vec::new(); - for f in files { - match std::fs::read(&f) { - Ok(b) => images.push((f.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), b)), - Err(e) => { - self.notify_fmt("Couldn't read {name}: {e}", &[("name", &f.display().to_string()), ("e", &e.to_string())]); - return; + .set_title(tl!("Choose images")); + self.ask(crate::pickers::Ask::Files(dialog), None, |app, files| { + let mut images = Vec::new(); + for f in files { + match std::fs::read(&f) { + Ok(b) => images.push((f.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), b)), + Err(e) => { + app.notify_fmt("Couldn't read {name}: {e}", &[("name", &f.display().to_string()), ("e", &e.to_string())]); + return; + } } } - } - self.begin_image_import(images); + app.begin_image_import(images); + }); } #[cfg(target_arch = "wasm32")] self.notify_tr("On the web, open or drop an image to convert it"); @@ -267,14 +265,15 @@ impl PdfCraftApp { return; } }; - let saved = |app: &mut PdfCraftApp, place: String| { - let pct = 100.0 * (1.0 - bytes.len() as f64 / before.max(1) as f64); + let after = bytes.len(); + let saved = move |app: &mut PdfCraftApp, place: String| { + let pct = 100.0 * (1.0 - after as f64 / before.max(1) as f64); app.notify_fmt( "Saved {place}: {before} → {after} ({pct}% smaller){detail}", &[ ("place", &place), ("before", &crate::panels::human_size(before)), - ("after", &crate::panels::human_size(bytes.len())), + ("after", &crate::panels::human_size(after)), ("pct", &format!("{pct:.0}")), ("detail", &detail), ], @@ -282,14 +281,16 @@ impl PdfCraftApp { }; #[cfg(not(target_arch = "wasm32"))] { - let path = match &self.save_override { - Some(p) => Some(p.clone()), - None => rfd::FileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(&name).save_file().map(|p| p.to_string_lossy().into_owned()), + let write = move |app: &mut Self, path: String| match crate::editing::write_atomically(&path, &bytes) { + Ok(()) => saved(app, path), + Err(e) => app.notify_fmt("Couldn't write {path}: {e}", &[("path", &path), ("e", &e.to_string())]), }; - let Some(path) = path else { return }; - match crate::editing::write_atomically(&path, &bytes) { - Ok(()) => saved(self, path), - Err(e) => self.notify_fmt("Couldn't write {path}: {e}", &[("path", &path), ("e", &e.to_string())]), + match self.save_override.clone() { + Some(p) => write(self, p), + None => { + let dialog = rfd::AsyncFileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(&name); + self.ask_one(crate::pickers::Ask::Save(dialog), None, move |app, p| write(app, p.to_string_lossy().into_owned())); + } } } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5abbbd865a..18e1c8d717 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1342,7 +1342,7 @@ pub(crate) fn save_prompt_key(ctx: &egui::Context) -> Option> { fn save_prompt(app: &mut PdfCraftApp, ctx: &egui::Context) { let Some(req) = app.close_request else { return }; let index = match req { - CloseRequest::Tab(i) => Some(i), + CloseRequest::Tab(id) => app.views.iter().position(|v| v.id == id), CloseRequest::Quit | CloseRequest::All => app.first_dirty(), }; let Some(name) = index.and_then(|i| app.views.get(i)).and_then(|v| app.session.get(v.id)).map(|d| d.name.clone()) else { diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 71e83f19c6..26daf6e3d2 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -8,8 +8,9 @@ use crate::PdfCraftApp; /// What the user was doing when we asked whether to save. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum CloseRequest { - /// Close this tab (index into `views`). - Tab(usize), + /// Close this document's tab. By id, not tab index: tabs can close and shift while a save + /// panel is open. + Tab(pdfcraft_engine::DocId), /// Quit the application once every dirty document is resolved. Quit, /// File ▸ Close all: like Quit, but the application stays open. @@ -161,48 +162,52 @@ impl PdfCraftApp { } } - /// Save the document shown in tab `index`. Returns `true` if it was written. + /// Save the document shown in tab `index`. Returns `true` if it was written now. When it has + /// to ask where (a new document, or Save As) it returns `false` and saves on a later frame, + /// once the user has chosen. pub fn save_view(&mut self, index: usize, target: SaveTarget) -> bool { + self.save_then(index, target, |_| {}) + } + + /// [`Self::save_view`], then `after` once the document is written: now, or on a later frame + /// when the user had to choose where. `after` never runs when the save fails or is cancelled. + pub(crate) fn save_then(&mut self, index: usize, target: SaveTarget, after: impl FnOnce(&mut Self) + Send + 'static) -> bool { let Some(id) = self.views.get(index).map(|v| v.id) else { return false }; let Some(doc) = self.session.get(id) else { return false }; let (name, path) = (doc.name.clone(), doc.path.clone()); - let bytes = match self.session.save_bytes(id) { - Ok(b) => b, - Err(e) => { - self.notify_fmt("Couldn't save {name}: {e}", &[("name", &name), ("e", &e.to_string())]); - return false; - } - }; - let destination = match (target, path, &self.save_override) { - (_, _, Some(p)) => Some(p.clone()), - (SaveTarget::InPlace, Some(p), _) => Some(p), - _ => self.ask_save_path(&name), - }; #[cfg(not(target_arch = "wasm32"))] { - let Some(dest) = destination else { return false }; - if let Err(e) = write_atomically(&dest, &bytes) { - self.notify_fmt("Couldn't save {name}: {e}", &[("name", &dest), ("e", &e.to_string())]); - return false; - } - match self.session.mark_saved(id, bytes, Some(dest.clone())) { - Ok(()) => { - self.forget_recovery(id); - if let Some(doc) = self.session.get(id) { - self.views[index].document_changed(&doc.info); - } - self.notify_fmt("Saved {name}", &[("name", &short_name(&dest))]); - true - } - Err(e) => { - self.notify_fmt("Saved, but reopening failed: {e}", &[("e", &e.to_string())]); - false + let destination = match (target, path, &self.save_override) { + (_, _, Some(p)) => p.clone(), + (SaveTarget::InPlace, Some(p), _) => p, + _ => { + let name = if name.to_ascii_lowercase().ends_with(".pdf") { name } else { format!("{name}.pdf") }; + let dialog = rfd::AsyncFileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(name); + // The bytes are taken once the user has chosen, so edits made meanwhile are saved too. + self.ask_one(crate::pickers::Ask::Save(dialog), None, move |app, dest| { + if app.save_doc_to(id, &dest.to_string_lossy()) { + after(app); + } + }); + return false; } + }; + let saved = self.save_doc_to(id, &destination); + if saved { + after(self); } + saved } #[cfg(target_arch = "wasm32")] { - let _ = destination; + let _ = (target, path); + let bytes = match self.session.save_bytes(id) { + Ok(b) => b, + Err(e) => { + self.notify_fmt("Couldn't save {name}: {e}", &[("name", &name), ("e", &e.to_string())]); + return false; + } + }; match download(&name, &bytes) { Ok(()) => { let _ = self.session.mark_saved(id, bytes, None); @@ -210,6 +215,7 @@ impl PdfCraftApp { self.views[index].document_changed(&doc.info); } self.notify_fmt("Downloaded {name}", &[("name", &name)]); + after(self); true } Err(e) => { @@ -220,22 +226,47 @@ impl PdfCraftApp { } } + /// Write document `id` to `dest` and make it the document's file. Returns `true` if written. #[cfg(not(target_arch = "wasm32"))] - fn ask_save_path(&self, name: &str) -> Option { - let name = if name.to_ascii_lowercase().ends_with(".pdf") { name.to_string() } else { format!("{name}.pdf") }; - rfd::FileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(name).save_file().map(|p| p.to_string_lossy().into_owned()) - } - - #[cfg(target_arch = "wasm32")] - fn ask_save_path(&self, _name: &str) -> Option { - None // browsers download instead + fn save_doc_to(&mut self, id: pdfcraft_engine::DocId, dest: &str) -> bool { + let Some(name) = self.session.get(id).map(|d| d.name.clone()) else { + self.notify_tr("The document was closed before it could be saved."); + return false; + }; + let bytes = match self.session.save_bytes(id) { + Ok(b) => b, + Err(e) => { + self.notify_fmt("Couldn't save {name}: {e}", &[("name", &name), ("e", &e.to_string())]); + return false; + } + }; + if let Err(e) = write_atomically(dest, &bytes) { + self.notify_fmt("Couldn't save {name}: {e}", &[("name", dest), ("e", &e.to_string())]); + return false; + } + match self.session.mark_saved(id, bytes, Some(dest.to_string())) { + Ok(()) => { + self.forget_recovery(id); + if let Some(doc) = self.session.get(id) + && let Some(view) = self.views.iter_mut().find(|v| v.id == id) + { + view.document_changed(&doc.info); + } + self.notify_fmt("Saved {name}", &[("name", &short_name(dest))]); + true + } + Err(e) => { + self.notify_fmt("Saved, but reopening failed: {e}", &[("e", &e.to_string())]); + false + } + } } /// Close a tab, asking first if it has unsaved changes. pub fn request_close_tab(&mut self, index: usize) { let dirty = self.views.get(index).and_then(|v| self.session.get(v.id)).is_some_and(|d| d.dirty); - if dirty { - self.close_request = Some(CloseRequest::Tab(index)); + if dirty && let Some(id) = self.views.get(index).map(|v| v.id) { + self.close_request = Some(CloseRequest::Tab(id)); } else { self.close_tab(index); } @@ -278,7 +309,10 @@ impl PdfCraftApp { pub fn resolve_close(&mut self, ctx: &egui::Context, choice: Option) { let Some(req) = self.close_request.take() else { return }; let index = match req { - CloseRequest::Tab(i) => i, + CloseRequest::Tab(id) => match self.views.iter().position(|v| v.id == id) { + Some(i) => i, + None => return, // already closed + }, CloseRequest::Quit | CloseRequest::All => match self.first_dirty() { Some(i) => i, None => { @@ -291,19 +325,31 @@ impl PdfCraftApp { }; match choice { None => {} // cancelled: nothing closes - Some(save) => { - if save && !self.save_view(index, SaveTarget::InPlace) { - return; // save failed or was cancelled: keep the document open - } - self.close_tab(index); - if req == CloseRequest::Quit || req == CloseRequest::All { - // Ask about the next dirty document, or quit. - match self.first_dirty() { - Some(_) => self.close_request = Some(req), - None if req == CloseRequest::Quit => self.quit(ctx), - None => {} + Some(false) => self.close_and_continue(ctx, index, req), + Some(true) => { + let Some(id) = self.views.get(index).map(|v| v.id) else { return }; + // Close only once the save has actually been written, which may be on a later + // frame when the user has to choose where. A failed or cancelled save keeps the + // document open. + let ctx = ctx.clone(); + self.save_then(index, SaveTarget::InPlace, move |app| { + if let Some(i) = app.views.iter().position(|v| v.id == id) { + app.close_and_continue(&ctx, i, req); } - } + }); + } + } + } + + /// Close tab `index` for a close request, then ask about the next dirty document, or quit. + fn close_and_continue(&mut self, ctx: &egui::Context, index: usize, req: CloseRequest) { + self.close_tab(index); + // A newer prompt wins: the user may have started another close while a save was pending. + if (req == CloseRequest::Quit || req == CloseRequest::All) && self.close_request.is_none() { + match self.first_dirty() { + Some(_) => self.close_request = Some(req), + None if req == CloseRequest::Quit => self.quit(ctx), + None => {} } } } diff --git a/crates/ui-egui/src/export_ui.rs b/crates/ui-egui/src/export_ui.rs index 4913bfaf4a..684d32fbba 100644 --- a/crates/ui-egui/src/export_ui.rs +++ b/crates/ui-egui/src/export_ui.rs @@ -217,29 +217,34 @@ impl PdfCraftApp { let status: ExportStatus = Arc::new(Mutex::new(Some((0, pages.len(), None)))); #[cfg(not(target_arch = "wasm32"))] { - let dir = match &self.export_dir_override { - Some(d) => Some(std::path::PathBuf::from(d)), - None => rfd::FileDialog::new().set_title(tl!("Choose a folder for the exported files").to_string()).pick_folder(), - }; - let Some(dir) = dir else { return }; - let st = status.clone(); - let shown = dir.display().to_string(); - let lang = crate::i18n::current(); - let work = move || { - let sink = |name: &str, bytes: Vec| { - crate::editing::write_atomically(&dir.join(name).to_string_lossy(), &bytes).map_err(|e| format!("{name}: {e}")) + let start = move |app: &mut Self, dir: std::path::PathBuf, inline: bool| { + let st = status.clone(); + let shown = dir.display().to_string(); + let lang = crate::i18n::current(); + let work = move || { + let sink = |name: &str, bytes: Vec| { + crate::editing::write_atomically(&dir.join(name).to_string_lossy(), &bytes).map_err(|e| format!("{name}: {e}")) + }; + let msg = run(src, kind, dpi, format, min_side, pages, stem, sink, &st, lang); + if let Ok(mut s) = st.lock() { + let (done, total) = s.as_ref().map_or((0, 0), |(d, t, _)| (*d, *t)); + let done_msg = crate::i18n::fmt(tl!("{msg} to {dir}"), &[("msg", &msg), ("dir", &shown)]); + *s = Some((done.max(total), total, Some(done_msg))); + } }; - let msg = run(src, kind, dpi, format, min_side, pages, stem, sink, &st, lang); - if let Ok(mut s) = st.lock() { - let (done, total) = s.as_ref().map_or((0, 0), |(d, t, _)| (*d, *t)); - let done_msg = crate::i18n::fmt(tl!("{msg} to {dir}"), &[("msg", &msg), ("dir", &shown)]); - *s = Some((done.max(total), total, Some(done_msg))); + if inline { + work(); // tests and automation: synchronous + } else { + std::thread::Builder::new().name("pdfcraft-export".into()).spawn(work).ok(); } + app.export_status = Some(status); }; - if self.export_dir_override.is_some() { - work(); // tests and automation: synchronous - } else { - std::thread::Builder::new().name("pdfcraft-export".into()).spawn(work).ok(); + match self.export_dir_override.clone() { + Some(d) => start(self, d.into(), true), + None => { + let dialog = rfd::AsyncFileDialog::new().set_title(tl!("Choose a folder for the exported files").to_string()); + self.ask_one(crate::pickers::Ask::Folder(dialog), None, move |app, dir| start(app, dir, false)); + } } } #[cfg(target_arch = "wasm32")] @@ -259,8 +264,8 @@ impl PdfCraftApp { if let Ok(mut s) = status.lock() { *s = Some((0, 0, Some(msg))); } + self.export_status = Some(status); } - self.export_status = Some(status); } /// Show export progress, and the result once it is done. diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index a12061da82..0ca18b9599 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -232,9 +232,23 @@ impl PdfCraftApp { } } } - if self.write_files(&named, tl!("Choose a folder for the extracted pages")) == 0 { - return; - } + // Delete the pages only once their files are written, which is on a later frame when + // the user chooses the folder, and only if they are still what was written: the + // document must be unedited since (page numbers shift, and edits to those pages + // aren't in the files) and still the active one (deleting edits the active document). + let generation = self.session.get(id).map(|d| d.edit_generation()); + self.write_files_then(named, tl!("Choose a folder for the extracted pages"), move |app| { + if !opts.delete { + return; + } + let unchanged = app.session.get(id).map(|d| d.edit_generation()) == generation; + if unchanged && app.active_ids().map(|(_, active)| active) == Some(id) { + app.apply_edit(pdfcraft_engine::Edit::DeletePages { pages }); + } else { + app.notify_tr("The pages were extracted but not deleted, because the document changed while you were choosing a folder."); + } + }); + return; } else { match self.session.extract(id, &pages) { Ok(bytes) => { @@ -258,38 +272,48 @@ impl PdfCraftApp { } } - /// Write named files into a chosen folder (desktop) or as downloads (web). Returns how many. - pub(crate) fn write_files(&mut self, named: &[(String, Arc>)], title: &str) -> usize { + /// Write named files into a chosen folder (desktop) or as downloads (web). + pub(crate) fn write_files(&mut self, named: &[(String, Arc>)], title: &str) { + self.write_files_then(named.to_vec(), title, |_| {}); + } + + /// [`Self::write_files`], then `after` once every file is written: now, or on a later frame + /// when the user chooses the folder. `after` never runs when writing fails or is cancelled. + pub(crate) fn write_files_then(&mut self, named: Vec<(String, Arc>)>, title: &str, after: impl FnOnce(&mut Self) + Send + 'static) { #[cfg(not(target_arch = "wasm32"))] { - let dir = match &self.export_dir_override { - Some(d) => Some(std::path::PathBuf::from(d)), - None => rfd::FileDialog::new().set_title(title).pick_folder(), + let write = move |app: &mut Self, dir: std::path::PathBuf| { + for (name, bytes) in &named { + if let Err(e) = crate::editing::write_atomically(&dir.join(name).to_string_lossy(), bytes) { + app.notify_fmt("Couldn't write {name}: {e}", &[("name", name), ("e", &e.to_string())]); + return; + } + } + if named.len() == 1 { + app.notify_fmt("Wrote 1 file to {dir}", &[("dir", &dir.display().to_string())]); + } else { + app.notify_fmt("Wrote {n} files to {dir}", &[("n", &named.len().to_string()), ("dir", &dir.display().to_string())]); + } + after(app); }; - let Some(dir) = dir else { return 0 }; - for (name, bytes) in named { - if let Err(e) = crate::editing::write_atomically(&dir.join(name).to_string_lossy(), bytes) { - self.notify_fmt("Couldn't write {name}: {e}", &[("name", name), ("e", &e.to_string())]); - return 0; + match &self.export_dir_override { + Some(d) => write(self, std::path::PathBuf::from(d)), + None => { + self.ask_one(crate::pickers::Ask::Folder(rfd::AsyncFileDialog::new().set_title(title)), None, write); } - } - if named.len() == 1 { - self.notify_fmt("Wrote 1 file to {dir}", &[("dir", &dir.display().to_string())]); - } else { - self.notify_fmt("Wrote {n} files to {dir}", &[("n", &named.len().to_string()), ("dir", &dir.display().to_string())]); } } #[cfg(target_arch = "wasm32")] { let _ = title; - for (name, bytes) in named { + for (name, bytes) in &named { if let Err(e) = crate::editing::download(name, bytes) { self.notify_fmt("Couldn't download {name}: {e}", &[("name", name), ("e", &e.to_string())]); - return 0; + return; } } + after(self); } - named.len() } /// Pages ▸ Rotate Pages with the dialog's range and filters. @@ -311,10 +335,10 @@ impl PdfCraftApp { self.apply_edit(pdfcraft_engine::Edit::RotatePages { pages, degrees: d.degrees }); } - /// Split the active document and write the parts: into a chosen folder (desktop) or as - /// downloads (web). Returns the number of files written. - pub fn split_active(&mut self, plan: &SplitPlan) -> usize { - let Some((_, id)) = self.active_ids() else { return 0 }; + /// Split the active document and write the parts: into a chosen folder (desktop, written on + /// a later frame once the user has chosen it) or as downloads (web). + pub fn split_active(&mut self, plan: &SplitPlan) { + let Some((_, id)) = self.active_ids() else { return }; let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_else(|| "document".into()); let (parts, titles) = match plan { SplitPlan::By(by) => (self.session.split(id, by), Vec::new()), @@ -329,7 +353,7 @@ impl PdfCraftApp { Ok(p) => p, Err(e) => { self.notify_fmt("Couldn't split: {e}", &[("e", &e.to_string())]); - return 0; + return; } }; let safe = |t: &str| t.chars().map(|c| if c.is_alphanumeric() || " -_.,()".contains(c) { c } else { '_' }).collect::(); @@ -346,7 +370,7 @@ impl PdfCraftApp { (name, bytes) }) .collect(); - self.write_files(&named, "Choose a folder for the split files") + self.write_files(&named, "Choose a folder for the split files"); } /// Summarize Comments: make the summary and open it as a new document. @@ -382,21 +406,23 @@ impl PdfCraftApp { pub fn import_data_dialog(&mut self) { #[cfg(not(target_arch = "wasm32"))] { - let picked = match self.save_override.clone() { - Some(p) if [".xfdf", ".fdf", ".xml", ".csv", ".txt"].iter().any(|e| p.ends_with(e)) => Some(std::path::PathBuf::from(p)), - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("Comment and form data").to_string(), &["xfdf", "fdf", "xml", "csv", "txt"]) - .set_title(tl!("Import data").to_string()) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { + let import = |app: &mut Self, path: std::path::PathBuf| match std::fs::read(&path) { Ok(bytes) => { let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - self.apply_edit(pdfcraft_engine::Edit::ImportData { name, bytes: std::sync::Arc::new(bytes) }); + app.apply_edit(pdfcraft_engine::Edit::ImportData { name, bytes: std::sync::Arc::new(bytes) }); + } + Err(e) => app.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + }; + match self.save_override.clone() { + Some(p) if [".xfdf", ".fdf", ".xml", ".csv", ".txt"].iter().any(|e| p.ends_with(e)) => import(self, p.into()), + Some(_) => {} + None => { + let dialog = rfd::AsyncFileDialog::new() + .add_filter(tl!("Comment and form data").to_string(), &["xfdf", "fdf", "xml", "csv", "txt"]) + .set_title(tl!("Import data").to_string()); + let target = self.active_ids().map(|(_, id)| id); + self.ask_one(crate::pickers::Ask::File(dialog), target, import); } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), } } #[cfg(target_arch = "wasm32")] @@ -413,18 +439,19 @@ impl PdfCraftApp { let bytes = doc.export_office(format); #[cfg(not(target_arch = "wasm32"))] { - let path = match self.save_override.clone() { - Some(p) => Some(std::path::PathBuf::from(p)), - None => rfd::FileDialog::new() - .set_title(tl!("Export").to_string()) - .add_filter(ext.to_uppercase(), &[ext]) - .set_file_name(format!("{stem}.{ext}")) - .save_file(), + let write = move |app: &mut Self, path: std::path::PathBuf| match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { + Ok(()) => app.notify_fmt("Exported to {path}", &[("path", &path.display().to_string())]), + Err(e) => app.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), }; - let Some(path) = path else { return }; - match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { - Ok(()) => self.notify_fmt("Exported to {path}", &[("path", &path.display().to_string())]), - Err(e) => self.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + match self.save_override.clone() { + Some(p) => write(self, p.into()), + None => { + let dialog = rfd::AsyncFileDialog::new() + .set_title(tl!("Export").to_string()) + .add_filter(ext.to_uppercase(), &[ext]) + .set_file_name(format!("{stem}.{ext}")); + self.ask_one(crate::pickers::Ask::Save(dialog), None, write); + } } } #[cfg(target_arch = "wasm32")] @@ -438,22 +465,23 @@ impl PdfCraftApp { pub fn merge_data_dialog(&mut self) { #[cfg(not(target_arch = "wasm32"))] { - let paths = rfd::FileDialog::new() + let dialog = rfd::AsyncFileDialog::new() .set_title(tl!("Select data files to merge").to_string()) - .add_filter(tl!("Form data and PDF forms").to_string(), &["fdf", "xfdf", "pdf"]) - .pick_files() - .unwrap_or_default(); - let mut files = Vec::new(); - for p in paths { - let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - match std::fs::read(&p) { - Ok(b) => files.push((name, b)), - Err(e) => return self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), + .add_filter(tl!("Form data and PDF forms").to_string(), &["fdf", "xfdf", "pdf"]); + self.ask(crate::pickers::Ask::Files(dialog), None, |app, paths| { + let mut files = Vec::new(); + for p in paths { + let name = p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + match std::fs::read(&p) { + Ok(b) => files.push((name, b)), + Err(e) => return app.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]), + } } - } - if !files.is_empty() { - self.merge_data_files(files); - } + if !files.is_empty() { + // Asks where to save: a second picker, now that the first one has closed. + app.merge_data_files(files); + } + }); } #[cfg(target_arch = "wasm32")] self.notify_tr("Merging data files needs the desktop app"); @@ -467,24 +495,27 @@ impl PdfCraftApp { }; #[cfg(not(target_arch = "wasm32"))] { - let path = match self.save_override.clone() { - Some(p) => Some(std::path::PathBuf::from(p)), - None => rfd::FileDialog::new() - .set_title(tl!("Save the spreadsheet").to_string()) - .add_filter("CSV", &["csv"]) - .set_file_name("report.csv") - .save_file(), - }; - let Some(path) = path else { return }; - match crate::editing::write_atomically(&path.to_string_lossy(), csv.as_bytes()) { - Ok(()) => { - if files.len() == 1 { - self.notify_fmt("Merged 1 file into {path}", &[("path", &path.display().to_string())]); - } else { - self.notify_fmt("Merged {n} files into {path}", &[("n", &files.len().to_string()), ("path", &path.display().to_string())]); + let count = files.len(); + let write = + move |app: &mut Self, path: std::path::PathBuf| match crate::editing::write_atomically(&path.to_string_lossy(), csv.as_bytes()) { + Ok(()) => { + if count == 1 { + app.notify_fmt("Merged 1 file into {path}", &[("path", &path.display().to_string())]); + } else { + app.notify_fmt("Merged {n} files into {path}", &[("n", &count.to_string()), ("path", &path.display().to_string())]); + } } + Err(e) => app.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + }; + match self.save_override.clone() { + Some(p) => write(self, p.into()), + None => { + let dialog = rfd::AsyncFileDialog::new() + .set_title(tl!("Save the spreadsheet").to_string()) + .add_filter("CSV", &["csv"]) + .set_file_name("report.csv"); + self.ask_one(crate::pickers::Ask::Save(dialog), None, write); } - Err(e) => self.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), } } #[cfg(target_arch = "wasm32")] @@ -497,11 +528,22 @@ impl PdfCraftApp { let stem = doc.name.trim_end_matches(".pdf").trim_end_matches(".PDF").to_string(); #[cfg(not(target_arch = "wasm32"))] { - let path = match self.save_override.clone() { - Some(p) => Some(std::path::PathBuf::from(p)), + let write = move |app: &mut Self, path: std::path::PathBuf| { + let ext = path.extension().map(|e| e.to_string_lossy().into_owned()).unwrap_or_default(); + let format = pdfcraft_engine::DataFormat::from_extension(&ext).unwrap_or(pdfcraft_engine::DataFormat::Xfdf); + match app.session.export_data(id, format, comments, fields) { + Ok(bytes) => match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { + Ok(()) => app.notify_fmt("Exported to {path}", &[("path", &path.display().to_string())]), + Err(e) => app.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + }, + Err(e) => app.notify_error(e), + } + }; + match self.save_override.clone() { + Some(p) => write(self, p.into()), None => { let title = if comments { tl!("Export comments") } else { tl!("Export form data") }; - let d = rfd::FileDialog::new().set_title(title).set_file_name(format!("{stem}.xfdf")); + let d = rfd::AsyncFileDialog::new().set_title(title).set_file_name(format!("{stem}.xfdf")); let d = if comments { d.add_filter("XFDF", &["xfdf"]).add_filter("FDF", &["fdf"]) } else { @@ -511,18 +553,8 @@ impl PdfCraftApp { .add_filter("CSV", &["csv"]) .add_filter(tl!("Text").to_string(), &["txt"]) }; - d.save_file() + self.ask_one(crate::pickers::Ask::Save(d), None, write); } - }; - let Some(path) = path else { return }; - let ext = path.extension().map(|e| e.to_string_lossy().into_owned()).unwrap_or_default(); - let format = pdfcraft_engine::DataFormat::from_extension(&ext).unwrap_or(pdfcraft_engine::DataFormat::Xfdf); - match self.session.export_data(id, format, comments, fields) { - Ok(bytes) => match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { - Ok(()) => self.notify_fmt("Exported to {path}", &[("path", &path.display().to_string())]), - Err(e) => self.notify_fmt("Couldn't write {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), - }, - Err(e) => self.notify_error(e), } } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0583abe43d..2fb5df7d69 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -443,6 +443,10 @@ pub struct PdfCraftApp { last_autosave: f64, pending_recovered: Option, allow_quit: bool, + /// The dialog seen at the last check, and a counter bumped whenever it changes (see + /// [`Self::dialog_epoch`]). + dialog_seen: Option, + dialog_epoch: u64, /// The egui context, for commands that change window or theme state. ctx: Option, styled: bool, @@ -623,6 +627,8 @@ impl PdfCraftApp { last_autosave: 0.0, pending_recovered: None, allow_quit: false, + dialog_seen: None, + dialog_epoch: 0, ctx: None, styled: false, fonts_ready: false, @@ -793,6 +799,17 @@ impl PdfCraftApp { }); } + /// A number that changes whenever the open dialog changes (opened, closed or replaced by + /// another), checked every frame. A picker started from a dialog's Browse… button answers + /// only into the same showing of that dialog. + pub(crate) fn dialog_epoch(&mut self) -> u64 { + if self.dialog != self.dialog_seen { + self.dialog_seen = self.dialog; + self.dialog_epoch = self.dialog_epoch.wrapping_add(1); + } + self.dialog_epoch + } + /// Save an attachment to disk, or open a PDF attachment in a new tab. pub fn attachment_action(&mut self, doc: DocId, index: usize, open: bool) { let Some(d) = self.session.get(doc) else { return }; @@ -807,12 +824,12 @@ impl PdfCraftApp { } (Ok(bytes), false) => { #[cfg(not(target_arch = "wasm32"))] - if let Some(path) = rfd::FileDialog::new().set_file_name(&att.name).save_file() { - match std::fs::write(&path, &bytes) { - Ok(()) => self.notify_fmt("Saved {name}", &[("name", &path.display().to_string())]), - Err(e) => self.notify_fmt("Couldn't save: {e}", &[("e", &e.to_string())]), + self.ask_one(pickers::Ask::Save(rfd::AsyncFileDialog::new().set_file_name(&att.name)), None, move |app, path| { + match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { + Ok(()) => app.notify_fmt("Saved {name}", &[("name", &path.display().to_string())]), + Err(e) => app.notify_fmt("Couldn't save: {e}", &[("e", &e.to_string())]), } - } + }); #[cfg(target_arch = "wasm32")] self.notify_fmt("Downloading attachments on the web arrives with M3.10 ({n} bytes ready)", &[("n", &bytes.len().to_string())]); } @@ -1352,6 +1369,7 @@ impl eframe::App for PdfCraftApp { fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { self.ctx = Some(ctx.clone()); + self.dialog_epoch(); // Notices raised outside `ui` (opened files, OS events, the control channel) translate too. let lang = i18n::Lang::from_pref(&self.language); i18n::set_current(lang); diff --git a/crates/ui-egui/src/marks_ui.rs b/crates/ui-egui/src/marks_ui.rs index 2812652fa3..c92ae1f61f 100644 --- a/crates/ui-egui/src/marks_ui.rs +++ b/crates/ui-egui/src/marks_ui.rs @@ -145,6 +145,7 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M let count = doc.info.pages.len(); let page = doc.info.pages.get(current).map(|p| (p.width as f64, p.height as f64)).unwrap_or((612.0, 792.0)); let d = &mut app.marks_draft; + let mut browse = false; let verb = if d.replace { tl!("Update") } else { tl!("Add") }; let title = match kind { MarkKind::HeaderFooter => crate::i18n::fmt(tl!("{verb} Header and Footer"), &[("verb", verb)]), @@ -224,7 +225,7 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M ui.radio_value(&mut d.use_file, true, tl!("File")); }); if d.use_file { - file_source(ui, d, MarkKind::Watermark); + browse |= file_source(ui, d, MarkKind::Watermark); } ui.horizontal_top(|ui| { ui.vertical(|ui| { @@ -279,7 +280,7 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M ui.radio_value(&mut d.use_file, true, tl!("File")); }); if d.use_file { - file_source(ui, d, MarkKind::Background); + browse |= file_source(ui, d, MarkKind::Background); } ui.horizontal(|ui| { ui.label(tl!("Opacity")); @@ -398,6 +399,23 @@ pub(crate) fn body(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens, kind: M cancel = true; } }); + // Browse…: the picker answers on a later frame, into the draft. + #[cfg(not(target_arch = "wasm32"))] + if browse { + let dialog = rfd::AsyncFileDialog::new() + .add_filter(tl!("PDF or image"), &["pdf", "png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]); + let epoch = app.dialog_epoch(); + app.ask_one(crate::pickers::Ask::File(dialog), None, move |app, p| { + // Only into the dialog that asked: not one closed meanwhile, or opened again since. + if app.dialog_epoch() != epoch { + return; + } + app.marks_draft.file = + std::fs::read(&p).ok().map(|b| (p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), std::sync::Arc::new(b))); + }); + } + #[cfg(target_arch = "wasm32")] + let _ = browse; (apply, cancel) } @@ -406,19 +424,16 @@ fn mark_file(d: &MarksDraft) -> Option { Some(pdfcraft_engine::MarkFile { name, bytes, page: d.file_page.max(1) - 1 }) } -/// Source ▸ File: Browse…, the page of a PDF, and the size relative to the page. -fn file_source(ui: &mut egui::Ui, d: &mut MarksDraft, kind: MarkKind) { +/// Source ▸ File: Browse…, the page of a PDF, and the size relative to the page. Returns true +/// when Browse… was clicked (the caller shows the picker once it no longer borrows the draft). +fn file_source(ui: &mut egui::Ui, d: &mut MarksDraft, kind: MarkKind) -> bool { + // Browse… is desktop-only. + #[cfg_attr(target_arch = "wasm32", allow(unused_mut))] + let mut browse = false; ui.horizontal(|ui| { #[cfg(not(target_arch = "wasm32"))] - if ui.button(tl!("Browse…")).clicked() - && let Some(p) = rfd::FileDialog::new() - .add_filter(tl!("PDF or image"), &["pdf", "png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .pick_file() - { - match std::fs::read(&p) { - Ok(b) => d.file = Some((p.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(), std::sync::Arc::new(b))), - Err(_) => d.file = None, - } + if ui.button(tl!("Browse…")).clicked() { + browse = true; } ui.label(d.file.as_ref().map(|f| f.0.clone()).unwrap_or_else(|| tl!("No file chosen").to_string())); if d.file.as_ref().is_some_and(|f| f.1.starts_with(b"%PDF")) { @@ -434,6 +449,7 @@ fn file_source(ui: &mut egui::Ui, d: &mut MarksDraft, kind: MarkKind) { *scale = pct / 100.0; } }); + browse } /// The edit the dialog's OK makes. diff --git a/crates/ui-egui/src/measure_ui.rs b/crates/ui-egui/src/measure_ui.rs index 4743b7d374..f6f8fb14d8 100644 --- a/crates/ui-egui/src/measure_ui.rs +++ b/crates/ui-egui/src/measure_ui.rs @@ -459,15 +459,20 @@ fn export_csv(app: &mut PdfCraftApp) { )); } #[cfg(not(target_arch = "wasm32"))] - if let Some(path) = app - .export_dir_override - .as_ref() - .map(|d| std::path::PathBuf::from(d).join("measurements.csv")) - .or_else(|| rfd::FileDialog::new().set_file_name("measurements.csv").add_filter("CSV", &["csv"]).save_file()) { - match crate::editing::write_atomically(&path.to_string_lossy(), csv.as_bytes()) { + let write = move |app: &mut PdfCraftApp, path: std::path::PathBuf| match crate::editing::write_atomically( + &path.to_string_lossy(), + csv.as_bytes(), + ) { Ok(()) => app.notify(tl!("Saved measurements.")), Err(e) => app.notify(e.to_string()), + }; + match app.export_dir_override.as_ref().map(|d| std::path::PathBuf::from(d).join("measurements.csv")) { + Some(path) => write(app, path), + None => { + let dialog = rfd::AsyncFileDialog::new().set_file_name("measurements.csv").add_filter("CSV", &["csv"]); + app.ask_one(crate::pickers::Ask::Save(dialog), None, write); + } } } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/src/ocr_ui.rs b/crates/ui-egui/src/ocr_ui.rs index c174e125df..c8a32bbe66 100644 --- a/crates/ui-egui/src/ocr_ui.rs +++ b/crates/ui-egui/src/ocr_ui.rs @@ -188,15 +188,28 @@ impl PdfCraftApp { self.notify_tr("Text recognition is already running"); return; } + // The settings showing now, not when the folder arrives. let settings = OcrSettings { dpi: self.ocr_draft.dpi as f32, language: self.ocr_draft.language.clone(), ..Default::default() }; + #[cfg(not(target_arch = "wasm32"))] + match self.export_dir_override.clone() { + Some(d) => self.ocr_files_into(files, settings, d.into()), + None => { + let dialog = rfd::AsyncFileDialog::new().set_title(tl!("Choose a folder for the searchable files").to_string()); + self.ask_one(crate::pickers::Ask::Folder(dialog), None, move |app, dir| app.ocr_files_into(files, settings, dir)); + } + } + #[cfg(target_arch = "wasm32")] + self.ocr_files_into(files, settings); + } + + /// [`Self::ocr_files`] once the folder for the searchable files is known. + fn ocr_files_into(&mut self, files: Vec<(String, Vec)>, settings: OcrSettings, #[cfg(not(target_arch = "wasm32"))] dir: std::path::PathBuf) { + // Another batch may have started while the folder picker was open. + if self.ocr_batch.is_some() { + self.notify_tr("Text recognition is already running"); + return; + } let progress = Arc::new(Mutex::new(BatchProgress { total: files.len(), ..Default::default() })); - #[cfg(not(target_arch = "wasm32"))] - let dir = match &self.export_dir_override { - Some(d) => Some(std::path::PathBuf::from(d)), - None => rfd::FileDialog::new().set_title(tl!("Choose a folder for the searchable files").to_string()).pick_folder(), - }; - #[cfg(not(target_arch = "wasm32"))] - let Some(dir) = dir else { return }; let p = progress.clone(); // The summary is written on the worker thread: draw it in the UI's language. let lang = crate::i18n::current(); diff --git a/crates/ui-egui/src/pickers.rs b/crates/ui-egui/src/pickers.rs index 5f64dcd7a1..decbd744f7 100644 --- a/crates/ui-egui/src/pickers.rs +++ b/crates/ui-egui/src/pickers.rs @@ -3,10 +3,25 @@ //! A blocking `rfd::FileDialog` runs `-[NSOpenPanel runModal]` on macOS: a nested run loop inside //! winit's event handler. An event that arrives while it spins re-enters the handler, winit //! panics, and because the panic can't unwind out of the AppKit callback the app aborts. -//! `rfd::AsyncFileDialog` shows the panel without a nested run loop and returns at once instead. -//! A worker thread waits for the choice, and the app uses it on a later frame -//! ([`PdfCraftApp::process_picked`]). The panel has no parent window yet, like every other -//! picker in the app, so it floats rather than opening as a sheet. +//! `rfd::AsyncFileDialog` returns at once instead, and the app uses the choice on a later frame +//! ([`PdfCraftApp::process_picked`]); a worker thread waits for it. +//! +//! How rfd 0.17 shows the panel (checked against its source): +//! - macOS: creating the future shows the panel, so it must be created on the main thread. With +//! no parent given, rfd uses the app's main (or first) window and opens the panel as a sheet +//! on it (`beginSheetModalForWindow`). Without a running app and a window it falls back to the +//! blocking `runModal`, which is why this only runs inside the running eframe app. +//! - Windows: the panel runs on a thread rfd starts when the future is first polled (here, by +//! the worker), not inside winit's callback. +//! - Linux and the BSDs: the XDG portal backend (with a Zenity fallback) also works off the UI +//! thread; the optional GTK3 backend shows the panel on rfd's own GTK thread. +//! +//! Every native picker goes through [`PdfCraftApp::ask`]: open, save, folder and multi-file +//! panels alike. What happens with the choice is a closure that runs on that later frame, so +//! work that depends on the file being written (closing the tab after Save, deleting extracted +//! pages) happens there and nowhere else. One picker shows at a time, and it keeps that claim +//! until its closure starts, so a closure that asks again (merge data, then save the +//! spreadsheet) always gets its second picker. use std::future::Future; use std::path::PathBuf; @@ -27,57 +42,95 @@ pub enum PickFor { Files(FilePurpose), } +/// Which native panel to show. +pub(crate) enum Ask { + /// Choose one existing file. + File(rfd::AsyncFileDialog), + /// Choose one or more existing files. + Files(rfd::AsyncFileDialog), + /// Choose a folder. + Folder(rfd::AsyncFileDialog), + /// Choose where to save a file. + Save(rfd::AsyncFileDialog), +} + +/// What to do with the chosen paths, on a later frame. +type Then = Box) + Send>; + +/// The document a pick edits, as it was when the picker opened. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct Target { + doc: DocId, + /// Its edit generation then: pages and objects the pick refers to by position are only + /// valid while it is unchanged. + generation: Option, +} + /// A finished pick, waiting for the next frame. -#[derive(Debug)] struct Picked { - pick_for: PickFor, - /// The document the pick acts on, for purposes that edit the active document. - target: Option, - /// The chosen files; empty when the picker was cancelled. + target: Option, + /// The chosen paths; empty when the picker was cancelled. paths: Vec, + then: Then, +} + +impl std::fmt::Debug for Picked { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Picked").field("target", &self.target).field("paths", &self.paths).finish_non_exhaustive() + } } /// Pickers in flight and the picks they finished. #[derive(Clone, Debug, Default)] pub struct Pickers { - /// A picker is showing; a second request is ignored until it closes. + /// A picker is showing, or its pick is waiting for the frame; a second request is refused + /// until the pick's closure starts. showing: Arc, done: Arc>>, } -/// Clears `showing` when the worker ends, including when it panics. -struct Showing(Arc); +/// Clears `showing` if the worker ends without queueing a pick (it panicked). +struct Showing(Option>); + +impl Showing { + /// The pick is queued: the frame that uses it clears the flag. + fn hand_over(mut self) { + self.0 = None; + } +} impl Drop for Showing { fn drop(&mut self) { - self.0.store(false, Ordering::SeqCst); + if let Some(flag) = &self.0 { + flag.store(false, Ordering::SeqCst); + } } } impl Pickers { - /// Wait for `pick` on a worker thread and queue its result for the next frame. `pick` must - /// already have shown the picker (rfd starts the panel when the future is created, on the - /// main thread). Returns false when the worker couldn't start. - fn spawn(&self, pick_for: PickFor, target: Option, ctx: Option, pick: F) -> bool + /// Wait for `pick` on a worker thread and queue its result for the next frame. On macOS + /// `pick` must already have shown the picker (rfd starts the panel when the future is + /// created, on the main thread). Returns false when the worker couldn't start. + fn spawn(&self, target: Option, ctx: Option, pick: F, then: Then) -> bool where F: Future> + Send + 'static, { - self.spawn_worker(pick_for, target, ctx, pick).is_some() + self.spawn_worker(target, ctx, pick, then).is_some() } /// [`Self::spawn`], returning the worker so tests can wait for it instead of polling. - fn spawn_worker(&self, pick_for: PickFor, target: Option, ctx: Option, pick: F) -> Option> + fn spawn_worker(&self, target: Option, ctx: Option, pick: F, then: Then) -> Option> where F: Future> + Send + 'static, { - let showing = Showing(self.showing.clone()); + let showing = Showing(Some(self.showing.clone())); let done = self.done.clone(); std::thread::Builder::new() .name("file-picker".into()) .spawn(move || { - let _showing = showing; let paths = pollster::block_on(pick); - done.lock().unwrap_or_else(PoisonError::into_inner).push(Picked { pick_for, target, paths }); + done.lock().unwrap_or_else(PoisonError::into_inner).push(Picked { target, paths, then }); + showing.hand_over(); if let Some(ctx) = ctx { ctx.request_repaint(); } @@ -85,9 +138,10 @@ impl Pickers { .ok() } - /// Queue a pick that is already known (tests and automation). - fn deliver(&self, pick_for: PickFor, target: Option, paths: Vec) { - self.done.lock().unwrap_or_else(PoisonError::into_inner).push(Picked { pick_for, target, paths }); + /// Queue a pick that is already known (tests and automation). Like a real pick, it keeps + /// the flag until the frame uses it. + fn deliver(&self, target: Option, paths: Vec, then: Then) { + self.done.lock().unwrap_or_else(PoisonError::into_inner).push(Picked { target, paths, then }); } fn take(&self) -> Vec { @@ -99,48 +153,102 @@ impl PdfCraftApp { /// Show a native picker for `pick_for` without blocking the frame. The choice is used on a /// later frame by [`Self::process_picked`]. pub(crate) fn pick(&mut self, pick_for: PickFor, dialog: rfd::AsyncFileDialog, multiple: bool) { - if self.pickers.showing.swap(true, Ordering::SeqCst) { - return; - } // Insert and Replace edit the active document: remember which one. let target = match pick_for { PickFor::Files(FilePurpose::InsertPages | FilePurpose::ReplacePages) => self.active_ids().map(|(_, id)| id), _ => None, }; - if let Some(paths) = self.pick_override.clone() { - self.pickers.showing.store(false, Ordering::SeqCst); - self.pickers.deliver(pick_for, target, paths.into_iter().map(PathBuf::from).collect()); - return; + let ask = if multiple { Ask::Files(dialog) } else { Ask::File(dialog) }; + self.ask(ask, target, move |app, paths| match pick_for { + PickFor::Open => paths.iter().for_each(|p| app.open_path(&p.to_string_lossy())), + PickFor::Files(purpose) => app.use_paths(purpose, &paths), + }); + } + + /// Show `ask` without blocking the frame, and call `then` with the chosen paths on a later + /// frame ([`Self::process_picked`]). Returns false, after telling the user, when no picker + /// could be shown: another one is still open, or its worker couldn't start. + /// + /// `then` is not called when the picker is cancelled, or when `target` is given and by then + /// that document is no longer the active one or has been edited (the user is told). Use + /// `target` whenever `then` edits the active document or refers to its pages or objects by + /// position. + /// + /// `pick_override` answers instead of a native panel (tests and automation); the answer + /// still arrives on a later frame, like a real pick. + pub(crate) fn ask(&mut self, ask: Ask, target: Option, then: impl FnOnce(&mut PdfCraftApp, Vec) + Send + 'static) -> bool { + if self.pickers.showing.swap(true, Ordering::SeqCst) { + self.notify_tr("Another file dialog is still open. Finish with it first."); + return false; } - // Creating the future shows the panel; it must happen here, on the main thread. - let started = if multiple { - let pick = dialog.pick_files(); - self.pickers.spawn(pick_for, target, self.ctx.clone(), async move { - pick.await.unwrap_or_default().into_iter().map(|h| h.path().to_path_buf()).collect() - }) - } else { - let pick = dialog.pick_file(); - self.pickers.spawn(pick_for, target, self.ctx.clone(), async move { pick.await.map(|h| h.path().to_path_buf()).into_iter().collect() }) + let target = target.map(|doc| Target { doc, generation: self.session.get(doc).map(|d| d.edit_generation()) }); + let then: Then = Box::new(then); + if let Some(paths) = self.pick_override.clone() { + self.pickers.deliver(target, paths.into_iter().map(PathBuf::from).collect(), then); + return true; + } + // On macOS creating the future shows the panel; it must happen here, on the main thread. + let ctx = self.ctx.clone(); + let one = |h: Option| h.map(|h| h.path().to_path_buf()).into_iter().collect::>(); + let started = match ask { + Ask::File(d) => { + let pick = d.pick_file(); + self.pickers.spawn(target, ctx, async move { one(pick.await) }, then) + } + Ask::Folder(d) => { + let pick = d.pick_folder(); + self.pickers.spawn(target, ctx, async move { one(pick.await) }, then) + } + Ask::Save(d) => { + let pick = d.save_file(); + self.pickers.spawn(target, ctx, async move { one(pick.await) }, then) + } + Ask::Files(d) => { + let pick = d.pick_files(); + self.pickers.spawn( + target, + ctx, + async move { pick.await.unwrap_or_default().into_iter().map(|h| h.path().to_path_buf()).collect() }, + then, + ) + } }; if !started { self.pickers.showing.store(false, Ordering::SeqCst); self.notify_tr("Couldn't show the file picker. Please try again."); } + started + } + + /// [`Self::ask`] for one path: `then` gets the first chosen path. + pub(crate) fn ask_one(&mut self, ask: Ask, target: Option, then: impl FnOnce(&mut PdfCraftApp, PathBuf) + Send + 'static) -> bool { + self.ask(ask, target, move |app, paths| { + if let Some(path) = paths.into_iter().next() { + then(app, path); + } + }) } /// Use the picks that finished since the last frame. Called every frame. pub(crate) fn process_picked(&mut self) { - for Picked { pick_for, target, paths } in self.pickers.take() { + for Picked { target, paths, then } in self.pickers.take() { + // The pick is in hand: the next picker may show, including one `then` asks for. + self.pickers.showing.store(false, Ordering::SeqCst); if paths.is_empty() { continue; } - if target.is_some() && self.active_ids().map(|(_, id)| id) != target { - self.notify_tr("The document changed while you were choosing a file, so nothing was added."); - continue; + if let Some(t) = target { + let active = self.active_ids().map(|(_, id)| id); + let generation = self.session.get(t.doc).map(|d| d.edit_generation()); + if active != Some(t.doc) || generation != t.generation { + self.notify_tr("The document changed while you were choosing a file, so nothing was changed."); + continue; + } } - match pick_for { - PickFor::Open => paths.iter().for_each(|p| self.open_path(&p.to_string_lossy())), - PickFor::Files(purpose) => self.use_paths(purpose, &paths), + // Last-resort guard (AGENTS.md §4), as for commands: this work used to run inside + // the command that asked, and a panic in it must not take the app down. + if let Err(m) = pdfcraft_engine::guard(|| then(self, paths)) { + self.notify_fmt("That didn't work: an internal error stopped it ({m}).", &[("m", m.as_str())]); } } } @@ -150,17 +258,23 @@ impl PdfCraftApp { mod tests { use super::*; + fn record(into: &Arc>>) -> Then { + let into = into.clone(); + Box::new(move |_, paths| into.lock().unwrap().extend(paths)) + } + #[test] - fn worker_queues_the_pick_and_clears_showing() { - let pickers = Pickers::default(); - pickers.showing.store(true, Ordering::SeqCst); - let worker = pickers.spawn_worker(PickFor::Open, None, None, async { vec![PathBuf::from("a.pdf")] }).expect("worker starts"); + fn worker_queues_the_pick_and_keeps_the_flag_until_the_frame_uses_it() { + let mut app = PdfCraftApp::new(); + app.pickers.showing.store(true, Ordering::SeqCst); + let got = Arc::new(Mutex::new(Vec::new())); + let worker = app.pickers.spawn_worker(None, None, async { vec![PathBuf::from("a.pdf")] }, record(&got)).expect("worker starts"); // Wait for the worker itself: polling with a deadline was flaky on loaded CI runners. assert!(worker.join().is_ok()); - assert!(!pickers.showing.load(Ordering::SeqCst)); - let picked = pickers.take(); - assert_eq!(picked.len(), 1); - assert_eq!(picked[0].paths, vec![PathBuf::from("a.pdf")]); + assert!(app.pickers.showing.load(Ordering::SeqCst), "the queued pick still owns the flag"); + app.process_picked(); + assert!(!app.pickers.showing.load(Ordering::SeqCst)); + assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("a.pdf")]); } #[test] @@ -170,10 +284,66 @@ mod tests { fn fail() -> Vec { panic!("picker failed") } - let worker = pickers.spawn_worker(PickFor::Open, None, None, async { fail() }).expect("worker starts"); + let got = Arc::new(Mutex::new(Vec::new())); + let worker = pickers.spawn_worker(None, None, async { fail() }, record(&got)).expect("worker starts"); // join returns once the panic has unwound and the drop guard has run. assert!(worker.join().is_err()); assert!(!pickers.showing.load(Ordering::SeqCst)); assert!(pickers.take().is_empty()); } + + #[test] + fn the_follow_up_runs_on_a_later_frame_and_not_when_cancelled() { + let mut app = PdfCraftApp::new(); + let got = Arc::new(Mutex::new(Vec::new())); + app.pickers.deliver(None, vec![], record(&got)); + app.pickers.deliver(None, vec![PathBuf::from("b.pdf")], record(&got)); + assert!(got.lock().unwrap().is_empty(), "nothing runs until the frame uses the picks"); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("b.pdf")], "a cancelled pick runs nothing"); + } + + #[test] + fn a_second_picker_is_refused_while_one_is_showing() { + let mut app = PdfCraftApp::new(); + app.pick_override = Some(vec!["c.pdf".into()]); + app.pickers.showing.store(true, Ordering::SeqCst); + let got = Arc::new(Mutex::new(Vec::new())); + let into = got.clone(); + assert!(!app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, move |_, paths| into.lock().unwrap().extend(paths))); + app.process_picked(); + assert!(got.lock().unwrap().is_empty()); + assert!(app.pickers.showing.load(Ordering::SeqCst), "the showing picker still owns the flag"); + assert!(app.toast.as_ref().is_some_and(|(m, _)| m.contains("Another file dialog")), "the user is told"); + } + + #[test] + fn a_follow_up_that_asks_again_gets_its_picker_even_if_another_request_came_first() { + let mut app = PdfCraftApp::new(); + app.pick_override = Some(vec!["first.fdf".into()]); + let got = Arc::new(Mutex::new(Vec::new())); + let into = got.clone(); + assert!(app.ask(Ask::Files(rfd::AsyncFileDialog::new()), None, move |app, _| { + // Merge data: the spreadsheet's save picker, asked from the first pick's closure. + assert!(app.ask(Ask::Save(rfd::AsyncFileDialog::new()), None, move |_, paths| into.lock().unwrap().extend(paths))); + })); + // Another command asks for a picker before the frame uses the first pick: refused. + assert!(!app.ask(Ask::File(rfd::AsyncFileDialog::new()), None, |_, _| panic!("must not run"))); + app.process_picked(); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("first.fdf")], "the chained picker answered"); + } + + #[test] + fn a_panicking_follow_up_is_reported_and_the_next_pick_still_runs() { + let mut app = PdfCraftApp::new(); + let got = Arc::new(Mutex::new(Vec::new())); + app.pickers.deliver(None, vec![PathBuf::from("x.pdf")], Box::new(|_, _| panic!("follow-up failed"))); + app.pickers.deliver(None, vec![PathBuf::from("y.pdf")], record(&got)); + app.process_picked(); + assert_eq!(*got.lock().unwrap(), vec![PathBuf::from("y.pdf")]); + let toast = app.toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert!(toast.contains("internal error"), "the user is told: {toast:?}"); + assert!(!app.pickers.showing.load(Ordering::SeqCst)); + } } diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index 76fcb7c954..e7428e5b35 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -143,7 +143,9 @@ impl PdfCraftApp { self.dialog = Some(crate::Dialog::Print); } - /// Print (or save) with the dialog's settings. Returns `true` on success. + /// Print (or save) with the dialog's settings. Returns `true` on success. Save as PDF without + /// a preset path returns `true` once the save picker is showing; the file is written on a + /// later frame, when the user has chosen where. pub fn print_now(&mut self) -> bool { let Some((_, id)) = self.active_ids() else { return false }; let Some(doc) = self.session.get(id) else { return false }; @@ -179,26 +181,29 @@ impl PdfCraftApp { } }, None => { - let path = match self.save_override.clone() { - Some(p) => Some(std::path::PathBuf::from(p)), - #[cfg(not(target_arch = "wasm32"))] - None => { - let stem = name.trim_end_matches(".pdf").trim_end_matches(".PDF"); - rfd::FileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(format!("{stem} (print).pdf")).save_file() - } - #[cfg(target_arch = "wasm32")] - None => None, - }; - let Some(path) = path else { return false }; - match std::fs::write(&path, &bytes) { + let write = move |app: &mut Self, path: std::path::PathBuf| match crate::editing::write_atomically(&path.to_string_lossy(), &bytes) { Ok(()) => { - self.notify_fmt("Saved the print-ready PDF to {path}", &[("path", &path.display().to_string())]); + app.notify_fmt("Saved the print-ready PDF to {path}", &[("path", &path.display().to_string())]); true } Err(e) => { - self.notify_fmt("Could not save: {e}", &[("e", &e.to_string())]); + app.notify_fmt("Could not save: {e}", &[("e", &e.to_string())]); false } + }; + match self.save_override.clone() { + Some(p) => write(self, p.into()), + #[cfg(not(target_arch = "wasm32"))] + None => { + // Saved on a later frame, once the user has chosen where. + let stem = name.trim_end_matches(".pdf").trim_end_matches(".PDF"); + let dialog = rfd::AsyncFileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(format!("{stem} (print).pdf")); + self.ask_one(crate::pickers::Ask::Save(dialog), None, move |app, path| { + write(app, path); + }) + } + #[cfg(target_arch = "wasm32")] + None => false, } } } diff --git a/crates/ui-egui/src/sign_ui.rs b/crates/ui-egui/src/sign_ui.rs index d7b5f80270..bb3314e8c9 100644 --- a/crates/ui-egui/src/sign_ui.rs +++ b/crates/ui-egui/src/sign_ui.rs @@ -291,26 +291,43 @@ impl PdfCraftApp { appearance: d.appearance.clone(), ..SignOptions::default() }; - let signed = self.session.sign(doc_id, &id, opts).map_err(|e| e.to_string())?; - // Signing saves, as in Acrobat: choose where (a cancelled save cancels signing). + // Signing saves, as in Acrobat: choose where (a cancelled save cancels signing). The + // document is signed once the user has chosen. let name = self.session.get(doc_id).map(|d| d.name.clone()).unwrap_or_default(); let stem = name.trim_end_matches(".pdf").trim_end_matches(".PDF").to_string(); - let path = match self.save_override.clone() { - Some(p) => Some(PathBuf::from(p)), - #[cfg(not(target_arch = "wasm32"))] - None => rfd::FileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(format!("{stem}_signed.pdf")).save_file(), - #[cfg(target_arch = "wasm32")] - None => None, + let sign_and_save = move |app: &mut Self, path: PathBuf| -> Result<(), String> { + let signed = app.session.sign(doc_id, &id, opts).map_err(|e| e.to_string())?; + crate::editing::write_atomically(&path.to_string_lossy(), signed.as_slice()).map_err(|e| format!("Could not save: {e}"))?; + app.session.mark_signed(doc_id, signed, Some(path.to_string_lossy().into_owned())).map_err(|e| e.to_string())?; + if let Some(view) = app.views.iter_mut().find(|v| v.id == doc_id) { + view.invalidate_content(); + } + app.right = Some(crate::RightPanel::Signatures); + app.notify_fmt("Signed and saved to {path}", &[("path", &path.display().to_string())]); + Ok(()) }; - let Some(path) = path else { return Err(String::new()) }; - std::fs::write(&path, signed.as_slice()).map_err(|e| format!("Could not save: {e}"))?; - self.session.mark_signed(doc_id, signed, Some(path.to_string_lossy().into_owned())).map_err(|e| e.to_string())?; - if let Some((i, _)) = self.active_ids() { - self.views[i].invalidate_content(); + match self.save_override.clone() { + Some(p) => sign_and_save(self, PathBuf::from(p)), + #[cfg(not(target_arch = "wasm32"))] + None => { + let dialog = rfd::AsyncFileDialog::new().add_filter("PDF", &["pdf"]).set_file_name(format!("{stem}_signed.pdf")); + // The signature's page, rectangle and field refer to the document as it is now: + // the pick is dropped (with a notice) if it is edited or switched away from + // meanwhile. Once the picker shows, the dialog closes; an error from here on + // arrives as a notice. If no picker could show, the dialog stays open. + let asked = self.ask_one(crate::pickers::Ask::Save(dialog), Some(doc_id), move |app, path| { + if let Err(e) = sign_and_save(app, path) { + app.notify(e); + } + }); + if asked { Ok(()) } else { Err(String::new()) } + } + #[cfg(target_arch = "wasm32")] + None => { + let _ = (stem, sign_and_save); + Err(String::new()) + } } - self.right = Some(crate::RightPanel::Signatures); - self.notify_fmt("Signed and saved to {path}", &[("path", &path.display().to_string())]); - Ok(()) } /// Trust a certificate (Signatures panel ▸ Add to trusted certificates), revalidating. @@ -443,6 +460,9 @@ fn configure(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { let Some(d) = app.sign_draft.as_mut() else { return true }; let mut close = false; let mut go = false; + // Browse… is desktop-only. + #[cfg_attr(target_arch = "wasm32", allow(unused_mut))] + let mut browse = false; ui.radio_value(&mut d.new_id.create, false, tl!("Use a Digital ID from a file")); ui.radio_value(&mut d.new_id.create, true, tl!("Create a new Digital ID (self-signed, saved to a password-protected file)")); ui.add_space(8.0); @@ -487,10 +507,8 @@ fn configure(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { ui.horizontal(|ui| { ui.add(egui::TextEdit::singleline(&mut n.file).desired_width(220.0)).labelled_by(l.id); #[cfg(not(target_arch = "wasm32"))] - if ui.button(tl!("Browse…")).clicked() - && let Some(p) = rfd::FileDialog::new().add_filter(tl!("Digital ID"), &["p12", "pfx"]).pick_file() - { - n.file = p.to_string_lossy().into_owned(); + if ui.button(tl!("Browse…")).clicked() { + browse = true; } }); ui.end_row(); @@ -529,6 +547,22 @@ fn configure(ui: &mut egui::Ui, app: &mut PdfCraftApp, t: &Tokens) -> bool { Err(e) => d.error = Some(e), } } + // The picker answers on a later frame, into the draft if the same dialog is still open. + #[cfg(not(target_arch = "wasm32"))] + if browse { + let dialog = rfd::AsyncFileDialog::new().add_filter(tl!("Digital ID"), &["p12", "pfx"]); + let epoch = app.dialog_epoch(); + app.ask_one(crate::pickers::Ask::File(dialog), None, move |app, p| { + // Not one closed meanwhile, or opened again since. + if app.dialog_epoch() == epoch + && let Some(d) = app.sign_draft.as_mut() + { + d.new_id.file = p.to_string_lossy().into_owned(); + } + }); + } + #[cfg(target_arch = "wasm32")] + let _ = browse; close } diff --git a/crates/ui-egui/src/stamps_ui.rs b/crates/ui-egui/src/stamps_ui.rs index e80a37bfe5..4288c8892d 100644 --- a/crates/ui-egui/src/stamps_ui.rs +++ b/crates/ui-egui/src/stamps_ui.rs @@ -133,21 +133,22 @@ impl PdfCraftApp { pub(crate) fn pick_stamp_file(&mut self) { #[cfg(not(target_arch = "wasm32"))] { - let picked = match self.save_override.clone() { - Some(p) if [".png", ".jpg", ".pdf"].iter().any(|e| p.ends_with(e)) => Some(std::path::PathBuf::from(p)), - Some(_) => None, - None => rfd::FileDialog::new() - .add_filter(tl!("PDF or image"), &["pdf", "png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) - .set_title(tl!("Select a file for the stamp")) - .pick_file(), - }; - let Some(path) = picked else { return }; - match std::fs::read(&path) { + let read = |app: &mut Self, path: std::path::PathBuf| match std::fs::read(&path) { Ok(bytes) => { let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - self.start_custom_stamp(name, bytes); + app.start_custom_stamp(name, bytes); + } + Err(e) => app.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), + }; + match self.save_override.clone() { + Some(p) if [".png", ".jpg", ".pdf"].iter().any(|e| p.ends_with(e)) => read(self, p.into()), + Some(_) => {} + None => { + let dialog = rfd::AsyncFileDialog::new() + .add_filter(tl!("PDF or image"), &["pdf", "png", "jpg", "jpeg", "tif", "tiff", "gif", "bmp", "jp2", "j2k", "jpx"]) + .set_title(tl!("Select a file for the stamp")); + self.ask_one(crate::pickers::Ask::File(dialog), None, read); } - Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &path.display().to_string()), ("e", &e.to_string())]), } } #[cfg(target_arch = "wasm32")] diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index f5b81dda4f..eaa3020a91 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -396,7 +396,7 @@ fn save_prompt_stays_inside_the_screen_for_a_long_filename() { let mut h = Harness::builder().with_size(egui::vec2(1365.0, 719.0)).build_eframe(move |_cc| { let mut app = PdfCraftApp::new(); app.open_bytes(name, None, fixture(1)).expect("fixture opens"); - app.close_request = Some(CloseRequest::Tab(0)); + app.close_request = Some(CloseRequest::Tab(app.views[0].id)); app }); h.run_steps(4); diff --git a/crates/ui-egui/tests/pickers.rs b/crates/ui-egui/tests/pickers.rs index e3b8084452..c9402462db 100644 --- a/crates/ui-egui/tests/pickers.rs +++ b/crates/ui-egui/tests/pickers.rs @@ -1,8 +1,11 @@ //! Native file pickers never block the frame. A blocking picker ran a nested run loop inside -//! winit's event handler on macOS, which crashed the app when the user clicked File ▸ Open. -//! Commands now return at once, and the chosen files are used on a later frame. +//! winit's event handler on macOS, which crashed the app when the user clicked File ▸ Open (or +//! any other open, save or folder picker). Commands now return at once, and the chosen files are +//! used on a later frame. `pick_override` stands in for the native panel and, like it, answers +//! on a later frame. use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; use pdfcraft_render::PageRenderer; use pdfcraft_ui_egui::PdfCraftApp; @@ -104,3 +107,238 @@ fn a_pick_for_a_document_that_is_no_longer_active_is_dropped() { assert!(toast.contains("document changed"), "the user is told why nothing happened: {toast:?}"); let _ = std::fs::remove_dir_all(&dir); } + +/// A scratch folder for one test. +fn scratch(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("pdfcraft-pickers-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn page_count(bytes: Vec) -> usize { + PageRenderer::new(std::sync::Arc::new(bytes), Default::default()).page_count() +} + +fn dirty(app: &PdfCraftApp, view: usize) -> bool { + app.session.get(app.views[view].id).is_some_and(|d| d.dirty) +} + +fn rotate_first_page(app: &mut PdfCraftApp) { + app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 }); +} + +#[test] +fn save_as_writes_on_a_later_frame_including_edits_made_meanwhile() { + let dir = scratch("save-as"); + let out = dir.join("saved.pdf"); + let mut h = harness(&[3]); + h.state_mut().pick_override = Some(vec![out.to_string_lossy().into_owned()]); + assert!(h.state_mut().execute("file.save_as")); + assert!(!out.exists(), "the command must not wait for the picker"); + // The user keeps working while the panel is open. + h.state_mut().apply_edit(pdfcraft_engine::Edit::DeletePages { pages: vec![0] }); + h.run_steps(2); + assert_eq!(page_count(std::fs::read(&out).unwrap()), 2, "the edit made while choosing is saved too"); + assert!(!dirty(h.state(), 0)); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn save_on_close_closes_the_tab_only_once_the_file_is_written() { + let dir = scratch("save-close"); + let out = dir.join("closed.pdf"); + let mut h = harness(&[2]); + rotate_first_page(h.state_mut()); + h.state_mut().request_close_tab(0); + assert!(h.state().close_request.is_some(), "a dirty document asks first"); + h.state_mut().pick_override = Some(vec![out.to_string_lossy().into_owned()]); + let ctx = h.ctx.clone(); + h.state_mut().resolve_close(&ctx, Some(true)); + assert_eq!(h.state().views.len(), 1, "the tab stays open until the save is written"); + h.run_steps(2); + assert!(out.exists()); + assert!(h.state().views.is_empty(), "then it closes"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_cancelled_save_on_close_keeps_the_tab_open() { + let mut h = harness(&[2]); + rotate_first_page(h.state_mut()); + h.state_mut().request_close_tab(0); + h.state_mut().pick_override = Some(vec![]); // the user cancels the save panel + let ctx = h.ctx.clone(); + h.state_mut().resolve_close(&ctx, Some(true)); + h.run_steps(2); + assert_eq!(h.state().views.len(), 1); + assert!(dirty(h.state(), 0), "nothing was saved or lost"); +} + +#[test] +fn close_all_saves_each_untitled_document_in_turn() { + let dir = scratch("close-all"); + let mut h = harness(&[2, 3]); + for i in 0..2 { + h.state_mut().active = Some(i); + rotate_first_page(h.state_mut()); + } + h.state_mut().close_all(); + let ctx = h.ctx.clone(); + for (n, name) in ["first.pdf", "second.pdf"].iter().enumerate() { + assert!(h.state().close_request.is_some(), "asks about document {n}"); + h.state_mut().pick_override = Some(vec![dir.join(name).to_string_lossy().into_owned()]); + h.state_mut().resolve_close(&ctx, Some(true)); + h.run_steps(2); + } + assert!(h.state().views.is_empty()); + assert_eq!(page_count(std::fs::read(dir.join("first.pdf")).unwrap()), 2); + assert_eq!(page_count(std::fs::read(dir.join("second.pdf")).unwrap()), 3); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn extract_and_delete_deletes_the_pages_only_after_their_files_are_written() { + let dir = scratch("extract"); + let mut h = harness(&[3]); + h.state_mut().extract_draft.separate = true; + h.state_mut().extract_draft.delete = true; + h.state_mut().pick_override = Some(vec![dir.to_string_lossy().into_owned()]); + h.state_mut().extract_selection(); + assert_eq!(pages(h.state(), 0), 3, "nothing is deleted while the folder picker is open"); + h.run_steps(2); + assert!(dir.join("doc0 (page 1).pdf").exists()); + assert_eq!(pages(h.state(), 0), 2, "the extracted page is deleted once written"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_cancelled_folder_pick_extracts_and_deletes_nothing() { + let mut h = harness(&[3]); + h.state_mut().extract_draft.separate = true; + h.state_mut().extract_draft.delete = true; + h.state_mut().pick_override = Some(vec![]); + h.state_mut().extract_selection(); + h.run_steps(2); + assert_eq!(pages(h.state(), 0), 3); +} + +#[test] +fn an_image_pick_for_a_document_that_is_no_longer_active_changes_nothing() { + let dir = scratch("image-switch"); + let file = dir.join("picture.png"); + std::fs::write(&file, b"not really a png").unwrap(); + let mut h = harness(&[1, 1]); + h.state_mut().active = Some(0); + h.state_mut().pick_override = Some(vec![file.to_string_lossy().into_owned()]); + h.state_mut().add_image_dialog(); + h.state_mut().active = Some(1); + h.run_steps(2); + assert!(!dirty(h.state(), 0) && !dirty(h.state(), 1), "neither document changed"); + let toast = h.state().toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert!(toast.contains("document changed"), "the user is told why nothing happened: {toast:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn export_comments_writes_on_a_later_frame() { + let dir = scratch("export-data"); + let out = dir.join("data.xfdf"); + let mut h = harness(&[1]); + h.state_mut().pick_override = Some(vec![out.to_string_lossy().into_owned()]); + h.state_mut().export_data_dialog(true, false); + assert!(!out.exists(), "the command must not wait for the picker"); + h.run_steps(2); + assert!(std::fs::read_to_string(&out).unwrap().contains("xfdf")); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn extracted_pages_are_not_deleted_if_the_document_was_edited_while_choosing_the_folder() { + let dir = scratch("extract-edited"); + let mut h = harness(&[3]); + h.state_mut().extract_draft.separate = true; + h.state_mut().extract_draft.delete = true; + h.state_mut().pick_override = Some(vec![dir.to_string_lossy().into_owned()]); + h.state_mut().extract_selection(); + // While the folder picker is open the user deletes page 1 themselves: the page that would + // now be deleted is the original page 2, which was never written out. + h.state_mut().apply_edit(pdfcraft_engine::Edit::DeletePages { pages: vec![0] }); + h.run_steps(2); + assert!(dir.join("doc0 (page 1).pdf").exists(), "the extracted file is still written"); + assert_eq!(pages(h.state(), 0), 2, "nothing more is deleted"); + let toast = h.state().toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert!(toast.contains("not deleted"), "the user is told: {toast:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_close_prompt_still_means_its_own_document_after_a_pending_save_closes_another_tab() { + let dir = scratch("close-shift"); + let mut h = harness(&[1, 2, 3]); + for i in 0..3 { + h.state_mut().active = Some(i); + rotate_first_page(h.state_mut()); + } + let (b, c) = (h.state().views[1].id, h.state().views[2].id); + // Save-and-close the first tab; its save panel is still open... + h.state_mut().request_close_tab(0); + h.state_mut().pick_override = Some(vec![dir.join("a.pdf").to_string_lossy().into_owned()]); + let ctx = h.ctx.clone(); + h.state_mut().resolve_close(&ctx, Some(true)); + // ...when the user asks to close the second one. + h.state_mut().request_close_tab(1); + h.run_steps(2); + assert_eq!(h.state().views.len(), 2, "the first tab closed once saved"); + // Don't Save: the prompt was about the second document, which is now the first tab. + h.state_mut().resolve_close(&ctx, Some(false)); + let open: Vec<_> = h.state().views.iter().map(|v| v.id).collect(); + assert_eq!(open, vec![c], "the second document closed, not the third: {b:?} vs {c:?}"); + assert!(dirty(h.state(), 0), "the third document keeps its unsaved changes"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn an_image_pick_is_dropped_if_the_document_was_edited_meanwhile() { + let dir = scratch("image-edited"); + let file = dir.join("picture.png"); + std::fs::write(&file, b"not really a png").unwrap(); + let mut h = harness(&[2]); + h.state_mut().pick_override = Some(vec![file.to_string_lossy().into_owned()]); + // Replace image 0 on page 2... + h.state_mut().replace_image_dialog(1, 0); + // ...but page 1 is deleted while choosing: "page 2" is now a different page. + h.state_mut().apply_edit(pdfcraft_engine::Edit::DeletePages { pages: vec![0] }); + h.run_steps(2); + assert_eq!(pages(h.state(), 0), 1); + let toast = h.state().toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert!(toast.contains("document changed"), "the pick is dropped and the user told why: {toast:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_watermark_file_from_browse_lands_only_in_the_dialog_that_asked() { + let dir = scratch("browse"); + let file = dir.join("mark.pdf"); + std::fs::write(&file, fixture(1)).unwrap(); + for keep_open in [true, false] { + let mut h = harness(&[1]); + assert!(h.state_mut().execute("edit.watermark")); + h.state_mut().marks_draft.use_file = true; + h.state_mut().pick_override = Some(vec![file.to_string_lossy().into_owned()]); + h.run_steps(2); + h.get_by_label("Browse…").click(); + h.run_steps(1); + if !keep_open { + h.state_mut().dialog = None; // closed before the pick arrives + } + h.run_steps(2); + let got = h.state().marks_draft.file.as_ref().map(|f| f.0.clone()); + if keep_open { + assert_eq!(got.as_deref(), Some("mark.pdf"), "the open dialog gets the file"); + } else { + assert_eq!(got, None, "a closed dialog's pick is dropped"); + } + } + let _ = std::fs::remove_dir_all(&dir); +}