Merge #2701@142e496f5 (a failed lookup isn't "not found"; no timer on found libraries) into round3a-integration

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
francoisgn
2026-10-24 05:34:24 +03:00
co-authored by Claude Opus 5.5
4 changed files with 133 additions and 16 deletions
+2 -1
View File
@@ -2386,5 +2386,6 @@
"This library couldn't be checked": "Impossible de vérifier cette bibliothèque",
"Checking recent libraries": "Vérification des bibliothèques récentes",
"The photos are imported once a library is open": "Les photos seront importées dès qu'une bibliothèque sera ouverte",
"Open a library first, then drop the presets again": "Ouvrez d'abord une bibliothèque, puis déposez à nouveau les préréglages"
"Open a library first, then drop the presets again": "Ouvrez d'abord une bibliothèque, puis déposez à nouveau les préréglages",
"This library couldn't be checked:": "Impossible de vérifier cette bibliothèque :"
}
+55 -13
View File
@@ -195,6 +195,9 @@ pub enum Unavailable {
CatalogUnreadable(String),
/// The check itself failed (its worker stopped, or couldn't start).
CheckFailed(String),
/// Looking at the path failed without saying it is gone (a parent folder it may not read, a
/// share that timed out; the system's reason).
LookupFailed(String),
}
impl Unavailable {
@@ -202,7 +205,8 @@ impl Unavailable {
/// 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(_))
// a folder that refuses to list is there (permissions, macOS privacy): opening it says why
matches!(self, Unavailable::NotFound | Unavailable::NotAFolder | Unavailable::NoLibrary)
}
/// In the interface language.
@@ -214,12 +218,15 @@ impl Unavailable {
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(),
Unavailable::LookupFailed(e) => format!("{} {e}", crate::i18n::tr("This library couldn't be checked:")),
}
}
}
/// How long an answer stands before File ▸ Open Recent Library asks again (a drive plugged back
/// in, a library moved away meanwhile).
/// How old an answer must be to be asked again: a library not found, on its own (a drive plugged
/// back in); any other, when the window comes back to the front (a library moved away meanwhile).
/// Only "not found" goes on a timer: looking again at the others would wake every external drive
/// and network share in the list over and over.
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).
@@ -244,12 +251,22 @@ pub struct RecentChecks {
pending: std::collections::HashMap<String, web_time::Instant>,
/// How one path is checked ([`unavailable`]; tests put a slow one here).
check: std::sync::Arc<dyn Fn(&str) -> Option<Unavailable> + Send + Sync>,
/// [`RECHECK_AFTER`] (shorter in tests).
recheck_after: std::time::Duration,
/// Whether the window had the focus on the last frame: answers are refreshed when it comes back.
pub(crate) focused: bool,
}
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) }
RecentChecks {
known: Default::default(),
pending: Default::default(),
check: std::sync::Arc::new(unavailable),
recheck_after: RECHECK_AFTER,
focused: false,
}
}
}
@@ -294,6 +311,13 @@ impl RecentChecks {
RecentChecks { check: std::sync::Arc::new(check), ..Default::default() }
}
/// Ask again about answers older than `after` when the window comes back (tests).
#[cfg(test)]
pub fn recheck_after(mut self, after: std::time::Duration) -> RecentChecks {
self.recheck_after = after;
self
}
/// `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() });
@@ -387,14 +411,18 @@ fn apply_answers(app: &mut LightcraftApp) {
/// who has it. Runs on a worker: the reason is put into words on the UI thread.
fn unavailable(path: &str) -> Option<Unavailable> {
let dir = Path::new(path);
if !dir.exists() {
return Some(Unavailable::NotFound);
// only a definite answer marks it missing: a lookup that fails otherwise (a parent folder it may
// not read, a share that timed out) is "couldn't check", and the library can still be opened
match std::fs::metadata(dir) {
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Some(Unavailable::NotFound),
Err(e) => return Some(Unavailable::LookupFailed(e.to_string())),
Ok(m) if !m.is_dir() => return Some(Unavailable::NotAFolder),
Ok(_) => {}
}
if !dir.is_dir() {
return Some(Unavailable::NotAFolder);
}
if let Err(e) = std::fs::read_dir(dir) {
return Some(Unavailable::Unreadable(e.to_string()));
match std::fs::read_dir(dir) {
Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => return Some(Unavailable::Unreadable(e.to_string())),
Err(e) => return Some(Unavailable::LookupFailed(e.to_string())),
Ok(_) => {}
}
// opening would make a new, empty library in it
if !lightcraft_engine::library::is_library(dir) {
@@ -524,13 +552,27 @@ pub fn logic(app: &mut LightcraftApp, ctx: &egui::Context) {
start_checks(app, listed);
apply_answers(app);
}
// File ▸ Open Recent Library: paths never checked, or not for a while
// File ▸ Open Recent Library: paths never checked; libraries not found, every RECHECK_AFTER (a
// drive plugged back in; looking at a path that isn't there wakes nothing); the others when the
// window comes back to the front and their answer is that old. The list as shown (at most
// RECENT_LIBRARIES).
let focused = ctx.input(|i| i.focused);
let regained = focused && !std::mem::replace(&mut app.recent_checks.focused, focused);
let after = app.recent_checks.recheck_after;
let stale: Vec<String> = 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))
.take(crate::state::RECENT_LIBRARIES)
.filter(|p| {
!app.recent_checks.is_checking(p)
&& app
.recent_checks
.known
.get(*p)
.is_none_or(|a| (regained || matches!(a.why, Some(Unavailable::NotFound))) && a.at.elapsed() > after)
})
.cloned()
.collect();
start_checks(app, stale);
@@ -866,3 +866,74 @@ fn photos_dropped_on_the_launch_list_go_into_the_library_chosen() {
drop(h);
let _ = std::fs::remove_dir_all(&dir);
}
/// A lookup that fails without saying the library is gone (here a parent folder it may not read;
/// a network share that timed out is the same) is "couldn't check", not "not found": the entry
/// isn't greyed out in File ▸ Open Recent Library (CodeRabbit on #2689).
#[cfg(unix)]
#[test]
fn a_failed_lookup_doesnt_mark_a_recent_library_missing() {
use std::os::unix::fs::PermissionsExt;
let dir = temp("lookup");
let locked = dir.join("Locked");
let inside = make_library(&locked.join("Inside"));
let gone = dir.join("Gone").to_string_lossy().into_owned();
std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap();
let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default());
app.ui.settings.recent_libraries = vec![inside.clone(), gone.clone()];
let mut h = Headless::new(app, [900.0, 600.0], 1.0);
let answered = h.step_until(T, |h| h.app.recent_checks.is_known(&inside) && h.app.recent_checks.is_known(&gone));
std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o755)).unwrap();
assert!(answered);
assert_eq!(h.app.recent_checks.exists(&gone), Some(false), "a library that isn't there is missing");
assert_eq!(h.app.recent_checks.exists(&inside), None, "one that couldn't be looked at isn't");
let why = h.app.recent_checks.reason(&inside).map(|w| w.text()).unwrap_or_default();
assert!(why.starts_with("This library couldn't be checked:"), "{why}");
// nor is a folder that is there but refuses to be listed (permissions, macOS privacy)
use crate::panels::library_problem::Unavailable;
assert!(!Unavailable::Unreadable("denied".into()).is_missing() && !Unavailable::LookupFailed("timed out".into()).is_missing());
assert!(Unavailable::NotFound.is_missing());
drop(h);
let _ = std::fs::remove_dir_all(&dir);
}
/// A library found is asked about again when the window comes back to the front, not on a timer: a
/// timer would wake every external drive and network share in the list over and over. One not
/// found is looked for again on its own (a drive plugged back in with the app in front).
#[test]
fn recent_libraries_are_checked_again_only_when_the_window_comes_back() {
use std::sync::atomic::{AtomicUsize, Ordering};
let dir = temp("recheck");
let a = make_library(&dir.join("A"));
let gone = dir.join("Gone").to_string_lossy().into_owned();
let (calls, gone_calls) = (std::sync::Arc::new(AtomicUsize::new(0)), std::sync::Arc::new(AtomicUsize::new(0)));
let (c, g, gone_path) = (calls.clone(), gone_calls.clone(), gone.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 == gone_path {
g.fetch_add(1, Ordering::SeqCst);
return Some(crate::panels::library_problem::Unavailable::NotFound);
}
c.fetch_add(1, Ordering::SeqCst);
None
})
.recheck_after(Duration::ZERO);
app.ui.settings.recent_libraries = vec![a.clone(), gone.clone()];
let mut h = Headless::new(app, [900.0, 600.0], 1.0);
assert!(h.step_until(T, |h| h.app.recent_checks.exists(&a) == Some(true)));
assert!(h.step_until(T, |_| gone_calls.load(Ordering::SeqCst) >= 3), "the one not found is looked for again on its own");
// focused all along, every answer "old": nothing is asked again
for _ in 0..30 {
h.step();
}
assert_eq!(calls.load(Ordering::SeqCst), 1, "no check on a timer");
// the window went to the back and comes to the front again
h.app.recent_checks.focused = false;
assert!(h.step_until(T, |_| calls.load(Ordering::SeqCst) == 2), "asked again when the window comes back");
for _ in 0..10 {
h.step();
}
assert_eq!(calls.load(Ordering::SeqCst), 2, "once");
drop(h);
let _ = std::fs::remove_dir_all(&dir);
}
+5 -2
View File
@@ -194,8 +194,11 @@ background check found (`false`: moved, on a disconnected drive, a folder that c
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 (an unreadable catalog or a failed open doesn't grey it out). `lastOpened`: ISO 8601, `null` when not known; kept in `ui.json` →
background when the app starts (the first 8) and when the launch window opens; one not found is looked for again every
30 s (a drive plugged back in), the others only when the window comes back to the front (a timer would wake external
drives and shares). File ▸ Open Recent Library greys out only an entry a check found missing (not there, not a folder,
no library in it); an unreadable catalog, a folder that refuses to be listed, a failed open or a lookup that failed
(a share that timed out) 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.