From e2210ed14ed55e744afae55e48004d4c3256d213 Mon Sep 17 00:00:00 2001 From: francoisgn <32835330+francoisgn@users.noreply.github.com> Date: Fri, 23 Oct 2026 19:18:52 +0300 Subject: [PATCH] #1290: a failed open or an unreadable catalog never marks a recent library missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round: whether a recent library is there is kept apart from whether it can be opened. `Unavailable::is_missing` (not found, a file, no library, a folder that can't be listed) is the only answer that greys out File ▸ Open Recent Library and makes `app.recentLibraries` say `exists: false`; an unreadable catalog or a failed check gives `null`. A failed open from the launch window (say another program has the library) drops the answer so it is checked again, instead of storing the open's error as one; the launch list keeps the error on its row. Test: Open on a locked library from the launch window, then the menu entry stays enabled and `exists` is true (fails if a failed open marks it missing); an unreadable catalog gives `exists: null`. Docs worded to match. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/menubar.rs | 2 +- crates/ui-egui/src/panels/library_problem.rs | 27 +++++++++-- crates/ui-egui/src/panels/settings.rs | 7 +-- crates/ui-egui/src/tests_launch_library.rs | 49 +++++++++++++++++++- docs/control-protocol.md | 8 ++-- docs/whats-new.md | 2 +- 6 files changed, 82 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 86f05a99f..8e0cb85ea 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -643,7 +643,7 @@ pub(crate) fn expanded(app: &LightcraftApp, name: &str) -> Option> let d = std::path::Path::new(r); // 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(); + let gone = app.recent_checks.exists(r) == Some(false); item("app.openLibrary", json!({"path": r}), label(r), None, can && !gone, Some(open.as_deref() == Some(d))) }) .collect(); diff --git a/crates/ui-egui/src/panels/library_problem.rs b/crates/ui-egui/src/panels/library_problem.rs index 9b77b6c58..ca11f5e5a 100644 --- a/crates/ui-egui/src/panels/library_problem.rs +++ b/crates/ui-egui/src/panels/library_problem.rs @@ -198,6 +198,13 @@ pub enum Unavailable { } impl Unavailable { + /// The folder isn't a library that is there to open (gone, a file, not a library any more, or + /// unlistable): what greys out File ▸ Open Recent Library and makes `app.recentLibraries` say + /// `exists: false`. An unreadable catalog or a check that failed doesn't say the library is gone. + pub fn is_missing(&self) -> bool { + matches!(self, Unavailable::NotFound | Unavailable::NotAFolder | Unavailable::NoLibrary | Unavailable::Unreadable(_)) + } + /// In the interface language. pub fn text(&self) -> String { match self { @@ -252,6 +259,16 @@ impl RecentChecks { self.known.get(path).and_then(|a| a.why.as_ref()) } + /// Whether the library at `path` is there: `Some(false)` when a check found it missing + /// ([`Unavailable::is_missing`]), `Some(true)` when one found it, `None` when no check could tell. + pub fn exists(&self, path: &str) -> Option { + match self.known.get(path)?.why.as_ref() { + None => Some(true), + Some(why) if why.is_missing() => Some(false), + Some(_) => None, + } + } + /// Whether a check of `path` has answered. pub fn is_known(&self, path: &str) -> bool { self.known.contains_key(path) @@ -282,9 +299,11 @@ impl RecentChecks { 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() }); + /// Opening `path` from the launch window failed. That says nothing about whether it is there + /// (another program may have it open): its answer is dropped, so it is checked again on the next + /// frame. The launch list keeps the open's error on that row for as long as it shows. + pub fn failed(&mut self, path: &str) { + self.known.remove(path); } /// `path` left the list: forget its answer. @@ -429,7 +448,7 @@ fn open(app: &mut LightcraftApp, path: Option) { // 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); + app.recent_checks.failed(path); } if let Some(p) = app.library_problem.as_mut() { if let Some(c) = p.chooser.as_mut() { diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 86c4fd882..70d079367 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -1307,8 +1307,9 @@ 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, 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 +/// background check found (`false`: moved, on a disconnected drive, a folder that can't be listed +/// or no longer a library; `null`: not checked yet, or the check couldn't tell — an unreadable catalog, a check +/// that failed; `checking` 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 @@ -1337,7 +1338,7 @@ pub fn recent_libraries(app: &mut LightcraftApp, p: &Value) -> Result>()); + settle(&mut h); + let c = h.app.library_problem.as_ref().and_then(|p| p.chooser.clone()).unwrap(); + assert!(c.libraries[0].unavailable.is_some(), "the row shows why it didn't open: {c:?}"); + assert!(h.step_until(T, |h| !h.app.recent_checks.is_running() && h.app.recent_checks.is_known(&held)), "checked again"); + assert_eq!(h.app.recent_checks.exists(&held), Some(true), "a failed open doesn't mark it missing"); + let list = h.app.run("app.recentLibraries", json!({})).unwrap(); + assert_eq!(list[0]["exists"], true, "{list}"); + assert!(matches!(open_recent(&h.app).first(), Some(MenuNode::Item { enabled: true, .. })), "{:?}", open_recent(&h.app)); + drop(lock); + + // an unreadable catalog: not known to be gone + h.app.recent_checks = crate::panels::library_problem::RecentChecks::with_check(|_| { + Some(crate::panels::library_problem::Unavailable::CatalogUnreadable("denied".into())) + }); + assert!(h.step_until(T, |h| h.app.recent_checks.is_known(&held) && !h.app.recent_checks.is_running())); + assert_eq!(h.app.run("app.recentLibraries", json!({})).unwrap()[0]["exists"], serde_json::Value::Null); + assert!(matches!(open_recent(&h.app).first(), Some(MenuNode::Item { enabled: true, .. }))); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 136d679c8..c51c7d708 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -186,10 +186,12 @@ Enter opens it and Escape quits (over an open library: closes the window and kee opening a library ends it. While it shows, the settings are saved as usual (a removal is kept). `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 check found (`false`: moved, on a disconnected drive, a folder that can't be listed or no longer a +library; `null`: not checked yet, or the check couldn't tell, e.g. an unreadable catalog) and `checking` whether one +is running: the command never looks at a folder itself. A failed open never marks a library missing (another program +may have it). 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` → +greys out only an entry a check found missing (an unreadable catalog or a failed open doesn't grey it out). `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. diff --git a/docs/whats-new.md b/docs/whats-new.md index 16aa60cad..68d92e110 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -8,7 +8,7 @@ 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. 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 + has to wake up; File ▸ Open Recent Library uses the same answers (it greys out only libraries that are gone). 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