From 3aa26d8273846608783b100c1bc305a72e0332d7 Mon Sep 17 00:00:00 2001 From: Clyde C Brown Date: Wed, 21 Oct 2026 19:20:28 -0600 Subject: [PATCH] #2505: Edit Capture Time leaves a capture time it can't fully read as it was shift_iso writes back only a time it fully reads (a date alone, or HH:MM with optional seconds and fraction, an optional zone, nothing after); iso_seconds stays lenient for sorting and rules. photo.setCaptureTime leaves such photos as they were and counts them as `unreadable`, and a set that would move other photos by an unreadable time is refused; alone, the photo gets the typed time. The dialog's preview says the photo's time can't be read, OK says what was left, any error keeps the dialog open (button and ui.dialog.confirm), and Set mode doesn't start from an unreadable time. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/dates.rs | 111 ++++++++++++++- crates/engine/src/cmd/manage.rs | 25 +++- crates/engine/src/tests_library.rs | 55 +++++++ crates/ui-egui/src/control.rs | 1 + crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/menus.rs | 10 +- crates/ui-egui/src/panels/dialogs.rs | 19 ++- crates/ui-egui/src/tests_capture_time.rs | 174 +++++++++++++++++++++++ docs/parity-checklist.md | 5 +- docs/whats-new.md | 10 ++ 10 files changed, 398 insertions(+), 14 deletions(-) create mode 100644 crates/ui-egui/src/tests_capture_time.rs diff --git a/crates/catalog/src/dates.rs b/crates/catalog/src/dates.rs index ca463dc55..122dd7067 100644 --- a/crates/catalog/src/dates.rs +++ b/crates/catalog/src/dates.rs @@ -42,19 +42,63 @@ pub fn weekday(iso: &str) -> Option<&'static str> { /// Shift an ISO time by `secs`, keeping anything after the seconds (fractions, zone) as it was. /// A date without a time gets one. `None` if it doesn't parse, or if the result leaves years /// 0000–9999 (an ISO date here has four year digits, so it couldn't be read back); `secs` may be -/// anything, it never overflows. +/// anything, it never overflows. Stricter than [`iso_seconds`], which reads what it can for sorting: +/// a time cut short, seconds that aren't digits or anything after the zone is `None` here, not +/// written back as a clean-looking time. pub fn shift_iso(iso: &str, secs: i64) -> Option { + let iso = iso.trim(); + let tail = iso.get(tail_start(iso)?..)?; let t = iso_seconds(iso)?.checked_add(secs)?; let (first, last) = (iso_seconds("0000-01-01T00:00:00")?, iso_seconds("9999-12-31T23:59:59")?); if !(first..=last).contains(&t) { return None; } - let iso = iso.trim(); - // the seconds are optional (`10:00+02:00`): without them the tail starts right after the minutes - let tail = iso.get(if iso.get(16..17) == Some(":") { 19 } else { 16 }..).unwrap_or(""); Some(format!("{}{tail}", civil(t))) } +/// Where the part of an ISO time that [`shift_iso`] keeps as it was (fraction, zone) starts, or +/// `None` unless `iso` is a date alone (`2026-01-01`) or a date and `HH:MM`, optional `:SS` with +/// an optional fraction (`.5`), and an optional zone (`Z`, `+02:00`, `-0500`, `+02`), and nothing after. +fn tail_start(iso: &str) -> Option { + let b = iso.as_bytes(); + let digits = |x: &[u8]| !x.is_empty() && x.iter().all(u8::is_ascii_digit); + let digits_at = |a: usize, z: usize| b.get(a..z).is_some_and(digits); + let at = |i: usize, c: u8| b.get(i) == Some(&c); + if !(digits_at(0, 4) && at(4, b'-') && digits_at(5, 7) && at(7, b'-') && digits_at(8, 10)) { + return None; + } + if b.len() == 10 { + return Some(10); + } + if !((at(10, b'T') || at(10, b' ')) && digits_at(11, 13) && at(13, b':') && digits_at(14, 16)) { + return None; + } + // the seconds are optional (`10:00+02:00`): without them the tail starts right after the minutes + let start = if !at(16, b':') { + 16 + } else if digits_at(17, 19) { + 19 + } else { + return None; + }; + let mut zone = b.get(start..)?; + // a fraction only after the seconds: `10:00.5` is half a minute, not half a second + if let ([b'.', rest @ ..], 19) = (zone, start) { + let n = rest.iter().take_while(|c| c.is_ascii_digit()).count(); + zone = rest.get(n..)?; + if n == 0 { + return None; + } + } + let ok = match zone { + [] | [b'Z'] => true, + [b'+' | b'-', h1, h2, b':', m1, m2] => digits(&[*h1, *h2, *m1, *m2]), + [b'+' | b'-', hm @ ..] => matches!(hm.len(), 2 | 4) && digits(hm), + _ => false, + }; + ok.then_some(start) +} + /// Normalize user input to `YYYY-MM-DDTHH:MM:SS` (accepts a space instead of `T`, missing /// seconds or time). `None` if it isn't a valid date. pub fn normalize_iso(s: &str) -> Option { @@ -233,6 +277,65 @@ mod tests { assert_eq!(iso_seconds("2026-01-01T10:00+02:00"), iso_seconds("2026-01-01T10:00")); } + #[test] + fn shift_refuses_a_time_it_cant_fully_read() { + for bad in [ + "2026-01-01T10:0", + "2026-01-01T10", + "2026-01-01T", + "2026-01-01T10:00:xx", + "2026-01-01T10:00:0", + "2026-01-01T10:00:00+", + "2026-01-01T10:00:00+2", + "2026-01-01T10:00:00+02:0", + "2026-01-01T10:00:00.", + "2026-01-01T10:00:00.5.5", + "2026-01-01T10:00.5", + "2026-01-01T10:00:00ZZ", + "2026-01-01T10:00:00 and more", + "2026-01-01T10:00 and more", + "2026-01-01x10:00:00", + "2026/01/01T10:00:00", + ] { + assert_eq!(shift_iso(bad, 3600), None, "{bad} doesn't fully read, so it must not be written back"); + } + // multi-byte characters where the time, the seconds and the zone start never panic + for at in [11, 16, 18, 21] { + let mut s = String::from("2026-01-01T10:00:00+02:00"); + s.replace_range(at..at + 1, "é"); + assert_eq!(shift_iso(&s, 3600), None, "{s}"); + assert_eq!(shift_iso(s.get(..at + 2).unwrap(), 3600), None, "{s}"); + } + } + + #[test] + fn shift_keeps_a_fraction_and_zone_and_reads_back() { + for (from, to) in [ + ("2026-01-01", "2026-01-01T01:00:00"), + (" 2026-01-01T10:00 ", "2026-01-01T11:00:00"), + ("2026-01-01 10:00:30", "2026-01-01T11:00:30"), + ("2026-01-01T10:00:30.5", "2026-01-01T11:00:30.5"), + ("2026-01-01T10:00:30Z", "2026-01-01T11:00:30Z"), + ("2026-01-01T10:00:30+02:00", "2026-01-01T11:00:30+02:00"), + ("2026-01-01T10:00:30-0500", "2026-01-01T11:00:30-0500"), + ("2026-01-01T10:00:30+02", "2026-01-01T11:00:30+02"), + ("2026-01-01T10:00:30.125+02:00", "2026-01-01T11:00:30.125+02:00"), + ] { + assert_eq!(shift_iso(from, 3600).as_deref(), Some(to), "{from}"); + assert_eq!(shift_iso(to, 0).as_deref(), Some(to), "{to} reads back"); + } + } + + /// Sorting, grouping and date rules read with `iso_seconds`, which stays lenient: only the + /// write path refuses what it can't fully read. + #[test] + fn reading_stays_lenient_for_sorting() { + let at_ten = iso_seconds("2026-01-01T10:00:00"); + assert_eq!(iso_seconds("2026-01-01T10:00:00 and more"), at_ten); + assert_eq!(iso_seconds("2026-01-01T10:00:xx"), at_ten); + assert_eq!(iso_seconds("2026-01-01T10:0"), iso_seconds("2026-01-01")); + } + #[test] fn impossible_times_are_rejected_not_rolled_over() { for bad in ["2026-04-01T10:75:00", "2026-04-01 10:00:75", "2026-04-01T10:60", "2026-04-01T24:00:00", "2026-04-01T10:59:60"] { diff --git a/crates/engine/src/cmd/manage.rs b/crates/engine/src/cmd/manage.rs index 339b20b90..9b1f7e9f7 100644 --- a/crates/engine/src/cmd/manage.rs +++ b/crates/engine/src/cmd/manage.rs @@ -183,7 +183,7 @@ pub fn specs() -> Vec { "Edit Capture Time", [], None, - "{ids?, time?: `2026-09-30T14:05:00` (the active photo gets it, the others shift by the same amount), each?: bool (every photo gets `time`), shift?: seconds, hours?: time-zone shift in hours} → {changed, captured: [..]}", + "{ids?, time?: `2026-09-30T14:05:00` (the active photo gets it, the others shift by the same amount), each?: bool (every photo gets `time`), shift?: seconds, hours?: time-zone shift in hours} → {changed, captured: [..], unreadable: photos skipped because their capture time can't be read}", has_selection, |s, p| { use lightcraft_catalog::dates::{iso_seconds, normalize_iso, shift_iso}; @@ -213,12 +213,25 @@ pub fn specs() -> Vec { let from = normalize_iso(anchor_date.trim().get(..10).unwrap_or("")) .and_then(|_| iso_seconds(&anchor_date)) .ok_or_else(|| bad(c, "the photo's date doesn't parse"))?; - delta += iso_seconds(&t).unwrap_or(from) - from; + // the others move by the anchor's change, so it must read in full, not just + // its leading digits (a Lightroom catalog's capture time is copied verbatim); + // alone, it simply gets the time, which repairs it + if shift_iso(&anchor_date, 0).is_some() { + delta += iso_seconds(&t).unwrap_or(from) - from; + } else if targets.len() == 1 { + each = Some(t); + } else { + return Err(bad( + c, + "the active photo's capture time can't be read, so the other photos can't move by the same amount; set its time with it selected alone", + )); + } } } let mut ops = Vec::new(); let mut out = Vec::new(); let mut out_of_range = 0usize; + let mut unreadable = 0usize; for id in &targets { let from = match &each { Some(t) => t.clone(), @@ -226,9 +239,11 @@ pub fn specs() -> Vec { }; let Some(new) = shift_iso(&from, delta) else { // a date that reads but would leave years 0000–9999 is refused below; one - // that doesn't read is skipped, as before - if iso_seconds(&from).is_some() { + // that doesn't read is skipped and counted + if shift_iso(&from, 0).is_some() { out_of_range += 1; + } else { + unreadable += 1; } continue; }; @@ -244,7 +259,7 @@ pub fn specs() -> Vec { if n > 0 { s.commit("Edit Capture Time", Op::Batch { ops })?; } - Ok(json!({"changed": n, "captured": out})) + Ok(json!({"changed": n, "captured": out, "unreadable": unreadable})) } ), cmd!( diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 5460dc020..82bdca457 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -514,6 +514,61 @@ fn capture_time_shift_is_bounded() { assert_eq!(s.catalog.photo(id).unwrap().captured.as_deref(), Some("9999-12-31T12:00:00"), "unchanged"); } +/// A capture time that doesn't fully read (a Lightroom catalog's `captureTime` is copied +/// verbatim) is left as it is and counted, not shifted into a clean-looking wrong time, and it +/// isn't mistaken for one that would leave years 0000–9999. +#[test] +fn capture_time_shift_counts_photos_whose_time_does_not_read() { + let mut s = crate::Session::with_demo(); + let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect(); + let (good, bad) = (ids[0], ids[1]); + for (id, t) in [(good, "2026-01-01T10:00:00"), (bad, "2026-01-01T10:00:00 and more")] { + s.catalog.apply(lightcraft_catalog::Op::SetCaptured { id, captured: Some(t.into()) }).unwrap(); + } + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [good.0, bad.0], "hours": 1})).unwrap(); + assert_eq!(r["unreadable"], 1, "the unreadable photo is counted: {r}"); + assert_eq!(r["changed"], 1, "{r}"); + let captured = |s: &crate::Session, id| s.catalog.photo(id).unwrap().captured.clone(); + assert_eq!(captured(&s, good).as_deref(), Some("2026-01-01T11:00:00")); + assert_eq!(captured(&s, bad).as_deref(), Some("2026-01-01T10:00:00 and more"), "left as it was"); + s.execute("edit.undo", &serde_json::json!({})).unwrap(); + assert_eq!(captured(&s, good).as_deref(), Some("2026-01-01T10:00:00"), "one undo step"); + // a shift past year 9999 for the readable photo is still the out-of-range refusal, and the + // unreadable one alone isn't + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [bad.0], "hours": 24 * 365 * 9000})).unwrap(); + assert_eq!((r["changed"].as_u64(), r["unreadable"].as_u64()), (Some(0), Some(1)), "{r}"); + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [good.0, bad.0], "hours": 24 * 365 * 9000})); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("1 photo(s) outside")), "{r:?}"); +} + +/// Setting the active photo's time moves the others by its change, so an active photo whose own +/// time doesn't read can't anchor it: refused, nothing changes. Alone, or setting every photo's +/// time, it gets the time, which repairs it. +#[test] +fn capture_time_set_refuses_an_active_photo_whose_time_does_not_read() { + let mut s = crate::Session::with_demo(); + let ids: Vec<_> = s.catalog.photos().take(2).map(|p| p.id).collect(); + let (bad, other) = (ids[0], ids[1]); + let broken = "2026-01-01T10:00:00 and more"; + for (id, t) in [(bad, broken), (other, "2026-01-01T12:00:00")] { + s.catalog.apply(lightcraft_catalog::Op::SetCaptured { id, captured: Some(t.into()) }).unwrap(); + } + s.execute("library.select", &serde_json::json!({"ids": [bad.0, other.0], "active": bad.0})).unwrap(); + assert_eq!(s.active(), Some(bad)); + let captured = |s: &crate::Session, id| s.catalog.photo(id).unwrap().captured.clone(); + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [bad.0, other.0], "time": "2026-01-01T11:00:00"})); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("can't be read")), "{r:?}"); + assert_eq!(captured(&s, bad).as_deref(), Some(broken), "refused: unchanged"); + assert_eq!(captured(&s, other).as_deref(), Some("2026-01-01T12:00:00"), "refused: the others don't move"); + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [bad.0], "time": "2026-01-01T11:00:00"})).unwrap(); + assert_eq!((r["changed"].as_u64(), r["unreadable"].as_u64()), (Some(1), Some(0)), "{r}"); + assert_eq!(captured(&s, bad).as_deref(), Some("2026-01-01T11:00:00"), "alone, it gets the time"); + s.catalog.apply(lightcraft_catalog::Op::SetCaptured { id: bad, captured: Some(broken.into()) }).unwrap(); + let r = s.execute("photo.setCaptureTime", &serde_json::json!({"ids": [bad.0, other.0], "time": "2026-01-01T11:00:00", "each": true})).unwrap(); + assert_eq!((r["changed"].as_u64(), r["unreadable"].as_u64()), (Some(2), Some(0)), "{r}"); + assert_eq!(captured(&s, bad).as_deref(), Some("2026-01-01T11:00:00"), "each repairs it"); +} + /// Values are checked too: an agent's rule set with a date that isn't one, or a rating of 9, is /// refused with the rule's position, by album.createSmart and library.filter alike. #[test] diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 953874f6a..b1a2e90f2 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -392,6 +392,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest | crate::state::Dialog::SamModel { .. } | crate::state::Dialog::SmartRules { .. } | crate::state::Dialog::SaveEffect { .. } + | crate::state::Dialog::CaptureTime { .. } )) || (r.is_ok() && crate::panels::dialogs::keeps_open(app, &d)) { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index a08fb6151..d609b917c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -65,6 +65,8 @@ mod tests_button; #[cfg(test)] mod tests_canvas_keys; #[cfg(test)] +mod tests_capture_time; +#[cfg(test)] mod tests_compare; #[cfg(test)] mod tests_copy_paste_dialogs; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 3e8c3653b..63f7ca07b 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1319,7 +1319,15 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { - let time = app.session.active().and_then(|id| app.session.catalog.photo(id)).map(|p| p.date().replace('T', " ")).unwrap_or_default(); + // a time that doesn't fully read isn't offered as the starting point (`10:0` would read as midnight) + let time = app + .session + .active() + .and_then(|id| app.session.catalog.photo(id)) + .map(|p| p.date()) + .filter(|d| lightcraft_catalog::dates::shift_iso(d, 0).is_some()) + .map(|d| d.replace('T', " ")) + .unwrap_or_default(); let mode = p.get("mode").and_then(Value::as_str).unwrap_or("set").to_string(); app.ui.dialog = Some(Dialog::CaptureTime { mode, time: time.get(..19).unwrap_or(&time).to_string(), days: 0, hours: 0, minutes: 0, zone: 0.0 }); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 5562e94d1..5bf784ca6 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -522,10 +522,18 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { "shift" => params["shift"].as_i64(), _ => Some((*zone as f64 * 3600.0).round() as i64), }; - let after = delta.and_then(|d| lightcraft_catalog::dates::shift_iso(&cur, d)); + // the photo's own time is the problem (a Lightroom catalog's capture time + // is copied verbatim), not what was typed + let unreadable = lightcraft_catalog::dates::shift_iso(&cur, 0).is_none(); + let after = match (mode.as_str(), unreadable && n == 1) { + // alone, it simply gets the time typed (photo.setCaptureTime repairs it) + ("set", true) => lightcraft_catalog::dates::normalize_iso(time), + _ => delta.and_then(|d| lightcraft_catalog::dates::shift_iso(&cur, d)), + }; ui.label( egui::RichText::new(match after { Some(a) => format!("{name}: {} → {}", cur.replace('T', " "), a.replace('T', " ")), + None if unreadable && !(mode == "set" && n == 1) => crate::i18n::tr("The active photo's capture time can't be read").to_string(), None => "Enter a date as YYYY-MM-DD HH:MM:SS".into(), }) .color(t.text_label), @@ -1454,7 +1462,8 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { if confirm { match confirm_dialog(app, &dlg) { // the import review stays open on an error (e.g. an unusable folder template), and so - // does Export (e.g. no folder chosen) so the choices aren't lost + // does Export (e.g. no folder chosen) so the choices aren't lost, and Edit Capture Time + // (a time that isn't a date, or a shift it refuses) Err(e) if matches!( dlg, @@ -1470,6 +1479,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { | Dialog::KeywordSet { .. } | Dialog::SynchronizeFolder { .. } | Dialog::SaveEffect { .. } + | Dialog::CaptureTime { .. } ) => { app.toast(ctx, e) @@ -1497,6 +1507,11 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { app.toast(ctx, e); close = true; } + // photos whose capture time doesn't read are left as they were: say so + Ok(r) if matches!(dlg, Dialog::CaptureTime { .. }) && r["unreadable"].as_u64().is_some_and(|n| n > 0) => { + app.toast(ctx, crate::i18n::tr("Photos whose capture time can't be read were left as they were")); + close = true; + } Ok(_) if keeps_open(app, &dlg) => { if let Dialog::SamModel { error, .. } = &mut dlg { *error = None; diff --git a/crates/ui-egui/src/tests_capture_time.rs b/crates/ui-egui/src/tests_capture_time.rs new file mode 100644 index 000000000..b5903e053 --- /dev/null +++ b/crates/ui-egui/src/tests_capture_time.rs @@ -0,0 +1,174 @@ +//! Edit Capture Time on a photo whose own capture time doesn't fully read (a Lightroom catalog's +//! `captureTime` is copied verbatim): the preview says so instead of asking for a date the user +//! never typed, and OK leaves the photo as it was and says why. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::Dialog; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(60); + +fn set_captured(h: &mut Headless, id: lightcraft_catalog::PhotoId, t: &str) { + h.app.session.catalog.apply(lightcraft_catalog::Op::SetCaptured { id, captured: Some(t.into()) }).unwrap(); +} + +fn painted(h: &Headless, part: &str) -> bool { + h.painted_text().iter().any(|t| t.contains(part)) +} + +fn toast(h: &Headless) -> String { + h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default() +} + +/// Select `ids` with `active` and open Edit Capture Time. +fn open(h: &mut Headless, ids: &[lightcraft_catalog::PhotoId], active: lightcraft_catalog::PhotoId) { + let ids: Vec = ids.iter().map(|id| id.0).collect(); + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": ids, "active": active.0}}), T); + let r = h.request("ui.menu.invoke", json!({"id": "dialog.captureTime"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); +} + +#[test] +fn shift_preview_says_the_photos_capture_time_cant_be_read() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let id = h.app.session.visible_cloned()[0]; + set_captured(&mut h, id, "2026-01-01T10:00:00"); + open(&mut h, &[id], id); + match &mut h.app.ui.dialog { + Some(Dialog::CaptureTime { mode, hours, .. }) => { + *mode = "shift".into(); + *hours = 1; + } + other => panic!("Edit Capture Time is not open: {other:?}"), + } + h.settle(SETTLE); + assert!(painted(&h, "2026-01-01 10:00:00 → 2026-01-01 11:00:00"), "a readable time previews its shift: {:?}", h.painted_text()); + + let bad = "2026-01-01T10:00:00 and more"; + set_captured(&mut h, id, bad); + h.settle(SETTLE); + assert!(painted(&h, "can't be read"), "the preview says the photo's time can't be read: {:?}", h.painted_text()); + assert!(!painted(&h, "Enter a date"), "no date was typed, so the preview doesn't ask for one: {:?}", h.painted_text()); + + let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.app.ui.dialog.is_none(), "OK closes the dialog"); + let toast = toast(&h); + assert!(toast.contains("can't be read"), "OK says the photo was left as it was: {toast:?}"); + assert_eq!(h.app.session.catalog.photo(id).unwrap().captured.as_deref(), Some(bad), "the unreadable time is left as it was"); +} + +#[test] +fn set_mode_refuses_moving_others_by_an_unreadable_time_and_repairs_it_alone() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let ids = h.app.session.visible_cloned(); + let (bad, other) = (ids[0], ids[1]); + let broken = "2026-01-01T10:00:00 and more"; + set_captured(&mut h, bad, broken); + set_captured(&mut h, other, "2026-01-01T12:00:00"); + let open_set = |h: &mut Headless, select: &[lightcraft_catalog::PhotoId]| { + open(h, select, bad); + match &mut h.app.ui.dialog { + Some(Dialog::CaptureTime { mode, time, .. }) => { + *mode = "set".into(); + *time = "2026-01-01 11:00:00".into(); + } + other => panic!("Edit Capture Time is not open: {other:?}"), + } + h.settle(SETTLE); + }; + + // with another photo selected, OK can't move it by the unreadable time's change: it says so + // and stays open + open_set(&mut h, &[bad, other]); + let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let toast = toast(&h); + assert!(toast.contains("can't be read"), "OK says why nothing changed: {toast:?}"); + assert!(matches!(h.app.ui.dialog, Some(Dialog::CaptureTime { .. })), "the dialog stays open after the refusal"); + let captured = |h: &Headless, id| h.app.session.catalog.photo(id).unwrap().captured.clone(); + assert_eq!(captured(&h, bad).as_deref(), Some(broken)); + assert_eq!(captured(&h, other).as_deref(), Some("2026-01-01T12:00:00"), "the other photo doesn't move"); + h.request("ui.dialog.cancel", json!({}), T); + + // alone, the preview shows the time it will get, and OK sets it + open_set(&mut h, &[bad]); + let preview = h.painted_text().into_iter().find(|t| t.contains(" → ")).unwrap_or_default(); + assert!(preview.ends_with(" → 2026-01-01 11:00:00"), "the preview shows the time it will get, nothing after: {preview:?}"); + let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(h.app.ui.dialog.is_none(), "OK closes the dialog"); + assert_eq!(captured(&h, bad).as_deref(), Some("2026-01-01T11:00:00"), "alone, it gets the time typed"); +} + +/// Errors from the command say why and keep the dialog (and what was typed) open, from its OK +/// button and from the control channel alike; they used to close it without a word. +#[test] +fn errors_are_shown_and_the_dialog_stays_open() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let id = h.app.session.visible_cloned()[0]; + set_captured(&mut h, id, "9999-12-31T12:00:00"); + open(&mut h, &[id], id); + match &mut h.app.ui.dialog { + Some(Dialog::CaptureTime { time, .. }) => *time = "yesterday".into(), + other => panic!("Edit Capture Time is not open: {other:?}"), + } + h.settle(SETTLE); + assert_eq!(h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T)["ok"], true); + h.settle(SETTLE); + assert!(toast(&h).contains("not a date"), "OK says the typed time isn't a date: {:?}", toast(&h)); + assert!(matches!(&h.app.ui.dialog, Some(Dialog::CaptureTime { time, .. }) if time == "yesterday"), "the dialog stays open with what was typed"); + + // a shift past year 9999, confirmed through the control channel + match &mut h.app.ui.dialog { + Some(Dialog::CaptureTime { mode, days, .. }) => { + *mode = "shift".into(); + *days = 1; + } + other => panic!("Edit Capture Time is not open: {other:?}"), + } + h.settle(SETTLE); + let r = h.request("ui.dialog.confirm", json!({}), T); + assert_ne!(r["ok"], true, "the shift is refused: {r}"); + assert!(r.to_string().contains("9999"), "{r}"); + assert!(matches!(h.app.ui.dialog, Some(Dialog::CaptureTime { .. })), "ui.dialog.confirm keeps it open too, as the button does"); + assert_eq!(h.app.session.catalog.photo(id).unwrap().captured.as_deref(), Some("9999-12-31T12:00:00")); +} + +/// A capture time that doesn't fully read isn't offered as the time to set: `2026-01-01T10:0` +/// would read as midnight. The field starts empty and the preview asks for a date. +#[test] +fn set_mode_does_not_start_from_an_unreadable_time() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1300.0, 900.0], 1.0); + let id = h.app.session.visible_cloned()[0]; + set_captured(&mut h, id, "2026-01-01T10:00:00"); + open(&mut h, &[id], id); + assert!( + matches!(&h.app.ui.dialog, Some(Dialog::CaptureTime { time, .. }) if time == "2026-01-01 10:00:00"), + "a readable time is offered: {:?}", + h.app.ui.dialog + ); + h.request("ui.dialog.cancel", json!({}), T); + + set_captured(&mut h, id, "2026-01-01T10:0"); + open(&mut h, &[id], id); + assert!( + matches!(&h.app.ui.dialog, Some(Dialog::CaptureTime { time, .. }) if time.is_empty()), + "an unreadable time isn't offered: {:?}", + h.app.ui.dialog + ); + assert!(painted(&h, "Enter a date"), "alone, the photo will get the time typed, so the preview asks for one: {:?}", h.painted_text()); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 02f5bf08f..6baa5e6c0 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-20 · **Change:** minor (LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** patch (LR-LIB-CAPTURETIME: a capture time that doesn't fully read is left as it is, not shifted) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -113,7 +113,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos`, `crates/engine/src/tests_album_remove.rs`, `crates/ui-egui/src/tests_album_remove.rs` | changes the album only: the photos stay in the library, their library folder and their folder on disk (issue #1805); `id` defaults to the album on screen; returns `{removed, album}`. In a normal album ⌫ removes from the album (as in Lightroom; no confirmation, one undo step), in a smart album it changes nothing and says to edit its rules | | LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) | | LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow | -| LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time”. Impossible calendar dates cannot anchor relative shifts; rejection preserves metadata, selection, Undo and Redo. | +| LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time”. Impossible calendar dates cannot anchor relative shifts; rejection preserves metadata, selection, Undo and Redo. A capture time that doesn't fully read (cut short, or text after it) is left as it is and counted (`unreadable`); it can't anchor a set that moves other photos, and set alone repairs it. | | LR-LIB-SHOWFINDER | Reveal original in file manager | P0 | ✅ | `cmd:app.showInFinder` | ⌘R (see MENU-FILE-SHOWFINDER) | | LR-LIB-COVER | Album cover | P2 | ✅ | `cmd:album.setCover` | | | LR-LIB-CULL | Assisted culling | P2 | 🟡 | `cmd:photo.analyze`, `cmd:dialog.cull`, `crates/pipeline/src/cull.rs` | Photo ▸ Assisted Culling…: focus score (0–100), clipping, burst grouping (look-alike shots within 10 s) with the sharpest marked; reject below a focus score, pick each burst's best; classical measures — no eyes-closed / expression detection | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-21 | patch | LR-LIB-CAPTURETIME: a capture time that doesn't fully read is left as it is and counted (`unreadable`) instead of shifted into a wrong one; it can't anchor a set that moves other photos; status unchanged | | 2026-10-20 | patch | LR-VIEW-ZOOM: the 20 ms rule clears a slow-drag verdict only on a drag timed at full size; a drag that drafts small leaves it unchanged, as the code does; status unchanged | | 2026-10-19 | patch | LR-VIEW-ZOOM: the slow slider-drag verdict is described as per photo and control, as the code keys it; status unchanged | | 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cfeed33ed..46dff342a 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,15 @@ # What's new in LightCraft +## 2026-10-21 + +### Edit Capture Time leaves a capture time it can't read as it was +- A capture time that is cut short, has seconds that aren't digits or has text after it (a Lightroom catalog's capture + time is copied as it is) is no longer shifted into a clean-looking wrong time: Edit Capture Time leaves the photo as + it was and says so, and its preview says the photo's time can't be read instead of previewing a wrong one. + `photo.setCaptureTime` counts those photos as `unreadable`. Set date & time still repairs such a photo selected on + its own; with other photos selected it says why it can't move them by the same amount. When the dialog can't make a + change (a time that isn't a date, a shift past year 9999), it now says why and stays open, instead of closing. + ## 2026-10-19 ### Hidden Local locations come back one at a time