diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 97354c58f..7d30e69cd 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -980,9 +980,9 @@ fn main() -> eframe::Result { if let Some(ui) = prefs { app.ui = ui; } - // the recent libraries to choose from (issue #1290), each checked now + // the recent libraries to choose from (issue #1290), checked on a worker once shown if launch == Launch::Ask { - problem = Some(LibraryProblem::choose(&app.ui.settings)); + problem = Some(LibraryProblem::choose_later(&app.ui.settings)); } app.alt_asks_for_library = matches!(launch, Launch::Open { alt_asks: true, .. }); lightcraft_ui_egui::i18n::set_language(app.ui.language); @@ -1177,7 +1177,7 @@ mod tests { let path = d.join("ui.json"); let mut app = LightcraftApp::new(s, Services::default()); app.ui.settings.recent_libraries = vec!["/gone/a".into(), "/gone/b".into()]; - app.library_problem = Some(LibraryProblem::choose(&app.ui.settings)); + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); let mut w = PrefsWriter { path: Some(path.clone()), ..Default::default() }; app.ui.settings.recent_libraries.remove(0); w.save(&app).unwrap(); diff --git a/crates/ui-egui/locales/fr.json b/crates/ui-egui/locales/fr.json index 76f6e12cd..bd656a9e4 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -2358,5 +2358,8 @@ "There is no LightCraft library in this folder any more": "Ce dossier ne contient plus de bibliothèque LightCraft", "This is a file, not a library folder": "C’est un fichier, pas un dossier de bibliothèque", "This folder can't be read:": "Ce dossier ne peut pas être lu :", - "The library's catalog can't be read:": "Le catalogue de la bibliothèque ne peut pas être lu :" + "The library's catalog can't be read:": "Le catalogue de la bibliothèque ne peut pas être lu :", + "Not answering yet: the drive may be asleep or offline": "Pas encore de réponse : le disque est peut-être en veille ou hors ligne", + "This library couldn't be checked": "Impossible de vérifier cette bibliothèque", + "Checking recent libraries": "Vérification des bibliothèques récentes" } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3528babe5..2c7b185bf 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -488,6 +488,9 @@ pub struct LightcraftApp { /// Alt/Option held as the window first appears offers the recent libraries once (issue /// #1290); the desktop host arms it when nothing chose the library already. pub alt_asks_for_library: bool, + /// Whether each recent library can be opened, answered by a worker (issue #1290): the launch + /// list and File ▸ Open Recent Library never look at a folder on the UI thread. + pub recent_checks: panels::library_problem::RecentChecks, /// The activity stack shows every task, not just the first few ("+N more" was clicked). pub activity_expanded: bool, } @@ -568,6 +571,7 @@ impl LightcraftApp { screen_profile_read: None, library_problem: None, alt_asks_for_library: false, + recent_checks: Default::default(), activity_expanded: false, model_setup: Default::default(), } @@ -1431,6 +1435,8 @@ impl LightcraftApp { if let Some(l) = self.session.library.as_ref().filter(|l| l.on_disk) { let path = l.dir.to_string_lossy().into_owned(); panels::settings::remember_library(&mut self.ui.settings.recent_libraries, &path); + // it is open: whatever an earlier check said about it is over + self.recent_checks.opened(&path); // when, for the launch window (issue #1290); only the listed libraries keep a time let settings = &mut self.ui.settings; settings.recent_opened.insert(path, (self.session.clock)()); diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 2dde300a8..86f05a99f 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -641,8 +641,10 @@ pub(crate) fn expanded(app: &LightcraftApp, name: &str) -> Option> .iter() .map(|r| { let d = std::path::Path::new(r); - // a missing library (moved, drive disconnected) is shown greyed out - item("app.openLibrary", json!({"path": r}), label(r), None, can && d.is_dir(), Some(open.as_deref() == Some(d))) + // a library a check found missing (moved, drive disconnected) is greyed out; the + // check runs on a worker (`RecentChecks`), never here: the menu is rebuilt every frame + let gone = app.recent_checks.reason(r).is_some(); + item("app.openLibrary", json!({"path": r}), label(r), None, can && !gone, Some(open.as_deref() == Some(d))) }) .collect(); if v.is_empty() { diff --git a/crates/ui-egui/src/panels/library_problem.rs b/crates/ui-egui/src/panels/library_problem.rs index f57393f9a..9b77b6c58 100644 --- a/crates/ui-egui/src/panels/library_problem.rs +++ b/crates/ui-egui/src/panels/library_problem.rs @@ -7,6 +7,12 @@ //! is *Ask me*, or when Alt/Option is held as the window first appears (issue #1290): Open, Open //! Another…, New Library…, Remove from List and Quit, with the arrow keys, Return and Esc. //! [`launch_plan`] decides at launch which of the two the desktop app starts with. +//! +//! Whether each recent library can be opened is checked on a worker ([`RecentChecks`]), never on +//! the UI thread: a library on a network share whose server is gone, or on a disk that has to +//! spin up, can take seconds to answer. Until its answer comes the list shows the row as +//! "Checking…" (it can still be opened: opening says why if it can't), and File ▸ Open Recent +//! Library keeps it enabled; the same answers grey out the menu's entries. use std::path::{Path, PathBuf}; @@ -56,9 +62,13 @@ pub struct RecentLibrary { pub path: String, /// When it was last opened (ISO 8601), if known. pub opened: Option, - /// Why it can't be opened (`None`: it can), as checked when the window opened or when - /// opening it failed. + /// Why it can't be opened (`None`: it can, or it is still being checked), as checked when + /// the window opened or when opening it failed. pub unavailable: Option, + /// Its check hasn't answered yet ([`RecentChecks`]). + pub checking: bool, + /// Its check has gone on for longer than [`SLOW_AFTER`] (the drive may be asleep or offline). + pub slow: bool, } /// The launch window's list of recent libraries (issue #1290). @@ -68,6 +78,10 @@ pub struct Chooser { pub libraries: Vec, /// The highlighted row (`None`: the list is empty). pub selected: Option, + /// The person moved the highlight: answers arriving later don't move it again. + pub touched: bool, + /// Its rows' checks were started ([`logic`]). + pub asked: bool, /// The library open behind the window (Alt/Option held at launch), by /// [`lightcraft_engine::Session::library_generation`]: Esc keeps it. `None`: no library is open. pub over: Option, @@ -102,21 +116,40 @@ impl LibraryProblem { /// ([`unavailable`]): one that is gone, isn't a LightCraft library any more or can't be read /// is greyed out with the reason. The last library opened is selected, or else the first one /// that can be opened. - pub fn choose(settings: &AppSettings) -> LibraryProblem { + #[cfg(test)] + pub(crate) fn choose(settings: &AppSettings) -> LibraryProblem { + let mut p = LibraryProblem::choose_later(settings); + if let Some(c) = p.chooser.as_mut() { + for l in &mut c.libraries { + l.unavailable = unavailable(&l.path).map(|u| u.text()); + l.checking = false; + } + c.selected = initial_selection(&c.libraries, &settings.library_path); + c.asked = true; + } + p + } + + /// [`LibraryProblem::choose`] without looking at any folder: every row is "Checking…" until + /// [`logic`] has the answers from a worker ([`RecentChecks`]). The last library opened is + /// selected meanwhile (or the first row); once every answer is in, the selection follows the + /// same rule as [`LibraryProblem::choose`] unless the person moved it. + pub fn choose_later(settings: &AppSettings) -> LibraryProblem { // as many as File ▸ Open Recent Library keeps (a hand-edited settings file may hold more) let libraries: Vec = settings .recent_libraries .iter() .take(crate::state::RECENT_LIBRARIES) - .map(|path| RecentLibrary { path: path.clone(), opened: settings.recent_opened.get(path).cloned(), unavailable: unavailable(path) }) + .map(|path| RecentLibrary { + path: path.clone(), + opened: settings.recent_opened.get(path).cloned(), + unavailable: None, + checking: true, + slow: false, + }) .collect(); - let can = |l: &RecentLibrary| l.unavailable.is_none(); - let selected = libraries - .iter() - .position(|l| l.path == settings.library_path && can(l)) - .or_else(|| libraries.iter().position(can)) - .or((!libraries.is_empty()).then_some(0)); - LibraryProblem { chooser: Some(Chooser { libraries, selected, over: None }), ..Default::default() } + let selected = libraries.iter().position(|l| l.path == settings.library_path).or((!libraries.is_empty()).then_some(0)); + LibraryProblem { chooser: Some(Chooser { libraries, selected, over: None, touched: false, asked: false }), ..Default::default() } } /// The session promises to save nothing (the failed-library window or Continue Without @@ -131,7 +164,7 @@ impl LibraryProblem { let libraries: Vec = c .libraries .iter() - .map(|l| json!({"path": l.path, "lastOpened": l.opened, "available": l.unavailable.is_none(), "reason": l.unavailable})) + .map(|l| json!({"path": l.path, "lastOpened": l.opened, "available": l.unavailable.is_none(), "checking": l.checking, "reason": l.unavailable})) .collect(); json!({"libraries": libraries, "selected": c.selected, "libraryOpen": c.over.is_some()}) }); @@ -139,32 +172,220 @@ impl LibraryProblem { } } +/// The row first selected: the last library opened if it can be opened, else the first one that +/// can, else the first row. +fn initial_selection(libraries: &[RecentLibrary], last: &str) -> Option { + let can = |l: &RecentLibrary| l.unavailable.is_none(); + libraries.iter().position(|l| l.path == last && can(l)).or_else(|| libraries.iter().position(can)).or((!libraries.is_empty()).then_some(0)) +} + +/// Why a recent library can't be opened, found on a worker and put into words on the UI thread +/// (the interface language is per thread). +#[derive(Clone, Debug, PartialEq)] +pub enum Unavailable { + /// Nothing at that path (moved, renamed, or on a disconnected drive). + NotFound, + /// A file, not a folder. + NotAFolder, + /// The folder can't be listed (the system's reason). + Unreadable(String), + /// The folder holds no LightCraft library any more. + NoLibrary, + /// The library's catalog file can't be opened (the system's reason). + CatalogUnreadable(String), + /// The check itself failed (its worker stopped, or couldn't start). + CheckFailed(String), +} + +impl Unavailable { + /// In the interface language. + pub fn text(&self) -> String { + match self { + Unavailable::NotFound => crate::i18n::tr("Not found: moved, renamed or on a disconnected drive").to_string(), + Unavailable::NotAFolder => crate::i18n::tr("This is a file, not a library folder").to_string(), + Unavailable::Unreadable(e) => format!("{} {e}", crate::i18n::tr("This folder can't be read:")), + Unavailable::NoLibrary => crate::i18n::tr("There is no LightCraft library in this folder any more").to_string(), + Unavailable::CatalogUnreadable(e) => format!("{} {e}", crate::i18n::tr("The library's catalog can't be read:")), + Unavailable::CheckFailed(e) => e.clone(), + } + } +} + +/// How long an answer stands before File ▸ Open Recent Library asks again (a drive plugged back +/// in, a library moved away meanwhile). +pub const RECHECK_AFTER: std::time::Duration = std::time::Duration::from_secs(30); +/// How long a row stays "Checking…" before it says the drive isn't answering (it can still be +/// opened: opening says why if it can't). +pub const SLOW_AFTER: std::time::Duration = std::time::Duration::from_secs(5); + +/// The activity stack's row for a check (English: the stack translates it). +const CHECK_LABEL: &str = "Checking recent libraries"; + +/// One path's answer and when it came. +#[derive(Clone, Debug)] +struct Answer { + why: Option, + at: web_time::Instant, +} + +/// Answers to "can this recent library be opened?", each found on a worker thread of its own (a +/// library on a network share whose server is gone, or on a sleeping disk, can take seconds or +/// never answer; the others don't wait for it). +pub struct RecentChecks { + known: std::collections::HashMap, + /// Paths being checked, and since when. + pending: std::collections::HashMap, + /// How one path is checked ([`unavailable`]; tests put a slow one here). + check: std::sync::Arc Option + Send + Sync>, +} + +impl Default for RecentChecks { + /// Nothing known yet, checking with [`unavailable`]. + fn default() -> Self { + RecentChecks { known: Default::default(), pending: Default::default(), check: std::sync::Arc::new(unavailable) } + } +} + +impl RecentChecks { + /// Why `path` can't be opened, if a check said so (`None`: it can, or it isn't known yet). + pub fn reason(&self, path: &str) -> Option<&Unavailable> { + self.known.get(path).and_then(|a| a.why.as_ref()) + } + + /// Whether a check of `path` has answered. + pub fn is_known(&self, path: &str) -> bool { + self.known.contains_key(path) + } + + /// Whether `path` is being checked. + pub fn is_checking(&self, path: &str) -> bool { + self.pending.contains_key(path) + } + + /// Since when `path` has been checked, if it is. + fn checking_since(&self, path: &str) -> Option { + self.pending.get(path).copied() + } + + /// A check is running. + pub fn is_running(&self) -> bool { + !self.pending.is_empty() + } + + /// Check paths with another function (tests). + pub fn with_check(check: impl Fn(&str) -> Option + Send + Sync + 'static) -> RecentChecks { + RecentChecks { check: std::sync::Arc::new(check), ..Default::default() } + } + + /// `path` was just opened (or made): it can be opened. + pub fn opened(&mut self, path: &str) { + self.known.insert(path.to_string(), Answer { why: None, at: web_time::Instant::now() }); + } + + /// Opening `path` failed: what the open said is its answer until the next check. + pub fn failed(&mut self, path: &str, error: &str) { + self.known.insert(path.to_string(), Answer { why: Some(Unavailable::CheckFailed(error.to_string())), at: web_time::Instant::now() }); + } + + /// `path` left the list: forget its answer. + pub fn forget(&mut self, path: &str) { + self.known.remove(path); + } +} + +/// Check each of `paths` on a worker of its own (one that is being checked already is left to +/// finish). The activity stack lists each check while it runs (a quick one doesn't show: rows +/// appear after half a second); none can be stopped, as a stat that hangs can't be. +fn start_checks(app: &mut LightcraftApp, paths: Vec) { + for path in paths { + if app.recent_checks.pending.contains_key(&path) { + continue; + } + let check = app.recent_checks.check.clone(); + let p = path.clone(); + let started = crate::tasks::spawn(app, CHECK_LABEL, Some("library"), move || check(&p), { + let path = path.clone(); + move |app, ctx, why| { + app.recent_checks.pending.remove(&path); + app.recent_checks.known.insert(path, Answer { why, at: web_time::Instant::now() }); + apply_answers(app); + ctx.request_repaint(); + } + }); + match started { + Ok(()) => { + app.recent_checks.pending.insert(path, web_time::Instant::now()); + } + // no worker thread: say so on the row, and don't try again every frame + Err(e) => { + log::warn!("{e}"); + app.recent_checks.known.insert(path, Answer { why: Some(Unavailable::CheckFailed(e)), at: web_time::Instant::now() }); + apply_answers(app); + } + } + } +} + +/// Checks whose worker stopped without answering (it panicked: `tasks::poll` already said so) +/// end as failed, so the row and the menu don't wait for them forever. +fn reap_checks(app: &mut LightcraftApp) { + // (every check has the same label: when none is listed any more, the ones still pending died) + if app.recent_checks.pending.is_empty() || app.tasks.is_running(CHECK_LABEL) { + return; + } + let dead: Vec = app.recent_checks.pending.keys().cloned().collect(); + for path in dead { + app.recent_checks.pending.remove(&path); + let why = Unavailable::CheckFailed(crate::i18n::tr("This library couldn't be checked").to_string()); + app.recent_checks.known.insert(path, Answer { why: Some(why), at: web_time::Instant::now() }); + } + apply_answers(app); +} + +/// Put the answers in into the launch list; once none is missing, select as +/// [`LibraryProblem::choose`] does unless the person moved the highlight. +fn apply_answers(app: &mut LightcraftApp) { + let last = app.ui.settings.library_path.clone(); + let checks = &app.recent_checks; + let Some(c) = app.library_problem.as_mut().and_then(|p| p.chooser.as_mut()) else { return }; + let checking_before = c.libraries.iter().any(|l| l.checking); + for l in &mut c.libraries { + if l.checking && !checks.is_checking(&l.path) && checks.is_known(&l.path) { + l.unavailable = checks.reason(&l.path).map(Unavailable::text); + l.checking = false; + } + } + if checking_before && !c.libraries.iter().any(|l| l.checking) && !c.touched { + c.selected = initial_selection(&c.libraries, &last); + } +} + /// Why the library at `path` can't be opened (`None`: as far as can be told without opening it). /// /// Read-only: the folder and its catalog file are looked at, never locked or written (the list /// may name libraries on other drives, network shares or sleeping disks). A library another /// program has open is therefore found when Open is pressed: the open fails and the window says -/// who has it. -fn unavailable(path: &str) -> Option { +/// who has it. Runs on a worker: the reason is put into words on the UI thread. +fn unavailable(path: &str) -> Option { let dir = Path::new(path); if !dir.exists() { - return Some(crate::i18n::tr("Not found: moved, renamed or on a disconnected drive").to_string()); + return Some(Unavailable::NotFound); } if !dir.is_dir() { - return Some(crate::i18n::tr("This is a file, not a library folder").to_string()); + return Some(Unavailable::NotAFolder); } if let Err(e) = std::fs::read_dir(dir) { - return Some(format!("{} {e}", crate::i18n::tr("This folder can't be read:"))); + return Some(Unavailable::Unreadable(e.to_string())); } // opening would make a new, empty library in it if !lightcraft_engine::library::is_library(dir) { - return Some(crate::i18n::tr("There is no LightCraft library in this folder any more").to_string()); + return Some(Unavailable::NoLibrary); } // the catalog the open reads first: the snapshot, else the log alone (opened to read, not read) let snapshot = dir.join(lightcraft_catalog::journal::SNAPSHOT); let catalog = if snapshot.is_file() { snapshot } else { dir.join(lightcraft_catalog::journal::LOG) }; if let Err(e) = std::fs::File::open(&catalog) { - return Some(format!("{} {e}", crate::i18n::tr("The library's catalog can't be read:"))); + return Some(Unavailable::CatalogUnreadable(e.to_string())); } None } @@ -207,6 +428,9 @@ fn open(app: &mut LightcraftApp, path: Option) { Err(e) => { // a failed switch reopens the library that was open behind the launch list let generation = app.session.library.as_ref().map(|_| app.session.library_generation()); + if let Some(path) = path.as_deref() { + app.recent_checks.failed(path, &e); + } if let Some(p) = app.library_problem.as_mut() { if let Some(c) = p.chooser.as_mut() { if c.over.is_some() { @@ -214,6 +438,7 @@ fn open(app: &mut LightcraftApp, path: Option) { } if let Some(l) = c.libraries.iter_mut().find(|l| path.as_deref() == Some(l.path.as_str())) { l.unavailable = Some(e.clone()); + l.checking = false; } } if let Some(path) = path { @@ -244,6 +469,41 @@ pub fn logic(app: &mut LightcraftApp, ctx: &egui::Context) { } } alt_at_launch(app, ctx); + reap_checks(app); + // the launch list asks for fresh answers once + let listed: Option> = + app.library_problem.as_mut().and_then(|p| p.chooser.as_mut()).and_then(|c| { + (!std::mem::replace(&mut c.asked, true)).then(|| c.libraries.iter().filter(|l| l.checking).map(|l| l.path.clone()).collect()) + }); + if let Some(listed) = listed { + for path in &listed { + if !app.recent_checks.is_checking(path) { + app.recent_checks.forget(path); + } + } + start_checks(app, listed); + apply_answers(app); + } + // File ▸ Open Recent Library: paths never checked, or not for a while + let stale: Vec = app + .ui + .settings + .recent_libraries + .iter() + .filter(|p| !app.recent_checks.is_checking(p) && app.recent_checks.known.get(*p).is_none_or(|a| a.at.elapsed() > RECHECK_AFTER)) + .cloned() + .collect(); + start_checks(app, stale); + if app.recent_checks.is_running() { + // rows still checking say so once it takes long + let checks = &app.recent_checks; + if let Some(c) = app.library_problem.as_mut().and_then(|p| p.chooser.as_mut()) { + for l in c.libraries.iter_mut().filter(|l| l.checking) { + l.slow = checks.checking_since(&l.path).is_some_and(|t| t.elapsed() > SLOW_AFTER); + } + } + ctx.request_repaint_after(std::time::Duration::from_millis(250)); + } chooser_keys(app, ctx); } @@ -266,7 +526,7 @@ fn alt_at_launch(app: &mut LightcraftApp, ctx: &egui::Context) { if crate::panels::settings::can_switch_library(app).is_err() { return; } - let mut p = LibraryProblem::choose(&app.ui.settings); + let mut p = LibraryProblem::choose_later(&app.ui.settings); if let Some(c) = p.chooser.as_mut() { c.over = app.session.library.as_ref().map(|_| app.session.library_generation()); } @@ -314,6 +574,7 @@ fn select(app: &mut LightcraftApp, at: usize) { && at < c.libraries.len() { c.selected = Some(at); + c.touched = true; } } @@ -410,6 +671,7 @@ fn act(app: &mut LightcraftApp, choice: Choice) { Choice::Remove => { let Some((library, _)) = chosen else { return }; crate::panels::settings::forget_library(&mut app.ui.settings, &library.path); + app.recent_checks.forget(&library.path); if let Some(c) = app.library_problem.as_mut().and_then(|p| p.chooser.as_mut()) { c.libraries.retain(|l| l.path != library.path); c.selected = c.selected.map(|s| s.min(c.libraries.len().saturating_sub(1))).filter(|_| !c.libraries.is_empty()); @@ -549,6 +811,10 @@ fn recent_row(ui: &mut egui::Ui, t: &Tokens, i: usize, library: &RecentLibrary, ui.add(egui::Label::new(RichText::new(parent).size(11.0).color(dim)).truncate().selectable(false)); if let Some(why) = &library.unavailable { ui.add(egui::Label::new(RichText::new(why).size(11.0).color(t.caution)).wrap().selectable(false)); + } else if library.checking { + let text = + if library.slow { crate::i18n::tr("Not answering yet: the drive may be asleep or offline") } else { crate::i18n::tr("Checking…") }; + ui.add(egui::Label::new(RichText::new(text).size(11.0).color(dim)).wrap().selectable(false)); } }); let r = ui.interact(inner.response.rect, egui::Id::new(("library-recent", i)), egui::Sense::click()); diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 6b17cec91..86c4fd882 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -1306,8 +1306,11 @@ fn free_name(location: &std::path::Path, base: &str) -> std::path::PathBuf { } /// `app.recentLibraries {remove?: path, clear?: bool}`: the recently opened libraries, newest -/// first, as `[{path, name, exists, open, lastOpened}]` (`exists: false`: moved or on a -/// disconnected drive; `lastOpened`: ISO 8601 time, `null` when not known). `remove` forgets that +/// first, as `[{path, name, exists, checking, open, lastOpened}]`. `exists` is what the last +/// background check found (`false`: moved, on a disconnected drive or no longer a library; +/// `null`: not checked yet, `checking` then says whether a check is running): the command never +/// looks at a folder itself, as one on an offline network share can take seconds to answer. +/// `lastOpened`: ISO 8601 time, `null` when not known. `remove` forgets that /// one entry (an error if the list doesn't hold it) and `clear` forgets them all; neither touches /// anything on disk (issue #1290). pub fn recent_libraries(app: &mut LightcraftApp, p: &Value) -> Result { @@ -1317,21 +1320,25 @@ pub fn recent_libraries(app: &mut LightcraftApp, p: &Value) -> Result return Err("app.recentLibraries: remove must be a library path (a string)".into()), } if p.get("clear").and_then(Value::as_bool) == Some(true) { - app.ui.settings.recent_libraries.clear(); + for path in std::mem::take(&mut app.ui.settings.recent_libraries) { + app.recent_checks.forget(&path); + } app.ui.settings.recent_opened.clear(); } let open = app.session.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.clone()); - let settings = &app.ui.settings; + let (settings, checks) = (&app.ui.settings, &app.recent_checks); let list: Vec = settings .recent_libraries .iter() .map(|r| { let d = std::path::Path::new(r); - json!({"path": r, "name": library_name(d), "exists": d.is_dir(), "open": open.as_deref() == Some(d), "lastOpened": settings.recent_opened.get(r)}) + let exists = checks.is_known(r).then(|| checks.reason(r).is_none()); + json!({"path": r, "name": library_name(d), "exists": exists, "checking": checks.is_checking(r), "open": open.as_deref() == Some(d), "lastOpened": settings.recent_opened.get(r)}) }) .collect(); Ok(Value::Array(list)) diff --git a/crates/ui-egui/src/tests_launch_library.rs b/crates/ui-egui/src/tests_launch_library.rs index 699e19a8b..cb50c6a32 100644 --- a/crates/ui-egui/src/tests_launch_library.rs +++ b/crates/ui-egui/src/tests_launch_library.rs @@ -417,7 +417,8 @@ fn open_recent_library_and_the_command_remove_one_entry() { // the command: the open library can be forgotten too (it stays open), and nothing is deleted let list = app.run("app.recentLibraries", json!({"remove": first})).unwrap(); - assert_eq!(list, json!([{"path": "/elsewhere/third", "name": "third", "exists": false, "open": false, "lastOpened": null}])); + // (no frame ran: nothing checked yet, so `exists` is not known; the command never looks itself) + assert_eq!(list, json!([{"path": "/elsewhere/third", "name": "third", "exists": null, "checking": false, "open": false, "lastOpened": null}])); assert!(app.session.library.is_some() && lightcraft_engine::library::is_library(&dir.join("first"))); let e = app.run("app.recentLibraries", json!({"remove": first})).unwrap_err(); assert!(e.contains("not in the recent libraries"), "{e}"); @@ -574,3 +575,213 @@ fn alt_is_only_watched_while_the_window_first_appears_and_when_armed() { assert!(app.library_problem.as_ref().is_some_and(|p| p.chooser.is_some())); assert!(!app.alt_asks_for_library); } + +/// A check that waits until the test lets it go (a network share whose server is gone), then +/// answers like the real one for a missing folder. `release` lets every waiting check go. +fn slow_checks() -> ( + crate::panels::library_problem::RecentChecks, + std::sync::Arc<(std::sync::Mutex, std::sync::Condvar)>, + std::sync::Arc, +) { + let gate = std::sync::Arc::new((std::sync::Mutex::new(false), std::sync::Condvar::new())); + let calls = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let (g, n) = (gate.clone(), calls.clone()); + let checks = crate::panels::library_problem::RecentChecks::with_check(move |p| { + n.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + let (lock, cv) = &*g; + let open = lock.lock().unwrap(); + // (bounded: a test that forgets to release still ends) + let _unused = cv.wait_timeout_while(open, Duration::from_secs(10), |open| !*open).unwrap(); + (!Path::new(p).exists()).then_some(crate::panels::library_problem::Unavailable::NotFound) + }); + (checks, gate, calls) +} + +/// Let every check waiting in [`slow_checks`] answer. +fn release(gate: &std::sync::Arc<(std::sync::Mutex, std::sync::Condvar)>) { + let (lock, cv) = &**gate; + *lock.lock().unwrap() = true; + cv.notify_all(); +} + +/// Review of #2701: the launch list never waits on a folder. Its rows show "Checking…" while a +/// worker looks (the activity stack lists it), the window keeps drawing frames meanwhile, and the +/// answers grey out a missing library and move the selection to one that can be opened. +#[test] +fn a_slow_check_never_holds_the_launch_window() { + let dir = temp("slow"); + let a = make_library(&dir.join("A")); + let gone = dir.join("Gone").to_string_lossy().into_owned(); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.settings.recent_libraries = vec![gone.clone(), a.clone()]; + app.ui.settings.library_path = gone.clone(); + let (checks, gate, _) = slow_checks(); + app.recent_checks = checks; + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let t0 = std::time::Instant::now(); + settle(&mut h); + assert!(t0.elapsed() < Duration::from_secs(5), "frames went on while the check waited: {:?}", t0.elapsed()); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert!(c.libraries.iter().all(|l| l.checking && l.unavailable.is_none()), "{c:?}"); + assert_eq!(c.selected, Some(0), "the last library opened, until the answers say otherwise"); + assert!(h.painted_text().iter().any(|t| t == "Checking…"), "{:?}", h.painted_text()); + assert!(h.app.session.activity.list().iter().any(|t| t.label == "Checking recent libraries"), "the activity stack lists the check"); + + release(&gate); + let done = + h.step_until(T, |h| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| c.libraries.iter().all(|l| !l.checking))); + assert!(done, "the answers came back"); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert!(c.libraries[0].unavailable.as_deref().is_some_and(|w| w.contains("Not found")), "{c:?}"); + assert_eq!(c.libraries[1].unavailable, None); + assert_eq!(c.selected, Some(1), "the first library that can be opened, once known"); + assert!(h.app.session.activity.list().iter().all(|t| t.label != "Checking recent libraries"), "the row goes with the check"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// Answers that arrive after the person moved the highlight leave it where they put it. +#[test] +fn late_answers_keep_the_row_the_person_chose() { + let dir = temp("touched"); + let a = make_library(&dir.join("A")); + let gone = dir.join("Gone").to_string_lossy().into_owned(); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.settings.recent_libraries = vec![a.clone(), gone.clone()]; + app.ui.settings.library_path = a.clone(); + let (checks, gate, _) = slow_checks(); + app.recent_checks = checks; + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + settle(&mut h); + h.request("ui.key", json!({"key": "ArrowDown"}), T); + settle(&mut h); + release(&gate); + h.step_until(T, |h| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| c.libraries.iter().all(|l| !l.checking))); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert_eq!(c.selected, Some(1), "still the row moved to, though it can't be opened"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// File ▸ Open Recent Library is rebuilt every frame: it never looks at a folder itself. A +/// library is greyed out only once a worker found it missing, and each path is checked once. +#[test] +fn the_recent_menu_takes_its_answers_from_a_worker() { + use crate::menubar::MenuNode; + let dir = temp("menu-worker"); + let gone = dir.join("Gone").to_string_lossy().into_owned(); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.settings.recent_libraries = vec![gone.clone()]; + let (checks, gate, calls) = slow_checks(); + app.recent_checks = checks; + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let enabled = |h: &Headless| match open_recent(&h.app).first() { + Some(MenuNode::Item { enabled, .. }) => *enabled, + other => panic!("{other:?}"), + }; + for _ in 0..30 { + h.step(); + assert!(enabled(&h), "not known yet: kept enabled (opening says why if it can't)"); + } + release(&gate); + assert!(h.step_until(T, |h| !h.app.recent_checks.is_running() && h.app.recent_checks.is_known(&gone)), "answered"); + for _ in 0..30 { + h.step(); + } + assert!(!enabled(&h), "found missing: greyed out"); + assert_eq!(calls.load(std::sync::atomic::Ordering::SeqCst), 1, "checked once, not every frame"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// The reasons are put into words on the UI thread, in the interface language (the worker that +/// found them has none of its own). +#[test] +fn the_reasons_are_in_the_interface_language() { + let dir = temp("lang"); + let gone = dir.join("Gone").to_string_lossy().into_owned(); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.language = crate::i18n::Locale::Fr; + app.ui.settings.recent_libraries = vec![gone.clone()]; + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + assert!( + h.step_until(T, |h| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| c.libraries.iter().all(|l| !l.checking))) + ); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + let why = c.libraries[0].unavailable.clone().unwrap_or_default(); + let english = "Not found: moved, renamed or on a disconnected drive"; + crate::i18n::set_language(crate::i18n::Locale::Fr); + assert_eq!(why, crate::i18n::tr(english), "translated on the UI thread"); + assert_ne!(why, english, "the French catalog has this reason"); + crate::i18n::set_language(crate::i18n::Locale::En); + let _ = std::fs::remove_dir_all(&dir); +} + +/// One drive that never answers doesn't hold the others: each path has its own worker, and the +/// row that waits says the drive isn't answering once it takes long. +#[test] +fn one_hung_drive_doesnt_hold_the_other_libraries() { + let dir = temp("hung"); + let a = make_library(&dir.join("A")); + let hung = dir.join("Hung").to_string_lossy().into_owned(); + let gate = std::sync::Arc::new((std::sync::Mutex::new(false), std::sync::Condvar::new())); + let g = gate.clone(); + let h_path = hung.clone(); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.recent_checks = crate::panels::library_problem::RecentChecks::with_check(move |p| { + if p == h_path { + let (lock, cv) = &*g; + let open = lock.lock().unwrap(); + let _unused = cv.wait_timeout_while(open, Duration::from_secs(10), |open| !*open).unwrap(); + } + None + }); + app.ui.settings.recent_libraries = vec![hung.clone(), a.clone()]; + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let a_done = h.step_until(T, |h| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| !c.libraries[1].checking)); + assert!(a_done, "the reachable library answered while the other one hangs"); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert!(c.libraries[0].checking, "the hung one is still checking"); + release(&gate); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A check whose worker dies (a panic) ends as "couldn't be checked" instead of leaving the row, +/// and every later check, waiting forever. +#[test] +fn a_check_that_dies_doesnt_leave_the_list_waiting() { + let dir = temp("dies"); + let a = make_library(&dir.join("A")); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.recent_checks = crate::panels::library_problem::RecentChecks::with_check(|_| panic!("a check that dies")); + app.ui.settings.recent_libraries = vec![a.clone()]; + app.library_problem = Some(LibraryProblem::choose_later(&app.ui.settings)); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let done = h.step_until(T, |h| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| !c.libraries[0].checking)); + assert!(done, "the row stopped waiting"); + assert!(!h.app.recent_checks.is_running(), "no check is left running"); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert!(c.libraries[0].unavailable.as_deref().is_some_and(|w| w.contains("couldn't be checked")), "{c:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A library found missing comes back once it is opened again (a drive plugged back in): the +/// menu no longer greys it out. +#[test] +fn opening_a_library_clears_an_old_missing_answer() { + use crate::menubar::MenuNode; + let dir = temp("back"); + let a = make_library(&dir.join("A")); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.settings.recent_libraries = vec![a.clone()]; + app.recent_checks.failed(&a, "the drive was disconnected"); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + h.app.run("app.openLibrary", json!({"path": a})).unwrap(); + for _ in 0..4 { + h.step(); + } + assert!(matches!(open_recent(&h.app).first(), Some(MenuNode::Item { enabled: true, .. })), "{:?}", open_recent(&h.app)); + assert_eq!(h.app.recent_checks.reason(&a), None); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/ui-egui/src/tests_switch_library.rs b/crates/ui-egui/src/tests_switch_library.rs index 2ea310b04..33815ddbe 100644 --- a/crates/ui-egui/src/tests_switch_library.rs +++ b/crates/ui-egui/src/tests_switch_library.rs @@ -163,6 +163,10 @@ fn missing_recent_libraries_are_greyed_out_and_pruned_and_the_list_is_capped() { let mut app = app_in(&dir); let gone = dir.join("gone").to_string_lossy().into_owned(); app.ui.settings.recent_libraries = vec![gone.clone()]; + // the menu takes what a background check found (issue #1290 review): let it answer first + let ctx = egui::Context::default(); + crate::panels::library_problem::logic(&mut app, &ctx); + assert!(crate::tasks::wait(&mut app, &ctx, std::time::Duration::from_secs(20)), "the check answered"); let disabled = recent_submenu(&app) .iter() .any(|n| matches!(n, crate::menubar::MenuNode::Item { params, enabled: false, .. } if params["path"] == gone.as_str())); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index d57b0fe9a..136d679c8 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -173,17 +173,24 @@ With Settings ▸ General ▸ At launch set to Ask me (`ui.set {"settings": {"at default, opens the last library), or Alt/Option held during the first 2 s after the window appears, the desktop app shows a window listing the recent libraries (issue #1290). `--library` and `LIGHTCRAFT_LIBRARY` skip it, and so does `--memory`; `lightcraft-cli` never shows it. `ui.inspect` → `libraryProblem.chooser` is `{libraries: [{path, -lastOpened, available, reason}], selected, libraryOpen}` (`available` is checked read-only: the folder and its catalog -file are looked at, never locked or written, so a library another program has open shows as available and Open -reports who has it in `libraryProblem.error`) (else `null`; `libraryOpen: true` when it was opened over -the last library with Alt). Each row is the widget `libraryRecent:`; the buttons are `button:libraryOpen`, +lastOpened, available, checking, reason}], selected, libraryOpen}` (else `null`; `libraryOpen: true` when it was opened +over the last library with Alt). Each library is checked read-only on a worker thread of its own, never on the UI +thread (the folder and its catalog file are looked at, never locked or written, so a library another program has +open shows as available and Open reports who has it in `libraryProblem.error`). `checking: true` until its check +answers (the row says *Checking…*, then that the drive isn't answering after 5 s); `available` is `false` only +once a check found a `reason`, so a row still checking can be opened. The selection starts on the last library and +moves to the first one that can be opened once every check has answered, unless it was moved meanwhile. Each row is the widget `libraryRecent:`; the buttons are `button:libraryOpen`, `button:libraryOpenAnother` (the folder dialog of `app.openLibrary`), `button:libraryNew` (`app.newLibrary`), `button:libraryForget` (Remove from List) and `button:libraryQuit`; `ui.key` ArrowUp / ArrowDown move the selection, Enter opens it and Escape quits (over an open library: closes the window and keeps it). `app.openLibrary {path}` opening a library ends it. While it shows, the settings are saved as usual (a removal is kept). -`app.recentLibraries` returns `[{path, name, exists, open, lastOpened}]` (`lastOpened`: ISO 8601, `null` when not -known; kept in `ui.json` → `settings.recentLibrariesOpened`). `{remove: path}` forgets one entry (an error if the list +`app.recentLibraries` returns `[{path, name, exists, checking, open, lastOpened}]`. `exists` is what the last +background check found (`false`: moved, on a disconnected drive or no longer a library; `null`: not checked yet) and +`checking` whether one is running: the command never looks at a folder itself. Recent libraries are checked in the +background when the app starts, again after 30 s, and when the launch window opens; File ▸ Open Recent Library +greys out only an entry a check found missing. `lastOpened`: ISO 8601, `null` when not known; kept in `ui.json` → +`settings.recentLibrariesOpened`. `{remove: path}` forgets one entry (an error if the list doesn't hold it) and `{clear: true}` (or `app.clearRecentLibraries`) all of them; nothing on disk is touched. ### Open Library on a Lightroom catalog diff --git a/docs/whats-new.md b/docs/whats-new.md index 35e69750a..16aa60cad 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -6,7 +6,9 @@ - Settings ▸ General ▸ **At launch**: *Open the last library* (as before) or *Ask me*. With *Ask me*, LightCraft starts with a window listing your recent libraries: each one's name, the folder it is in and when you last opened it. One that is gone (moved, renamed, on a disconnected drive), can't be read or no longer holds a library is - greyed out with the reason. Making the list only looks: it never locks or writes anything in your libraries, so one + greyed out with the reason. Each one is checked in the background, showing *Checking…* until it answers (and that + the drive isn't answering after 5 s), so the list never waits on a network share that is offline or a disk that + has to wake up; File ▸ Open Recent Library uses the same answers. Making the list only looks: it never locks or writes anything in your libraries, so one that another program has open is reported, with who has it, when you press **Open**, and the window stays. The last library is selected; **Open** (or Return, or a double-click) opens it, **Open Another…** asks for a folder, **New Library…** makes one, **Quit** (or Esc) quits. ↑ and ↓ move through