diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index c2f38a70c..f6ac2f538 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -44,15 +44,16 @@ fn app_icon() -> egui::IconData { eframe::icon_data::from_png_bytes(png).unwrap_or_default() } -struct App(LightcraftApp, #[cfg(target_os = "macos")] Option); +struct App(LightcraftApp, PrefsWriter, #[cfg(target_os = "macos")] Option); impl eframe::App for App { fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { #[cfg(target_os = "macos")] - if let Some(m) = self.1.as_mut() { + if let Some(m) = self.2.as_mut() { m.update(&mut self.0, ctx); } self.0.logic(ctx); + self.1.tick(&mut self.0, ctx); } fn raw_input_hook(&mut self, _ctx: &egui::Context, raw: &mut egui::RawInput) { self.0.raw_input_hook(raw); @@ -61,7 +62,9 @@ impl eframe::App for App { self.0.ui(ui); } fn on_exit(&mut self) { - save_prefs(&self.0); + if let Err(e) = self.1.save(&self.0) { + eprintln!("lightcraft: {e}"); + } if let Err(e) = self.0.session.close_library() { eprintln!("lightcraft: saving the library failed: {e}"); } @@ -81,23 +84,118 @@ fn config_dir() -> Option { } } -/// The saved UI state and app settings (`/ui.json`), if any. -fn load_prefs() -> Option { +/// The saved UI state and app settings (`/ui.json`), if any, and a warning for the user +/// when the file exists but can't be used (issue #103): a damaged file is kept as +/// `ui.json.corrupt-` first, so the next save can't lose the library location in it; +/// one that can't be read at all is not written this session (`keep_file`). +fn load_prefs() -> (Option, Option, bool) { if std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() { - return None; + return (None, None, false); } - let bytes = std::fs::read(config_dir()?.join("ui.json")).ok()?; - serde_json::from_slice::(&bytes).ok().map(UiState::sanitized) + let Some(path) = config_dir().map(|d| d.join("ui.json")) else { return (None, None, false) }; + load_prefs_at(&path) } -fn save_prefs(app: &LightcraftApp) { - if std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() { - return; +fn load_prefs_at(path: &std::path::Path) -> (Option, Option, bool) { + let bytes = match std::fs::read(path) { + Ok(b) => b, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return (None, None, false), + Err(e) => { + let msg = format!("The app settings ({}) couldn't be read: {e}. Defaults are used, and the file isn't overwritten.", path.display()); + return (None, Some(msg), true); + } + }; + match serde_json::from_slice::(&bytes) { + Ok(ui) => (Some(ui.sanitized()), None, false), + Err(e) => { + let secs = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0); + let keep = path.with_file_name(format!("ui.json.corrupt-{secs}")); + let kept = std::fs::rename(path, &keep); + let what = match &kept { + Ok(()) => format!("it was kept as {}", keep.display()), + Err(r) => format!("it couldn't be set aside ({r})"), + }; + let msg = format!( + "The app settings ({}) are damaged ({e}); {what}. Defaults are used — reopen your library with Settings → Open Library… if it isn't shown.", + path.display() + ); + (None, Some(msg), kept.is_err()) + } } - if let Some(d) = config_dir() { - let _ = std::fs::create_dir_all(&d); - if let Ok(bytes) = serde_json::to_vec_pretty(&app.ui) { - let _ = std::fs::write(d.join("ui.json"), bytes); +} + +/// Write `bytes` to `path` atomically: a temp file, fsynced, renamed over it (a crash or a full +/// disk leaves the old file or the new one, never a truncated one). +fn write_atomic(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> { + use std::io::Write; + let tmp = path.with_extension("json.tmp"); + let written = (|| { + let mut f = std::fs::File::create(&tmp)?; + f.write_all(bytes)?; + f.sync_all() + })(); + if let Err(e) = written { + let _ = std::fs::remove_file(&tmp); + return Err(e); + } + std::fs::rename(&tmp, path)?; + if let Some(dir) = path.parent() + && let Ok(d) = std::fs::File::open(dir) + { + let _ = d.sync_all(); + } + Ok(()) +} + +/// Saves `ui.json` (app settings, incl. the library to open at launch): as soon as the library +/// changes, every few seconds when anything else changed, and at exit. +#[derive(Default)] +struct PrefsWriter { + written: Vec, + library: String, + checked: f64, + /// The last write failed (reported once until a write works). + failing: bool, + /// `ui.json` couldn't be read at launch: never overwrite it this session. + keep_file: bool, +} + +impl PrefsWriter { + fn save(&mut self, app: &LightcraftApp) -> Result<(), String> { + if self.keep_file || std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() { + return Ok(()); + } + let Some(d) = config_dir() else { return Ok(()) }; + let bytes = serde_json::to_vec_pretty(&app.ui).map_err(|e| e.to_string())?; + if bytes == self.written { + return Ok(()); + } + std::fs::create_dir_all(&d) + .and_then(|()| write_atomic(&d.join("ui.json"), &bytes)) + .map_err(|e| format!("saving the app settings failed: {e}"))?; + self.written = bytes; + self.library = app.ui.settings.library_path.clone(); + Ok(()) + } + + fn tick(&mut self, app: &mut LightcraftApp, ctx: &egui::Context) { + let now = ctx.input(|i| i.time); + let moved = app.ui.settings.library_path != self.library; + if !moved && now - self.checked < 3.0 { + return; + } + self.checked = now; + match self.save(app) { + Ok(()) => self.failing = false, + Err(e) => { + eprintln!("lightcraft: {e}"); + if !self.failing { + app.notices.push(format!("{e}. LightCraft keeps trying.")); + } + self.failing = true; + // don't retry every frame + self.library = app.ui.settings.library_path.clone(); + } } } } @@ -300,7 +398,7 @@ fn main() -> eframe::Result { _ => files.push(a), } } - let prefs = load_prefs(); + let (prefs, prefs_warning, keep_prefs_file) = load_prefs(); // --library, else the library last opened from Settings, else the default location let library_dir = library_dir.or_else(|| { prefs @@ -334,6 +432,14 @@ fn main() -> eframe::Result { app.ui = ui; } app.integrated_titlebar = cfg!(target_os = "macos"); + app.notices.extend(prefs_warning); + // what's on disk now: only changes are written + let writer = PrefsWriter { + written: serde_json::to_vec_pretty(&app.ui).unwrap_or_default(), + library: app.ui.settings.library_path.clone(), + keep_file: keep_prefs_file, + ..Default::default() + }; if let Some(port) = control_port { let rx = control_server::start(port, cc.egui_ctx.clone()); app = app.with_control(rx); @@ -352,9 +458,61 @@ fn main() -> eframe::Result { let menu = (std::env::var_os("LIGHTCRAFT_NO_NATIVE_MENU").is_none()).then(|| native_menu::NativeMenu::install(&mut app, &cc.egui_ctx)); Ok(Box::new(App( app, + writer, #[cfg(target_os = "macos")] menu, ))) }), ) } + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + fn dir(tag: &str) -> std::path::PathBuf { + let d = std::env::temp_dir().join(format!("lc-app-prefs-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d + } + + /// Issue #103: a damaged ui.json (which holds the library location) is kept aside and + /// reported, not silently replaced by the defaults. + #[test] + fn damaged_ui_json_is_kept_and_reported() { + let d = dir("damaged"); + let path = d.join("ui.json"); + std::fs::write(&path, br#"{"settings": {"library_path": "/Volumes/Photos/Lib"#).unwrap(); + let (ui, warning, keep) = load_prefs_at(&path); + assert!(ui.is_none() && !keep); + let warning = warning.unwrap(); + assert!(warning.contains("damaged") && warning.contains("ui.json.corrupt-"), "{warning}"); + assert!(!path.exists()); + let kept: Vec<_> = std::fs::read_dir(&d).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().to_string()).collect(); + assert!(kept.len() == 1 && kept[0].starts_with("ui.json.corrupt-"), "{kept:?}"); + // a missing file is just "no settings yet" + assert_eq!(load_prefs_at(&path).1, None); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn ui_json_is_written_atomically() { + let d = dir("atomic"); + let path = d.join("ui.json"); + std::fs::write(&path, b"old").unwrap(); + write_atomic(&path, b"{\"new\": true}").unwrap(); + assert_eq!(std::fs::read(&path).unwrap(), b"{\"new\": true}"); + assert!(!d.join("ui.json.tmp").exists()); + // a failed write leaves the old file + let missing = d.join("gone").join("ui.json"); + assert!(write_atomic(&missing, b"x").is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"{\"new\": true}"); + let ui = UiState::default(); + write_atomic(&path, &serde_json::to_vec_pretty(&ui).unwrap()).unwrap(); + let (loaded, warning, _) = load_prefs_at(&path); + assert!(loaded.is_some() && warning.is_none()); + let _ = std::fs::remove_dir_all(&d); + } +} diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 35a477470..7c81427c3 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -867,6 +867,8 @@ pub fn specs() -> Vec { "logRecords": j.log_records(), "logBytes": j.log_bytes(), "lastError": lib.last_error, + // settings files that were unreadable or damaged at open (kept, defaults used) + "settingsWarnings": lib.settings_warnings, // changes applied in memory whose write failed (retried by every save) "unsavedOps": s.unsaved().map_or(0, |u| u.0), "unsavedError": s.unsaved().map(|u| u.1), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3de92bf1a..2d68329f5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -645,6 +645,8 @@ mod tests_persist; #[cfg(test)] mod tests_prefs; #[cfg(test)] +mod tests_settings_files; +#[cfg(test)] mod tests_spots; #[cfg(test)] mod tests_xmp; diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index a068eaeb3..616cfb0f8 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -64,6 +64,12 @@ pub struct Library { pub forgot_local: Option, presets_written: String, view_written: Vec, + /// Settings files that were unreadable or damaged when the library opened (`library.info` → + /// `settingsWarnings`; shown by the UI once, see [`Session::take_library_warnings`]). + pub settings_warnings: Vec, + warnings_reported: usize, + /// Settings files that couldn't be read or set aside: never overwritten this session. + blocked: Vec<&'static str>, /// Keeps other processes out of this library while it's open (last: released after the /// journal's background snapshot has landed). lock: Option, @@ -191,8 +197,49 @@ fn program_name() -> String { } } -fn read_json(store: &mut dyn Store, name: &str) -> Option { - store.read(name).ok().flatten().and_then(|b| serde_json::from_slice(&b).ok()) +/// Problems reading the settings files (`presets.json`, `prefs.json`, `view.json`) when the library +/// opened (issue #103). A missing file is not a problem (defaults). A file that doesn't parse is +/// kept as `.corrupt-` before the defaults are used (so the next save can't +/// lose it); a file that can't be read at all (locked by another program, I/O error) is never +/// written this session, so its content survives until the library is opened again. +#[derive(Default)] +struct SettingsLoad { + warnings: Vec, + /// Files not to overwrite this session. + blocked: Vec<&'static str>, +} + +impl SettingsLoad { + fn read(&mut self, store: &mut dyn Store, name: &'static str) -> Option { + let bytes = match store.read(name) { + Ok(b) => b?, + Err(e) => { + log::error!("library: {name}: {e}"); + self.blocked.push(name); + self.warnings.push(format!( + "{name} couldn't be read ({e}). LightCraft uses the defaults for now and won't overwrite the file; reopen the library to try again." + )); + return None; + } + }; + let err = match serde_json::from_slice(&bytes) { + Ok(v) => return Some(v), + Err(e) => e, + }; + let secs = web_time::SystemTime::now().duration_since(web_time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0); + let keep = format!("{name}.corrupt-{secs}"); + log::error!("library: {name} is damaged: {err}"); + match store.write_atomic(&keep, &bytes) { + Ok(()) => self.warnings.push(format!("{name} is damaged ({err}). It was kept as {keep}, and the defaults are used.")), + Err(w) => { + self.blocked.push(name); + self.warnings.push(format!( + "{name} is damaged ({err}) and couldn't be set aside ({w}). LightCraft uses the defaults and won't overwrite the file." + )); + } + } + None + } } impl Session { @@ -259,8 +306,9 @@ impl Session { crate::demo::load(self); journal.snapshot(&self.catalog)?; } + let mut settings = SettingsLoad::default(); // presets - if let Some(f) = read_json::(files.as_mut(), "presets.json") { + if let Some(f) = settings.read::(files.as_mut(), "presets.json") { for p in &mut self.presets { p.favorite = p.builtin && f.favorites.contains(&p.id) || (!p.builtin && p.favorite); } @@ -274,7 +322,7 @@ impl Session { self.profile_recent = f.profile_recent.into_iter().filter(known).take(crate::presets::RECENT_PROFILES).collect(); } // preferences - let prefs = read_json::(files.as_mut(), "prefs.json").unwrap_or_default(); + let prefs = settings.read::(files.as_mut(), "prefs.json").unwrap_or_default(); self.xmp = prefs.xmp; self.last_export = prefs.last_export; self.export_presets = prefs.export_presets; @@ -295,7 +343,7 @@ impl Session { self.media.smart_dir = Some(d.clone()); } // view state - if let Some(v) = read_json::(files.as_mut(), "view.json") { + if let Some(v) = settings.read::(files.as_mut(), "view.json") { self.source = v.source; self.browse = v.browse; self.filter = v.filter; @@ -328,6 +376,9 @@ impl Session { forgot_local: None, presets_written, view_written, + settings_warnings: settings.warnings, + warnings_reported: 0, + blocked: settings.blocked, lock: lock.take(), }); // forget untouched Local records of folders not browsed for a while (journaled at once) @@ -381,7 +432,7 @@ impl Session { } let presets = presets_json(self); let Some(lib) = self.library.as_mut() else { return Ok(()) }; - if presets != lib.presets_written { + if presets != lib.presets_written && !lib.blocked.contains(&"presets.json") { if let Err(e) = lib.files.write_atomic("presets.json", presets.as_bytes()) { log::error!("library: presets: {e}"); } else { @@ -462,7 +513,7 @@ impl Session { } let v = self.view_json(); let Some(lib) = self.library.as_mut() else { return }; - if v != lib.view_written { + if v != lib.view_written && !lib.blocked.contains(&"view.json") { match lib.files.write_atomic("view.json", &v) { Ok(()) => lib.view_written = v, Err(e) => log::error!("library: view: {e}"), @@ -491,9 +542,22 @@ impl Session { }) .unwrap_or_default(); let Some(lib) = self.library.as_mut() else { return Ok(()) }; + if lib.blocked.contains(&"prefs.json") { + return Err(EngineError::Other( + "prefs: prefs.json couldn't be read when the library opened, so it isn't overwritten; reopen the library to save preferences".into(), + )); + } lib.files.write_atomic("prefs.json", &v).map_err(|e| EngineError::Other(format!("prefs: {e}"))) } + /// Settings-file warnings of the open library not handed out yet (the UI shows each once). + pub fn take_library_warnings(&mut self) -> Vec { + let Some(lib) = self.library.as_mut() else { return vec![] }; + let new = lib.settings_warnings[lib.warnings_reported..].to_vec(); + lib.warnings_reported = lib.settings_warnings.len(); + new + } + /// The thumbnail disk cache budget in bytes ([`Session::cache_mb`], else the default). pub fn cache_bytes(&self) -> u64 { if self.cache_mb == 0 { crate::media::DISK_CACHE_BYTES } else { u64::from(self.cache_mb) << 20 } diff --git a/crates/engine/src/tests_settings_files.rs b/crates/engine/src/tests_settings_files.rs new file mode 100644 index 000000000..ddc81f20e --- /dev/null +++ b/crates/engine/src/tests_settings_files.rs @@ -0,0 +1,97 @@ +//! Issue #103: a damaged or unreadable settings file (prefs.json, presets.json, view.json) is +//! never silently replaced by the defaults: a damaged one is kept aside, an unreadable one is not +//! written this session, and the user is told. + +use std::sync::{Arc, Mutex}; + +use lightcraft_catalog::{MemStore, Store}; +use serde_json::json; + +use crate::Session; +use crate::library::LibraryStores; + +fn temp_dir(tag: &str) -> std::path::PathBuf { + let d = std::env::temp_dir().join(format!("lc-settings-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d +} + +#[test] +fn damaged_prefs_are_kept_aside_and_reported() { + let lib = temp_dir("damaged"); + let bad = br#"{"exportPresets": [{"name": "Proof", "par"#; + std::fs::write(lib.join("prefs.json"), bad).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + let kept: Vec<_> = std::fs::read_dir(&lib) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().to_string()) + .filter(|n| n.starts_with("prefs.json.corrupt-")) + .collect(); + assert_eq!(kept.len(), 1, "{kept:?}"); + assert_eq!(std::fs::read(lib.join(&kept[0])).unwrap(), bad); + let warnings = s.take_library_warnings(); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("prefs.json is damaged") && warnings[0].contains(&kept[0]), "{warnings:?}"); + assert!(s.take_library_warnings().is_empty(), "each warning is handed out once"); + let info = s.execute("library.info", &json!({})).unwrap(); + assert_eq!(info["settingsWarnings"].as_array().map(Vec::len), Some(1), "{info}"); + // saving goes on (the damaged original is safe in its copy) + s.execute("library.preferences", &json!({"cacheMb": 300})).unwrap(); + assert!(String::from_utf8(std::fs::read(lib.join("prefs.json")).unwrap()).unwrap().contains("300")); + assert_eq!(std::fs::read(lib.join(&kept[0])).unwrap(), bad); + drop(s); + let _ = std::fs::remove_dir_all(&lib); +} + +/// `presets.json` and `prefs.json` can't be read (locked by another program, an I/O error). +#[derive(Clone, Default)] +struct Unreadable { + files: MemStore, + unreadable: Arc>>, +} + +impl Store for Unreadable { + fn read(&mut self, name: &str) -> std::io::Result>> { + if self.unreadable.lock().unwrap().iter().any(|n| n == name) { + return Err(std::io::Error::other("locked by another process")); + } + self.files.read(name) + } + fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.files.write_atomic(name, data) + } + fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.files.append(name, data) + } + fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> { + self.files.truncate(name, len) + } + fn describe(&self) -> String { + "unreadable".into() + } +} + +#[test] +fn unreadable_settings_are_not_overwritten() { + let files = Unreadable::default(); + let presets = br#"{"user": [{"id": "user.mine", "name": "Mine", "group": "User Presets", "settings": {}}]}"#.to_vec(); + let prefs = br#"{"cacheMb": 777}"#.to_vec(); + files.files.set("presets.json", presets.clone()); + files.files.set("prefs.json", prefs.clone()); + *files.unreadable.lock().unwrap() = vec!["presets.json".into(), "prefs.json".into()]; + let mut s = Session::new(); + let stores = LibraryStores { dir: "unreadable".into(), catalog: Box::new(MemStore::new()), files: Box::new(files.clone()), on_disk: false }; + s.open_library_in(stores, true).unwrap(); + let warnings = s.take_library_warnings(); + assert_eq!(warnings.len(), 2, "{warnings:?}"); + assert!(warnings.iter().all(|w| w.contains("couldn't be read") && w.contains("won't overwrite")), "{warnings:?}"); + + // a preset change and a preference change: neither file is replaced by defaults + s.execute("preset.create", &json!({"name": "New"})).unwrap(); + let e = s.execute("library.preferences", &json!({"cacheMb": 300})); + assert!(e.is_err(), "saving preferences is refused, not done over the unread file"); + assert_eq!(files.files.get("presets.json").unwrap(), presets); + assert_eq!(files.files.get("prefs.json").unwrap(), prefs); +} diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index b130c436b..feaa847c4 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -82,6 +82,8 @@ pub fn inspect(app: &LightcraftApp, ctx: &egui::Context) -> Value { "loupe": app.loupe_shown.map(|(p, src)| json!({"photo": p.0, "source": src, "pending": app.renderer.is_pending(crate::render::Slot::Main)})), "hoverPreview": app.hover_preview.as_ref().map(|h| h.label.clone()), "status": app.ui.status, + "notices": app.notices, + "quitPrompt": app.quit_prompt, "unsaved": app.session.unsaved().map(|(n, e)| json!({"ops": n, "error": e})), "libraryProblem": app.library_problem.as_ref().map(crate::panels::library_problem::LibraryProblem::to_json), "scan": app.scan.as_ref().map(crate::import::ScanTask::status), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6700fbde2..b6ea924c4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -35,6 +35,8 @@ mod tests_masking; #[cfg(test)] mod tests_panels; #[cfg(test)] +mod tests_quit_unsaved; +#[cfg(test)] mod tests_scroll; #[cfg(test)] mod tests_unsaved; @@ -125,6 +127,12 @@ pub struct LightcraftApp { pub native_shortcuts: std::collections::HashSet, /// The host is [`headless::Headless`] (it answers viewport screenshot commands itself). pub headless_host: bool, + /// Warnings to show one at a time (damaged settings files…, issue #103). + pub notices: Vec, + /// Quitting was stopped because changes couldn't be saved: the prompt's text. + pub quit_prompt: Option, + /// Quit Anyway was chosen: the window may close with unsaved changes. + pub quit_confirmed: bool, control_rx: Option>, pending_screenshots: Vec, screenshot_token: u64, @@ -192,6 +200,9 @@ impl LightcraftApp { native_menu: false, native_shortcuts: Default::default(), headless_host: false, + notices: vec![], + quit_prompt: None, + quit_confirmed: false, control_rx: None, pending_screenshots: vec![], screenshot_token: 0, @@ -462,6 +473,11 @@ impl LightcraftApp { } else { self.fonts_ready = true; } + panels::notices::logic(self); + // closing the window (or Quit) with changes only in memory: retry, else ask first + if ctx.input(|i| i.viewport().close_requested()) && !panels::notices::may_close(self) { + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + } let now = ctx.input(|i| i.time); let dt = now - self.last_time; if dt > 0.0 { @@ -608,6 +624,7 @@ impl LightcraftApp { // full-screen preview: the photo alone on black egui::CentralPanel::default().frame(egui::Frame::NONE.fill(egui::Color32::BLACK)).show(ui, |ui| panels::detail::show(self, ui)); panels::second::show(self, &ctx); + panels::notices::show(self, &ctx); panels::dialogs::show(self, &ctx); panels::library_problem::show(self, &ctx); panels::toast(self, &ctx); @@ -645,6 +662,7 @@ impl LightcraftApp { state::ViewMode::Reference => panels::compare::show_reference(self, ui), }); panels::second::show(self, &ctx); + panels::notices::show(self, &ctx); panels::dialogs::show(self, &ctx); panels::library_problem::show(self, &ctx); import::progress(self, &ctx); diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index 8634ee341..4891ed8b7 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -12,6 +12,7 @@ pub mod grid; pub mod left; pub mod library_problem; pub mod masking; +pub mod notices; pub mod presets; pub mod profiles; pub mod right; diff --git a/crates/ui-egui/src/panels/notices.rs b/crates/ui-egui/src/panels/notices.rs new file mode 100644 index 000000000..ce051fb52 --- /dev/null +++ b/crates/ui-egui/src/panels/notices.rs @@ -0,0 +1,115 @@ +//! Things the user must not miss (issue #103): settings files that were damaged or unreadable when +//! the library opened (and host warnings such as a damaged `ui.json`), and quitting while changes +//! are only in memory. + +use egui::{Align2, RichText, vec2}; + +use crate::LightcraftApp; +use crate::theme::Tokens; +use crate::widgets::register; + +/// Collect the library's settings-file warnings (each is shown once). +pub fn logic(app: &mut LightcraftApp) { + let new = app.session.take_library_warnings(); + app.notices.extend(new); +} + +/// The window may close now (`true`), or the quit prompt is shown (`false`): changes that +/// couldn't be written are retried first — the log, then a snapshot of everything in memory. +pub fn may_close(app: &mut LightcraftApp) -> bool { + if app.quit_confirmed || app.session.unsaved().is_none() { + return true; + } + let _ = app.session.persist(); + if app.session.unsaved().is_some() + && let Err(e) = app.session.close_library() + { + log::error!("quit: library not saved: {e}"); + } + match app.session.unsaved() { + None => true, + Some((n, e)) => { + app.quit_prompt = Some(format!( + "{} couldn't be written to disk: {e}\nIf you quit now, {} lost.", + if n == 1 { "1 change".to_string() } else { format!("{n} changes") }, + if n == 1 { "it is" } else { "they are" } + )); + false + } + } +} + +enum Choice { + Retry, + QuitAnyway, + Cancel, + Ok, +} + +fn window(ctx: &egui::Context, id: &str, title: &str, text: &str, buttons: &[(&str, &str, Choice)]) -> Option { + let t = Tokens::get(ctx); + let screen = ctx.content_rect(); + egui::Area::new(egui::Id::new((id, "dim"))).order(egui::Order::Middle).fixed_pos(screen.min).show(ctx, |ui| { + ui.allocate_rect(screen, egui::Sense::click_and_drag()); + ui.painter().rect_filled(screen, 0.0, egui::Color32::from_black_alpha(140)); + }); + let mut chosen = None; + let frame = egui::Frame::window(&ctx.global_style()).inner_margin(egui::Margin::symmetric(18, 14)); + egui::Window::new(title) + .id(egui::Id::new(id)) + .order(egui::Order::Foreground) + .collapsible(false) + .resizable(false) + .frame(frame) + .anchor(Align2::CENTER_CENTER, [0.0, 0.0]) + .default_width(440.0) + .show(ctx, |ui| { + // a fixed width: a wrapped label in an auto-sized, centred window would move it every frame + ui.set_width(420.0); + ui.spacing_mut().item_spacing.y = 8.0; + ui.add(egui::Label::new(RichText::new(text).color(t.text_label)).wrap()); + ui.add_space(4.0); + ui.horizontal(|ui| { + for (i, (bid, label, _)) in buttons.iter().enumerate() { + let r = ui.add(egui::Button::new(*label).min_size(vec2(0.0, 26.0))); + register(ui.ctx(), format!("button:{bid}"), r.rect); + if r.clicked() { + chosen = Some(i); + } + } + }); + }); + chosen +} + +pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { + if let Some(text) = app.quit_prompt.clone() { + let buttons = [ + ("quitRetry", "Try Saving Again", Choice::Retry), + ("quitAnyway", "Quit Anyway", Choice::QuitAnyway), + ("quitCancel", "Cancel", Choice::Cancel), + ]; + let Some(i) = window(ctx, "quit-unsaved", "Quit with unsaved changes?", &text, &buttons) else { return }; + match buttons[i].2 { + Choice::Retry => { + app.quit_prompt = None; + if may_close(app) { + app.ui.quit = true; + } + } + Choice::QuitAnyway => { + app.quit_prompt = None; + app.quit_confirmed = true; + app.ui.quit = true; + } + Choice::Cancel | Choice::Ok => app.quit_prompt = None, + } + return; + } + if let Some(text) = app.notices.first().cloned() { + let buttons = [("noticeOk", "OK", Choice::Ok)]; + if window(ctx, "settings-notice", "Settings file problem", &text, &buttons).is_some() { + app.notices.remove(0); + } + } +} diff --git a/crates/ui-egui/src/tests_quit_unsaved.rs b/crates/ui-egui/src/tests_quit_unsaved.rs new file mode 100644 index 000000000..a3da72be4 --- /dev/null +++ b/crates/ui-egui/src/tests_quit_unsaved.rs @@ -0,0 +1,128 @@ +//! Issue #103: quitting with changes that couldn't be saved asks first (after one more try), and +//! settings-file problems found when the library opened are shown, not swallowed. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; + +use lightcraft_catalog::{MemStore, Store}; +use lightcraft_engine::library::LibraryStores; +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +/// Every write fails while `fail` is set (an unplugged drive). +#[derive(Clone, Default)] +struct Unplugged { + files: MemStore, + fail: Arc, +} + +impl Unplugged { + fn check(&self) -> std::io::Result<()> { + if self.fail.load(Ordering::SeqCst) { Err(std::io::Error::other("drive unplugged")) } else { Ok(()) } + } +} + +impl Store for Unplugged { + fn read(&mut self, name: &str) -> std::io::Result>> { + self.files.read(name) + } + fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.check()?; + self.files.write_atomic(name, data) + } + fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.check()?; + self.files.append(name, data) + } + fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> { + self.files.truncate(name, len) + } + fn describe(&self) -> String { + "unplugged".into() + } +} + +fn has(h: &Headless, id: &str) -> bool { + h.app.widgets.iter().any(|(w, _)| w == id) +} + +/// Click a widget and let the synthetic events (move, press, release) play out. +fn click(h: &mut Headless, id: &str) { + // a window that just appeared is placed on its second frame + h.step(); + h.step(); + assert_eq!(h.request("ui.clickWidget", json!({"id": id}), T)["ok"], true, "{id}"); + for _ in 0..4 { + h.step(); + } +} + +#[test] +fn quitting_with_unsaved_changes_asks_first() { + let store = Unplugged::default(); + let mut s = lightcraft_engine::Session::new(); + let stores = LibraryStores { dir: "unplugged".into(), catalog: Box::new(store.clone()), files: Box::new(MemStore::new()), on_disk: false }; + s.open_library_in(stores, false).unwrap(); + let app = LightcraftApp::new(s, Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + h.step(); + // nothing unsaved: closing is fine + assert!(crate::panels::notices::may_close(&mut h.app)); + + store.fail.store(true, Ordering::SeqCst); + let r = h.request("engine.execute", json!({"command": "album.create", "params": {"name": "Kept"}}), T); + assert_eq!(r["ok"], false, "{r}"); + assert!(!crate::panels::notices::may_close(&mut h.app), "retried, still failing: ask"); + h.step(); + let prompt = h.app.quit_prompt.clone().unwrap(); + assert!(prompt.contains("1 change couldn't be written to disk") && prompt.contains("drive unplugged"), "{prompt}"); + for b in ["button:quitRetry", "button:quitAnyway", "button:quitCancel"] { + assert!(has(&h, b), "{b}"); + } + assert_eq!(h.request("ui.inspect", json!({}), T)["result"]["quitPrompt"], json!(prompt)); + // Cancel: back to work, nothing quit + click(&mut h, "button:quitCancel"); + assert!(h.app.quit_prompt.is_none() && !h.app.quit_confirmed); + + // the drive is back: Try Saving Again saves and quits + assert!(!crate::panels::notices::may_close(&mut h.app)); + h.step(); + store.fail.store(false, Ordering::SeqCst); + click(&mut h, "button:quitRetry"); + assert!(h.app.session.unsaved().is_none(), "saved"); + assert!(h.app.quit_prompt.is_none() && !h.app.quit_confirmed); + + // Quit Anyway is a deliberate choice + store.fail.store(true, Ordering::SeqCst); + let _ = h.request("engine.execute", json!({"command": "album.create", "params": {"name": "Lost"}}), T); + assert!(!crate::panels::notices::may_close(&mut h.app)); + h.step(); + click(&mut h, "button:quitAnyway"); + assert!(h.app.quit_confirmed && crate::panels::notices::may_close(&mut h.app)); +} + +#[test] +fn damaged_settings_file_is_shown() { + let files = MemStore::new(); + files.set("presets.json", b"{\"user\": [ {\"id\": ".to_vec()); + let mut s = lightcraft_engine::Session::new(); + let stores = LibraryStores { dir: "lib".into(), catalog: Box::new(MemStore::new()), files: Box::new(files.clone()), on_disk: false }; + s.open_library_in(stores, false).unwrap(); + let app = LightcraftApp::new(s, Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + h.step(); + h.step(); + assert_eq!(h.app.notices.len(), 1); + assert!(h.app.notices[0].contains("presets.json is damaged"), "{:?}", h.app.notices); + assert!(has(&h, "button:noticeOk")); + assert!(files.files.lock().unwrap().keys().any(|k| k.starts_with("presets.json.corrupt-"))); + click(&mut h, "button:noticeOk"); + assert!(h.app.notices.is_empty(), "{:?}", h.app.notices); + h.step(); + assert!(!has(&h, "button:noticeOk")); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index e84314e99..ed4b77d3e 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -21,7 +21,7 @@ mode ([mcp.md](mcp.md)) is a thin layer over this channel. Implementation: | `engine.commands` | — | Engine + UI commands: id, label, menu, shortcut, params doc, enabled | | `ui.menu.list` | — | Menu entries (flat: id, label, menu path, shortcut, enabled) | | `ui.menu.tree` | — | The menu bar as shown (File … Help): items `{id, params?, label, shortcut?, enabled, checked?}`, separators, submenus — the model behind the native macOS menu bar and the in-window menus | -| `ui.inspect` | — | UI state, window, canvas/image rects, `scroll: {grid, filmstrip}` (scroll offsets in points, `null` until drawn), active photo, selection, perf (`frameMs` = layout, `logicMs` = per-frame logic before it, `updateMs` = both, `maxUpdateMs`, `fps`, render queue …, `gpu` = adapter in use, `gpuReason` = why renders don't use the GPU, `gpuFallback` = latest render redone on the CPU and why — see `docs/gpu-pipeline.md`), status, memory (bytes per cache, see `library.memory`; plus stage caches and textures), `export: {running: {total, done, current} \| null, last}` | +| `ui.inspect` | — | UI state, window, canvas/image rects, `scroll: {grid, filmstrip}` (scroll offsets in points, `null` until drawn), active photo, selection, perf (`frameMs` = layout, `logicMs` = per-frame logic before it, `updateMs` = both, `maxUpdateMs`, `fps`, render queue …, `gpu` = adapter in use, `gpuReason` = why renders don't use the GPU, `gpuFallback` = latest render redone on the CPU and why — see `docs/gpu-pipeline.md`), status, memory (bytes per cache, see `library.memory`; plus stage caches and textures), `export: {running: {total, done, current} \| null, last}`, `notices` (warnings waiting to be shown, e.g. a damaged settings file; OK = `button:noticeOk`), `quitPrompt` (why quitting was stopped: unsaved changes; `button:quitRetry` / `button:quitAnyway` / `button:quitCancel`) | | `ui.widgets` | `{filter?}` | On-screen widgets `{id, rect: [x, y, w, h]}` (screen points) | | `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, fx?, fy?}` / `{id, toX?, toY?, dx?, dy?, steps?}` / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews) | | `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points | diff --git a/docs/parity.md b/docs/parity.md index 495e062df..231b24002 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -463,7 +463,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-PREF-GENERAL | General settings | P0 | ✅ | `cmd:app.settings`, `cmd:app.openLibrary`, `crates/ui-egui/src/panels/settings.rs` | General / Import / Performance / Interface tabs; app settings in ui.json, library settings in prefs.json | +| LR-PREF-GENERAL | General settings | P0 | ✅ | `cmd:app.settings`, `cmd:app.openLibrary`, `crates/ui-egui/src/panels/settings.rs`, `crates/ui-egui/src/panels/notices.rs` | General / Import / Performance / Interface tabs; app settings in ui.json (written atomically, saved when the library changes and every few seconds), library settings in prefs.json; a damaged settings file is kept as `.corrupt-