mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 05:57:09 -06:00
fix(lightroom): content duplicates start from import defaults; Show in Library skips Recently Deleted (#2589)
A catalog photo whose bytes match another library photo got the generic develop defaults, so a raw with no white balance or lens setting in the catalog lost its as-shot white balance and embedded lens corrections and rendered unlike a file import. It now gets the import defaults (camera defaults plus the default preset), as a file import does, before the catalog's settings go on top. Show in Library selected the lowest mapped photo id, which can be a master left in Recently Deleted whose new virtual copy the import added; All Photos doesn't show it. It now selects the lowest mapped photo not in Recently Deleted, and the button is left out when there is none. Tests: a content-duplicate raw with no catalog settings equals its camera defaults (as-shot WB, lens profile on); Show in Library skips a trashed mapped photo and offers nothing when only trashed or vanished ids are mapped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
7c74ae0ac3
commit
a31a43670b
@@ -1033,6 +1033,9 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co
|
||||
p.meta.iso = m.iso;
|
||||
p.meta.gps = m.gps;
|
||||
p.meta.focus = m.focus.clone();
|
||||
// the settings a file import gives it (as-shot white balance, embedded lens
|
||||
// corrections, the default preset); the catalog's settings apply on top
|
||||
crate::import::apply_import_defaults(s, &mut p);
|
||||
p
|
||||
} else {
|
||||
let id = s.catalog.alloc_photo_id();
|
||||
|
||||
@@ -245,6 +245,52 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo()
|
||||
assert_eq!(s.catalog.photo(other).unwrap().deleted, case == "trashed", "{case}: the other photo is left as it was");
|
||||
}
|
||||
}
|
||||
/// A content duplicate starts from the settings a file import gives a raw (its as-shot white
|
||||
/// balance and embedded lens corrections), as its file would on its own, before the catalog's
|
||||
/// settings go on top; not from the generic defaults (#2589).
|
||||
#[test]
|
||||
fn a_content_duplicate_raw_starts_from_its_camera_defaults() {
|
||||
let mut s = crate::Session::new();
|
||||
let other = library_photo(&mut s, "/elsewhere/a.DNG", |p| {
|
||||
p.deleted = true;
|
||||
p.format = "DNG".into();
|
||||
p.kind = lightcraft_catalog::MediaKind::Raw;
|
||||
p.as_shot_wb = Some((5200.0, 4.0));
|
||||
p.embedded_lens = Some(lightcraft_develop::EmbeddedLens::default());
|
||||
});
|
||||
let mut data = sample();
|
||||
data.photos.retain(|p| number(&p.image, "masterImage") == 0);
|
||||
// a catalog row with no develop settings of its own
|
||||
data.photos[0].settings.clear();
|
||||
let files = crate::import::ImportReport {
|
||||
duplicates: vec![crate::import::Duplicate {
|
||||
path: "/missing/test.ARW".into(),
|
||||
existing: Some(other.0),
|
||||
reason: "content",
|
||||
existing_deleted: true,
|
||||
}],
|
||||
..Default::default()
|
||||
};
|
||||
let undo0 = s.undo.len();
|
||||
let r = apply_prepared(
|
||||
&mut s,
|
||||
data,
|
||||
ApplyPrepared {
|
||||
update_existing: false,
|
||||
files,
|
||||
index: &mut ImportIndex::default(),
|
||||
archive_path: None,
|
||||
missing_sources: &HashSet::new(),
|
||||
undo0,
|
||||
},
|
||||
)
|
||||
.unwrap()
|
||||
.report;
|
||||
let new = s.catalog.photo(PhotoId(r["mapping"]["1"].as_u64().unwrap())).unwrap();
|
||||
assert_eq!(*new.develop, new.camera_defaults(), "{r}");
|
||||
assert_eq!((new.develop.wb.temp, new.develop.wb.tint), (5200.0, 4.0), "the file's as-shot white balance");
|
||||
assert!(new.develop.optics.lens_profile, "the file's embedded lens corrections are on");
|
||||
}
|
||||
/// A catalog photo matched (by path) to a library photo in Recently Deleted leaves it there, as
|
||||
/// the import was set up to do (a Lightroom import doesn't restore, see `OnDeleted`), and the report
|
||||
/// counts it so the photos don't go missing unexplained (#2589). A new virtual copy of it is
|
||||
|
||||
@@ -303,7 +303,7 @@ fn terminal(app: &mut LightcraftApp, ctx: &egui::Context, task: &LightroomTask,
|
||||
}
|
||||
};
|
||||
if matches!(kind, Kind::Import) && value.get("cancelled") != Some(&Value::Bool(true)) {
|
||||
app.lightroom_result = Some(result_dialog(&value, &message));
|
||||
app.lightroom_result = Some(result_dialog(&value, &message, &app.session.catalog));
|
||||
show_result(app);
|
||||
}
|
||||
app.lightroom_last = Some(value);
|
||||
@@ -318,8 +318,9 @@ fn show_result(app: &mut LightcraftApp) {
|
||||
}
|
||||
|
||||
/// The lasting result window for an inspect or import: counts, failures, missing originals, the
|
||||
/// skipped images (one line, in the UI language) and at most eight warnings.
|
||||
fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog {
|
||||
/// skipped images (one line, in the UI language) and at most eight warnings. Show in Library goes
|
||||
/// to the lowest mapped photo that is not in Recently Deleted, where All Photos can show it.
|
||||
fn result_dialog(value: &Value, summary: &str, catalog: &lightcraft_catalog::Catalog) -> crate::state::Dialog {
|
||||
let report = value.get("report").unwrap_or(value);
|
||||
let mut details = Vec::new();
|
||||
if let Some(photos) = report["photos"].as_u64() {
|
||||
@@ -366,7 +367,10 @@ fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog {
|
||||
details.push(crate::i18n::tr_format!("{count} more warnings.", count = warnings.len() - 8));
|
||||
}
|
||||
}
|
||||
let first = value.get("error").is_none().then(|| report["mapping"].as_object()?.values().filter_map(Value::as_u64).min()).flatten();
|
||||
// a catalog photo can map to a library photo left in Recently Deleted (#2589), which All
|
||||
// Photos doesn't show
|
||||
let live = |id: &u64| catalog.photo(lightcraft_catalog::PhotoId(*id)).is_some_and(|p| !p.deleted);
|
||||
let first = value.get("error").is_none().then(|| report["mapping"].as_object()?.values().filter_map(Value::as_u64).filter(live).min()).flatten();
|
||||
crate::state::Dialog::LightroomResult {
|
||||
summary: summary.to_string(),
|
||||
library: report["library"].as_str().or_else(|| value["library"].as_str()).map(str::to_string),
|
||||
@@ -680,6 +684,33 @@ mod tests {
|
||||
assert!(!h.app.session.visible().is_empty());
|
||||
}
|
||||
|
||||
/// Show in Library goes to a photo All Photos shows: not a master left in Recently Deleted
|
||||
/// whose new virtual copy the import added (#2589), and nowhere when no mapped photo is shown.
|
||||
#[test]
|
||||
fn show_in_library_skips_photos_in_recently_deleted() {
|
||||
use lightcraft_catalog::{Op, Photo, Source};
|
||||
let mut s = lightcraft_engine::Session::new();
|
||||
let mut add = |deleted: bool| {
|
||||
let id = s.catalog.alloc_photo_id();
|
||||
let mut p = Photo::new(id, Source::File { path: format!("/p/{}.jpg", id.0) }, "a.jpg", "JPEG", 2, 2, "now");
|
||||
p.deleted = deleted;
|
||||
s.commit("Add", Op::AddPhoto { photo: Box::new(p) }).unwrap();
|
||||
id.0
|
||||
};
|
||||
let (trashed, live) = (add(true), add(false));
|
||||
assert!(trashed < live);
|
||||
let first = |mapping: Value| {
|
||||
let crate::state::Dialog::LightroomResult { first, .. } = result_dialog(&json!({"report": {"mapping": mapping}}), "done", &s.catalog)
|
||||
else {
|
||||
panic!("not a Lightroom result")
|
||||
};
|
||||
first
|
||||
};
|
||||
assert_eq!(first(json!({"1": trashed, "2": live})), Some(live));
|
||||
assert_eq!(first(json!({"1": trashed})), None, "only a photo in Recently Deleted: no Show in Library");
|
||||
assert_eq!(first(json!({"1": live + 100})), None, "no longer in the library");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lightroom_failure_stays_readable_without_offering_a_success_action() {
|
||||
let mut h = demo();
|
||||
@@ -714,7 +745,9 @@ mod tests {
|
||||
{"image": 9, "reason": "masterSkipped"},
|
||||
]}});
|
||||
crate::i18n::set_language(crate::i18n::Locale::En);
|
||||
let crate::state::Dialog::LightroomResult { details: en, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") };
|
||||
let crate::state::Dialog::LightroomResult { details: en, .. } = result_dialog(&value, "done", &Default::default()) else {
|
||||
panic!("not a Lightroom result")
|
||||
};
|
||||
assert_eq!(en.len(), 4, "{en:?}");
|
||||
assert_eq!(en[1], "3 catalog images were not imported because their records in the catalog are incomplete.");
|
||||
assert_eq!(&en[2..], ["w1", "w2"]);
|
||||
@@ -724,11 +757,15 @@ mod tests {
|
||||
fn photos_left_in_recently_deleted_are_one_line() {
|
||||
let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 2}});
|
||||
crate::i18n::set_language(crate::i18n::Locale::En);
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") };
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else {
|
||||
panic!("not a Lightroom result")
|
||||
};
|
||||
assert_eq!(details.len(), 2, "{details:?}");
|
||||
assert!(details[1].starts_with("2 catalog photos are in this library's Recently Deleted"), "{details:?}");
|
||||
let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 0}});
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") };
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else {
|
||||
panic!("not a Lightroom result")
|
||||
};
|
||||
assert_eq!(details.len(), 1, "no line when none are: {details:?}");
|
||||
}
|
||||
|
||||
@@ -737,7 +774,9 @@ mod tests {
|
||||
let warnings: Vec<_> = (0..10).map(|i| format!("w{i}")).collect();
|
||||
let skipped: Vec<_> = (0..500).map(|i| json!({"image": i, "reason": "missingFile"})).collect();
|
||||
let value = json!({"report": {"warnings": warnings, "skipped": skipped}});
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") };
|
||||
let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else {
|
||||
panic!("not a Lightroom result")
|
||||
};
|
||||
// one line for the skipped images, then the usual eight warnings and the rest counted
|
||||
assert_eq!(details.len(), 10, "{details:?}");
|
||||
assert!(details[0].starts_with("500 catalog images were not imported"));
|
||||
|
||||
@@ -52,7 +52,8 @@ lightcraft-cli run --library "LightCraft Library" library.importLightroom path="
|
||||
The import itself (`library.importLightroom`, `lightcraft-cli run`) adds records to the currently open LightCraft
|
||||
library; only the desktop window above can make a new library for it first. The desktop result stays open until dismissed and
|
||||
shows the destination library, the number of records added, missing originals and warnings.
|
||||
**Show in Library** opens All Photos, clears filters and selects an imported record. Errors stay
|
||||
**Show in Library** opens All Photos, clears filters and selects an imported record (not one left in Recently
|
||||
Deleted; the button is left out when every record is there). Errors stay
|
||||
readable in the same result window. If another dialog is open, the result waits until that dialog closes. The result
|
||||
is also available through `library.importLightroom {"status":true}` during that session.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user