From 13d0cbc51f5cf1dca28b7d231e4dc16e2d6bd762 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 10:46:26 -0800 Subject: [PATCH] Preferences: the recovery folder check makes a folder, and a pick survives a closed document The Choose... check now creates and removes an empty folder in the picked folder, as the recovery store makes its own folder there, and reports a folder it cannot remove again. The folder picker's answer fills the open Preferences dialog even when the active document closed while the picker was open, since the recovery folder is an app preference. --- crates/ui-egui/src/picks.rs | 34 +++++++++++++++++++++++- crates/ui-egui/src/prefs_dialog.rs | 23 +++++++--------- crates/ui-egui/src/prefs_dialog/tests.rs | 29 +++++++++++++++++--- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/crates/ui-egui/src/picks.rs b/crates/ui-egui/src/picks.rs index fd5c4d61c..81253e83c 100644 --- a/crates/ui-egui/src/picks.rs +++ b/crates/ui-egui/src/picks.rs @@ -47,6 +47,9 @@ pub type PickAnswer = Vec; /// the dialog is then shown in line. pub type StartPick = Box Option>>; +/// A button's action that runs again with the picked path. +type Call = Rc; + /// What runs again with the picked path. #[derive(Clone)] pub(crate) enum Entry { @@ -56,6 +59,9 @@ pub(crate) enum Entry { Confirm(Box), /// A panel's or a dialog's button. Call(Rc), + /// A dialog's button that acts on no document (an app preference): its answer goes on even + /// when the document active at the time has closed meanwhile. + AppCall(Rc), } /// Commands that ask for a file but act on no document: their answer goes on even when the @@ -80,6 +86,7 @@ impl Entry { fn document(&self, app: &VectorcraftApp) -> Option { match self { Entry::Command(id, _) if DOCUMENT_FREE.contains(&id.as_str()) => None, + Entry::AppCall(_) => None, _ => app.session.active().map(|d| d.uid), } } @@ -193,6 +200,26 @@ pub(crate) fn in_dialog( app: &mut VectorcraftApp, d: &mut Dialog, f: impl Fn(&mut VectorcraftApp, &mut Dialog) -> Result<(), String> + 'static, +) -> Result<(), String> { + dialog_entry(app, d, Entry::Call, f) +} + +/// [`in_dialog`] for an action that acts on no document (an app preference): its answer runs `f` +/// again on the open dialog even when the document active at the time has closed meanwhile. +pub(crate) fn in_app_dialog( + app: &mut VectorcraftApp, + d: &mut Dialog, + f: impl Fn(&mut VectorcraftApp, &mut Dialog) -> Result<(), String> + 'static, +) -> Result<(), String> { + dialog_entry(app, d, Entry::AppCall, f) +} + +/// [`in_dialog`] with `wrap` making the entry (a [`Entry::Call`] or an [`Entry::AppCall`]). +fn dialog_entry( + app: &mut VectorcraftApp, + d: &mut Dialog, + wrap: fn(Call) -> Entry, + f: impl Fn(&mut VectorcraftApp, &mut Dialog) -> Result<(), String> + 'static, ) -> Result<(), String> { let f = Rc::new(move |app: &mut VectorcraftApp, d: &mut Dialog| { let r = f(app, d); @@ -204,7 +231,7 @@ pub(crate) fn in_dialog( }); let (again, kind) = (f.clone(), d.kind.clone()); let entry = move || { - Entry::Call(Rc::new(move |app: &mut VectorcraftApp| { + wrap(Rc::new(move |app: &mut VectorcraftApp| { let Some(mut d) = app.ui.dialog.take_if(|d| d.kind == kind) else { app.status(dialog_gone()); return; @@ -428,6 +455,11 @@ fn rerun(app: &mut VectorcraftApp, entry: Entry) { as_entry(app, move || Entry::Call(again), |app| f(app)); Ok(()) } + Entry::AppCall(f) => { + let again = f.clone(); + as_entry(app, move || Entry::AppCall(again), |app| f(app)); + Ok(()) + } }; if let Err(e) = r { app.status(e); diff --git a/crates/ui-egui/src/prefs_dialog.rs b/crates/ui-egui/src/prefs_dialog.rs index f3d5a7ae6..3aa48bc10 100644 --- a/crates/ui-egui/src/prefs_dialog.rs +++ b/crates/ui-egui/src/prefs_dialog.rs @@ -411,11 +411,12 @@ fn category_fields(ui: &mut egui::Ui, app: &mut VectorcraftApp, d: &mut Dialog, } } -/// Data Recovery › Choose…: the folder picked becomes the Folder field's value when files can be +/// Data Recovery › Choose…: the folder picked becomes the Folder field's value when a folder can be /// made in it. Otherwise the status bar says why and the field keeps its value, as it does when -/// the picker is cancelled. +/// the picker is canceled. The answer fills the field even when the active document closed while +/// the picker was open. fn choose_recovery_folder(app: &mut VectorcraftApp, d: &mut Dialog) { - let r = crate::picks::in_dialog(app, d, |app, d| { + let r = crate::picks::in_app_dialog(app, d, |app, d| { let Some(path) = crate::picks::folder(app) else { return Ok(()) }; writable_folder(&path)?; d.fields.insert("recoveryFolder".into(), json!(path)); @@ -426,7 +427,9 @@ fn choose_recovery_folder(app: &mut VectorcraftApp, d: &mut Dialog) { } } -/// `Ok` when `path` is a folder that a file can be created in: a file is made there and removed. +/// `Ok` when `path` is a folder that a folder can be created in, as the recovery store makes its +/// own there: an empty folder is made there and removed. A folder that can't be removed again is +/// an error too. fn writable_folder(path: &str) -> Result<(), String> { let dir = std::path::Path::new(path); if !dir.is_dir() { @@ -434,15 +437,9 @@ fn writable_folder(path: &str) -> Result<(), String> { } let nanos = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |t| t.as_nanos()); let check = dir.join(format!(".vectorcraft-write-check-{}-{nanos}", std::process::id())); - match std::fs::OpenOptions::new().write(true).create_new(true).open(&check) { - Ok(f) => { - drop(f); - // Best effort: the empty file is all there is to remove. - let _ = std::fs::remove_file(&check); - Ok(()) - } - Err(e) => Err(format!("Data Recovery can't keep copies in {path}: {e}")), - } + let refused = |e: std::io::Error| format!("Data Recovery can't keep copies in {path}: {e}"); + std::fs::create_dir(&check).map_err(refused)?; + std::fs::remove_dir(&check).map_err(refused) } /// Is one of the dialog's Units (General, Stroke, Type, East Asian Type) Picas? diff --git a/crates/ui-egui/src/prefs_dialog/tests.rs b/crates/ui-egui/src/prefs_dialog/tests.rs index 81a702da0..6e8095fcd 100644 --- a/crates/ui-egui/src/prefs_dialog/tests.rs +++ b/crates/ui-egui/src/prefs_dialog/tests.rs @@ -447,7 +447,7 @@ fn recovery_folder_has_a_choose_button_with_a_folder_picker() { assert!(text.contains("Data Recovery") && !text.contains("Choose…"), "{text}"); } -/// Choose… puts the folder picked into the Folder field, and OK saves it; a cancelled picker +/// Choose… puts the folder picked into the Folder field, and OK saves it; a canceled picker /// leaves the field as it was. #[test] fn a_chosen_recovery_folder_fills_the_field_and_cancel_keeps_it() { @@ -471,7 +471,7 @@ fn a_chosen_recovery_folder_fills_the_field_and_cancel_keeps_it() { let _ = std::fs::remove_dir_all(&dir); } -/// A picked folder that no file can be created in is refused: the field keeps its value and the +/// A picked folder that no folder can be created in is refused: the field keeps its value and the /// status bar says why. #[test] fn a_recovery_folder_that_cannot_be_written_is_refused() { @@ -497,7 +497,7 @@ fn a_recovery_folder_that_cannot_be_written_is_refused() { std::fs::create_dir(&locked).unwrap(); std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o555)).unwrap(); // An account that may write anyway (root) has nothing to refuse. - if std::fs::write(locked.join("probe"), b"").is_err() { + if std::fs::create_dir(locked.join("probe")).is_err() { let answer = locked.to_string_lossy().into_owned(); a.services.pick_folder = Some(Box::new(move || Some(answer.clone()))); a.ui.status.clear(); @@ -509,3 +509,26 @@ fn a_recovery_folder_that_cannot_be_written_is_refused() { } let _ = std::fs::remove_dir_all(&dir); } + +/// The recovery folder is an app preference: a folder picked off the UI thread (Linux) fills the +/// open dialog's field even when the active document closed while the picker was open. +#[test] +fn a_chosen_recovery_folder_fills_the_field_after_the_active_document_closes() { + let dir = temp_folder("recovery-closed"); + let picked = dir.to_string_lossy().into_owned(); + let mut a = VectorcraftApp::new(Session::new(), Default::default()); + let (shown, _) = crate::tests_picks::off_the_ui_thread(&mut a.services); + a.run("file.new", json!({"width": 100, "height": 80})).unwrap(); + open(&mut a, Some("File Handling")); + let mut d = a.ui.dialog.take().unwrap(); + choose_recovery_folder(&mut a, &mut d); + a.ui.dialog = Some(d); + assert!(matches!(shown.requests().as_slice(), [crate::picks::PickRequest::Folder])); + a.session.execute("file.close", &json!({})).unwrap(); + a.sync_views(); + assert!(a.session.documents().is_empty()); + shown.answer(&[&picked]); + crate::picks::poll(&mut a, &egui::Context::default()); + assert_eq!(a.ui.dialog.as_ref().unwrap().str("recoveryFolder"), picked, "{}", a.ui.status); + let _ = std::fs::remove_dir_all(&dir); +}