#1290: recent libraries are checked on workers, never on the UI thread

Review round (CodeRabbit + an adversarial review, handled together):
- The launch list starts with every row "Checking…" and a worker per path checks it (a
  share whose server is gone, or a sleeping disk, can take seconds or never answer; one
  that hangs doesn't hold the others). After 5 s a row says the drive isn't answering;
  it can still be opened. Once every answer is in, the selection moves to the first
  library that can be opened, unless the person moved it.
- File ▸ Open Recent Library and `app.recentLibraries` no longer look at folders either
  (the menu is rebuilt every frame): they use the same answers, refreshed after 30 s, when
  a library is opened, removed from the list, or fails to open. `app.recentLibraries`
  gives `exists: null` and `checking` until a check answered.
- Reasons are found on the worker and put into words on the UI thread, in the interface
  language. A check whose worker dies ends as "This library couldn't be checked".
- Each check holds an activity row ("Checking recent libraries"; quick ones don't show).
- Docs: control protocol (`checking`, `available`, `exists`), What's New; fr strings.
Tests: slow check never holds the window; late answers keep the person's selection; the
menu takes its answers from a worker (fails with the old is_dir()); reasons in French; one
hung drive doesn't hold the others; a dying check ends; reopening clears a missing answer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
francoisgn
2026-10-23 18:46:42 +03:00
parent 3fe02631e6
commit a4ed555d45
10 changed files with 547 additions and 39 deletions
+3 -3
View File
@@ -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();
+4 -1
View File
@@ -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"
}
+6
View File
@@ -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)());
+4 -2
View File
@@ -641,8 +641,10 @@ pub(crate) fn expanded(app: &LightcraftApp, name: &str) -> Option<Vec<MenuNode>>
.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() {
+286 -20
View File
@@ -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<String>,
/// 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<String>,
/// 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<RecentLibrary>,
/// The highlighted row (`None`: the list is empty).
pub selected: Option<usize>,
/// 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<u64>,
@@ -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<RecentLibrary> = 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<serde_json::Value> = 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<usize> {
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<Unavailable>,
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<String, Answer>,
/// Paths being checked, and since when.
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>,
}
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<web_time::Instant> {
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<Unavailable> + 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<String>) {
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<String> = 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<String> {
/// 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(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<String>) {
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<String>) {
}
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<Vec<String>> =
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<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))
.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());
+12 -5
View File
@@ -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<Value, String> {
@@ -1317,21 +1320,25 @@ pub fn recent_libraries(app: &mut LightcraftApp, p: &Value) -> Result<Value, Str
if !forget_library(&mut app.ui.settings, path) {
return Err(format!("app.recentLibraries: {path:?} is not in the recent libraries"));
}
app.recent_checks.forget(path);
}
Some(_) => 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<Value> = 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))
+212 -1
View File
@@ -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<bool>, std::sync::Condvar)>,
std::sync::Arc<std::sync::atomic::AtomicUsize>,
) {
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<bool>, 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);
}
@@ -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()));
+13 -6
View File
@@ -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:<index>`; 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:<index>`; 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
+3 -1
View File
@@ -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