From 4fa26f4a248e00dac6aa4b13db6e45bbf5bf78be Mon Sep 17 00:00:00 2001 From: mmmaarian03-crypto Date: Mon, 9 Nov 2026 04:04:17 +0200 Subject: [PATCH] Reload: a file changed under an open dialog waits for it to close (#3917) (#5278) --- crates/ui-egui/src/platform/file_watch.rs | 27 ++++++++- crates/ui-egui/tests/it/reload.rs | 73 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/platform/file_watch.rs b/crates/ui-egui/src/platform/file_watch.rs index 57de3428a3..49013d1b74 100644 --- a/crates/ui-egui/src/platform/file_watch.rs +++ b/crates/ui-egui/src/platform/file_watch.rs @@ -288,6 +288,9 @@ pub struct FileWatch { epoch: HashMap, /// The notice bar's Reload was pressed for this document. pub reload_request: Option, + /// The newest version found for the active document while a dialog was open over it, + /// applied when the dialog closes (#3917). + held: Option, } impl FileWatch { @@ -363,12 +366,27 @@ impl PdfCraftApp { let (found, _) = self.watch.inline.tick(); self.watch.found.lock().unwrap_or_else(PoisonError::into_inner).extend(found); } - let found = std::mem::take(&mut *self.watch.found.lock().unwrap_or_else(PoisonError::into_inner)); + let mut found = std::mem::take(&mut *self.watch.found.lock().unwrap_or_else(PoisonError::into_inner)); + if !self.holds_reload() + && let Some(held) = self.watch.held.take() + { + // Dropped when reloading was turned off meanwhile (in Preferences, the dialog itself) + // or when a newer version of the same document was just found: only that one loads. + if self.reload_changed_files && !found.iter().any(|c| c.doc == held.doc) { + found.insert(0, held); + } + } for c in found { self.file_changed(c); } } + /// A dialog or prompt about the active document is open: its new version waits (#3917). The + /// full-screen prompt is not modal elsewhere, but it is about the version on screen. + fn holds_reload(&self) -> bool { + self.modal_open() || self.pending_full_screen.is_some() + } + /// Hand `list` to the watcher, starting it when needed. `false`: not delivered (no context /// yet, or the worker couldn't start or ended); the next frame tries again. fn send_to_watcher(&mut self, list: Vec) -> bool { @@ -400,6 +418,13 @@ impl PdfCraftApp { /// changes. fn file_changed(&mut self, c: Changed) { let Some(i) = self.views.iter().position(|v| v.id == c.doc) else { return }; + // A dialog acts on the active document when its OK is pressed: replacing the document + // under it would apply the dialog to another version (#3917). Keep the newest version + // found until the dialog closes; the checks below run then. + if self.holds_reload() && self.active == Some(i) { + self.watch.held = Some(c); + return; + } // Found for an earlier reading or writing of the file, or for another file than the // document's now (a save, Save As or a reopen came between): older than the document in // memory, so it is not applied over it (#1802). The watcher was told the newer version. diff --git a/crates/ui-egui/tests/it/reload.rs b/crates/ui-egui/tests/it/reload.rs index d9e6b142b3..688b10c619 100644 --- a/crates/ui-egui/tests/it/reload.rs +++ b/crates/ui-egui/tests/it/reload.rs @@ -366,3 +366,76 @@ fn opening_a_file_that_is_open_focuses_its_tab() { let app = h.state(); assert_eq!((app.views.len(), app.active, app.views[0].id), (2, Some(0), id), "focused, not opened again"); } + +#[test] +fn a_file_rebuilt_under_an_open_dialog_reloads_when_the_dialog_closes() { + // #3917: a dialog's OK acts on the active document. Reloading it in place while the dialog is + // open would apply the dialog (Rotate, Protect, Comment Properties…) to the new version. + let file = TempPdf::new("under-dialog", &fixture(2)); + let mut h = harness(&file.path); + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::RotatePages); + h.run_steps(2); + file.rebuild(&fixture(4)); + h.run_steps(4); + assert_eq!(pages(&h), 2, "not replaced under the dialog"); + file.rebuild(&fixture(5)); + h.run_steps(4); + assert_eq!(pages(&h), 2, "still not replaced under the dialog"); + h.state_mut().dialog = None; + h.run_steps(3); + assert_eq!((pages(&h), dirty(&h)), (5, false), "the newest version is applied once the dialog closes"); +} + +#[test] +fn a_version_held_under_preferences_is_dropped_when_reloading_is_turned_off_there() { + // #3917 review: closing Preferences after unticking "Reload documents when their files + // change on disk" must not still apply the version found while it was open. + let file = TempPdf::new("held-pref-off", &fixture(2)); + let mut h = harness(&file.path); + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::RotatePages); + h.run_steps(2); + file.rebuild(&fixture(4)); + h.run_steps(4); + h.state_mut().reload_changed_files = false; + h.state_mut().dialog = None; + h.run_steps(4); + assert_eq!(pages(&h), 2, "reloading was turned off before the dialog closed"); +} + +#[test] +fn a_newer_version_found_as_the_dialog_closes_replaces_the_held_one() { + // #3917 review: only the newest version is applied, not the held one first (which would run + // its scripts, and whose dialogs could hold the newest one back again). Each reload is a new + // edit generation: one is expected. + let file = TempPdf::new("held-coalesced", &fixture(2)); + let mut h = harness(&file.path); + h.state_mut().dialog = Some(pdfcraft_ui_egui::Dialog::RotatePages); + h.run_steps(2); + file.rebuild(&fixture(4)); + h.run_steps(4); + // The watcher reads a changed file at its second look: one frame sees the change, and the + // frame that closes the dialog reads it, together with the held version. + file.rebuild(&fixture(5)); + h.step(); + let generation = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(h.state().views[0].id).unwrap().edit_generation(); + let before = generation(&h); + h.state_mut().dialog = None; + h.step(); + assert_eq!((pages(&h), generation(&h) - before), (5, 1), "the held version is not loaded on the way"); +} + +#[test] +fn a_file_rebuilt_under_the_full_screen_prompt_reloads_once_it_is_answered() { + // #3917 review: the full-screen prompt is about the document's current version. + let file = TempPdf::new("under-full-screen", &fixture(2)); + let mut h = harness(&file.path); + let id = h.state().views[0].id; + h.state_mut().pending_full_screen = Some(id); + h.run_steps(2); + file.rebuild(&fixture(4)); + h.run_steps(4); + assert_eq!(pages(&h), 2, "not replaced under the prompt"); + h.state_mut().pending_full_screen = None; + h.run_steps(3); + assert_eq!(pages(&h), 4, "applied once the prompt is gone"); +}