From 9bb072fe4a50a3c610dbceb9583a357efc4dd8bc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 23 Oct 2026 14:37:06 +0000 Subject: [PATCH] #2672: a stale destination window refuses Create a new library too; tests read confirm's error, not the last toast Review follow-up on #2685. CodeRabbit: the library-switch check ran for Merge only, so a window left over from before a switch could still make a library from defaults that described the old one; it now covers both choices (new test). The Mac session: on macOS the switch's own notice can replace the error toast before the test reads it, so the stale-window tests check confirm's error directly. docs/gaps.md gap 13 updated with the parity row (CodeRabbit). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- crates/ui-egui/src/lightroom_destination.rs | 5 ++- .../src/tests_lightroom_destination.rs | 39 ++++++++++++++++++- docs/gaps.md | 5 ++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/lightroom_destination.rs b/crates/ui-egui/src/lightroom_destination.rs index 08c13e747..a62c59825 100644 --- a/crates/ui-egui/src/lightroom_destination.rs +++ b/crates/ui-egui/src/lightroom_destination.rs @@ -188,12 +188,13 @@ fn params(catalog: &str, new_library: bool, name: &str, location: &str) -> Value /// The window's confirm button: import as chosen. "Don't ask again" is kept once a merge has /// started (it only applies to merging). Another library opened while the window was up (by an -/// agent, the control channel) is not merged into. +/// agent, the control channel) makes the window stale for both choices: nothing is merged into +/// it, and no new library is made from defaults that described the library it replaced. pub fn confirm(app: &mut LightcraftApp, dialog: &Dialog) -> Result { let Dialog::LightroomDestination { catalog, new_library, name, location, dont_ask, generation, .. } = dialog else { return Err("not the Lightroom catalog destination window".into()); }; - if !*new_library && *generation != app.session.library_generation() { + if *generation != app.session.library_generation() { return Err(crate::i18n::tr("Another library was opened since this window appeared: nothing was imported").to_string()); } let r = app.run("file.importLightroom", params(catalog, *new_library, name, location))?; diff --git a/crates/ui-egui/src/tests_lightroom_destination.rs b/crates/ui-egui/src/tests_lightroom_destination.rs index df72bd535..b2def730b 100644 --- a/crates/ui-egui/src/tests_lightroom_destination.rs +++ b/crates/ui-egui/src/tests_lightroom_destination.rs @@ -380,12 +380,47 @@ fn merge_is_refused_when_another_library_was_opened_meanwhile() { let other = f.root.join("Other"); let dialog = h.app.ui.dialog.take(); h.app.run("app.openLibrary", json!({"path": other.to_string_lossy()})).unwrap(); + let stale = dialog.clone(); h.app.ui.dialog = dialog; h.step(); click(&mut h, "button:dialogOk"); assert!(h.app.ui.dialog.is_none(), "the window closes: the library it named is gone"); - let toast = h.app.ui.toast.clone().map(|t| t.0).unwrap_or_default(); - assert!(toast.contains("nothing was imported"), "{toast}"); + // confirm's own error, not the last toast: another notice (the library switch's, on macOS) + // can replace the toast before it is read + let stale = stale.unwrap(); + let err = crate::lightroom_destination::confirm(&mut h.app, &stale).unwrap_err(); + assert!(err.contains("nothing was imported"), "{err}"); assert!(h.app.lightroom.is_none() && h.app.lightroom_last.is_none()); assert_eq!(open_dir(&h), Some(other)); } + +/// The same for *Create a new library* (CodeRabbit on #2685): a window left over from before a +/// library switch makes no library and imports nothing. +#[test] +fn a_new_library_is_refused_when_another_library_was_opened_meanwhile() { + let f = Fixture::new("stale-new"); + let mut h = f.app(false); + h.app.run("file.importLightroom", json!({"path": f.lrcat()})).unwrap(); + h.step(); + click(&mut h, "radio:lightroomDestination-new"); + let other = f.root.join("Other"); + let dialog = h.app.ui.dialog.take(); + let target = match &dialog { + Some(crate::state::Dialog::LightroomDestination { name, location, .. }) => std::path::Path::new(location).join(name), + _ => panic!("the destination window is up"), + }; + h.app.run("app.openLibrary", json!({"path": other.to_string_lossy()})).unwrap(); + let stale = dialog.clone(); + h.app.ui.dialog = dialog; + h.step(); + click(&mut h, "button:dialogOk"); + assert!(h.app.ui.dialog.is_none(), "the window closes: the library it named is gone"); + // confirm's own error, not the last toast: another notice (the library switch's, on macOS) + // can replace the toast before it is read + let stale = stale.unwrap(); + let err = crate::lightroom_destination::confirm(&mut h.app, &stale).unwrap_err(); + assert!(err.contains("nothing was imported"), "{err}"); + assert!(h.app.lightroom.is_none() && h.app.lightroom_last.is_none()); + assert_eq!(open_dir(&h), Some(other)); + assert!(!target.exists(), "no library was made at {}", target.display()); +} diff --git a/docs/gaps.md b/docs/gaps.md index be411a205..b3680e66b 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-21 · **Change:** minor (gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-23 · **Change:** minor (gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -27,7 +27,7 @@ Hours are Opus 5.5 agent wall-clock hours (see [target-app-parity.md](target-app | 10 B | **Per-model verification thin, plus cheap per-body decode fixes**: 551 of 1,446 Adobe models verified (38%), ~85% of photographers on a verified body; Nikon 5 bodies verified, Canon 34, Leica 14 (the Q3 by a recorded check of 38 private DNGs). Cheap fixes: Z 6II packed 14-bit NEF (#769, PR #671), Sony A100 regression (#702); F828 SRF now discovered and imported as preview-only, sensor decoding still missing, DSLR-A200…A390 / A850 12-bit ARW (#535) | measured: [raw/coverage.tsv](raw/coverage.tsv) | per-body bugs (levels, crops, WB) surface only from users | 25–45 | [raw-parity](raw-parity.md#by-maker-measured) | | 11 | **Enhance: Raw Details, Super Resolution; AI denoise without shippable weights or X-Trans** | ENH rows 0%; denoise Bayer only, RawNIND weights GPL and opt-in | Fujifilm users get no AI denoise; nobody gets it out of the box | 30–60 + model decision | [raw-parity](raw-parity.md#demosaic-levels-crops-and-lens-metadata) | | 12 | **Content-aware / generative remove, detect distractions, reflections removal** | REM rows 75%; patch synthesis missing | cleanup work goes back to Photoshop | 25–45 (+ generative model) | [target-app-parity](target-app-parity.md#ai) | -| 13 | **Lightroom catalog migration is one-way and approximate**: unsupported Adobe profiles / AI settings archived, history kept as data; main-file pages read on demand up to 16 GiB, with dense-table and WAL bounds retained (#798); offline record counts and destination are now shown in a lasting import result | lightroom-catalog-import.md | switching users lose some looks | 8–12 | [file-format-parity](file-format-parity.md) | +| 13 | **Lightroom catalog migration is one-way and approximate**: unsupported Adobe profiles / AI settings archived, history kept as data; main-file pages read on demand up to 16 GiB, with dense-table and WAL bounds retained (#798); offline record counts and destination are now shown in a lasting import result; before an import the app asks whether the catalog merges into the open library or goes into a new library (opt-out in Settings, #2672) | lightroom-catalog-import.md | switching users lose some looks | 8–12 | [file-format-parity](file-format-parity.md) | | 14 | **HDR display** (EDR / Windows HDR); the headroom limit and SDR rendition from `crs` XMP (the HDR switch, `HDREditMode`, is read). HLG AVIF export done; the display's SDR/HDR decision is tested, blocked on eframe's 8-bit surface | HDR rows 70%; LR-VIEW-HDR-DISPLAY ⬜ | HDR editing works blind on HDR monitors | 12–25 | [hardware-parity](hardware-parity.md) | | 15 | **Print module**: identity plate / watermark / photo info overlays, Cut Guides for the packages, Custom Package Auto Layout, printer-driver Page Setup and direct printing, JPEG file output (the Print dialog writes a colour-managed PDF) | LRC-PRINT rows | Classic users who print | 15–25 | [target-app-parity](target-app-parity.md#features) | | 16 | **Map view, Book, Slideshow module, publish services** | LRC-MAP / BOOK / SS rows ⬜ | Classic users; publish services matter for Flickr / SmugMug workflows | 60–100 | [target-app-parity](target-app-parity.md#features) | @@ -79,6 +79,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-23 | minor | Gap 13: before a Lightroom catalog import the app asks whether it merges into the open library or goes into a new one (opt-out in Settings, #2672); hours unchanged | | 2026-10-21 | minor | Gap 28 shrinks: F8 shows / hides the right panel (`view.rightPanel`); rating `[` `]`, flag cycling and F1 help were already bound, so the entry lists only filter-bar keys, Tab / ⇧Tab and F5; 2–4 h (was 4–8) | | 2026-10-20 | minor | #2362: import destination albums use the shared hierarchy/search picker instead of a flat name list; existing import workflows retained; ranked gaps/readiness counts unchanged | | 2026-10-19 | minor | Gap 21: Indonesian (`id`) shipped with every catalog message but What's New; four new languages left (Hindi, Arabic, Korean, Vietnamese), 135–239 h |