aleleba/lightcraft
1
0
Fork 0
mirror of https://github.com/storytold/lightcraft.git synced 2026-10-26 19:17:02 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge PR #1773

Browse Source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-15 00:31:26 -04:00
co-authored by Claude Opus 5.5
parent 211255770f 9ef40b9886
commit 2475cf5e32
30 changed files with 134 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/browse.rs
+2 -2
View File
@@ -224,7 +224,7 @@ fn forget_local(s: &mut Session, p: &Value) -> Result<Value> {
/// The browsed photos among `ids` (default: the selection) become library photos.
fn add_to_library(s: &mut Session, p: &Value) -> Result<Value> {
let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|id| s.catalog.photo(*id).is_some_and(|ph| ph.local)).collect();
let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|id| s.catalog.photo(*id).is_some_and(|ph| ph.local)).collect();
if ids.is_empty() {
return Ok(json!({"added": 0}));
}
@@ -410,7 +410,7 @@ fn move_to_folder(s: &mut Session, p: &Value) -> Result<Value> {
if !dir.is_dir() {
return Err(bad(C, format!("{folder} is not a folder (create it first with folder.create)")));
}
let ids = s.targets(p);
let ids = s.targets(p)?;
if ids.is_empty() {
return Err(bad(C, "no photos to move: select some or give ids"));
}
crates/engine/src/cmd/convert.rs
+4 -4
View File
@@ -59,7 +59,7 @@ pub(crate) fn write_dng_with(file_bytes: Option<&crate::merge::ByteReader>, path
fn convert(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "photo.convertToDng";
let ids = s.targets(p);
let ids = s.targets(p)?;
let mut ops = Vec::new();
let mut converted = Vec::new();
let mut skipped = Vec::new();
@@ -125,7 +125,7 @@ pub fn specs() -> Vec<CommandSpec> {
/// the library and stack it on top of the original. The host opens the file in the editor.
fn edit_external(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "photo.editExternal";
let mut ids = s.targets(p);
let mut ids = s.targets(p)?;
ids.dedup();
if ids.is_empty() {
return Err(bad(C, "no active photo"));
@@ -311,7 +311,7 @@ pub(crate) fn reload_ops(
/// Re-read photos whose files changed on disk (an external editor saved them): new size,
/// dimensions and content hash, cached sources dropped. → {reloaded: [ids]}
fn reload(s: &mut Session, p: &Value) -> Result<Value> {
let ids = s.targets(p);
let ids = s.targets(p)?;
let paths: Vec<(lightcraft_catalog::PhotoId, String)> = ids
.iter()
.filter_map(|id| match s.catalog.photo(*id).map(|ph| ph.source.clone()) {
@@ -340,7 +340,7 @@ fn reload(s: &mut Session, p: &Value) -> Result<Value> {
fn duplicate(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "photo.duplicate";
let mut made = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
let Some(ph) = s.catalog.photo(id).map(|p| (**p).clone()) else { continue };
let Source::File { path } = &ph.source else { return Err(bad(C, "a generated demo photo has no file to duplicate")) };
let src = Path::new(path);
crates/engine/src/cmd/cull.rs
+1 -1
View File
@@ -20,7 +20,7 @@ fn secs(iso: &str) -> Option<i64> {
}
fn analyze(s: &mut Session, p: &Value) -> Result<Value> {
let ids: Vec<PhotoId> = if p.get("ids").is_some() || s.selection.ids.len() > 1 { s.targets(p) } else { s.visible_cloned() };
let ids: Vec<PhotoId> = if p.get("ids").is_some() || s.selection.ids.len() > 1 { s.targets(p)? } else { s.visible_cloned() };
let reject_below = p.get("rejectBelow").and_then(Value::as_f64).map(|v| v as f32);
let pick_best = p.get("pickBest").and_then(Value::as_bool).unwrap_or(false);
// measure (thumbnail-level sources: fast, and enough for focus at 512 px)
crates/engine/src/cmd/develop.rs
+8 -7
View File
@@ -254,7 +254,7 @@ pub fn specs() -> Vec<CommandSpec> {
})
}),
cmd!("develop.reset", "Reset Edits", ["Photo"], Some("Cmd+Shift+R"), "{ids?}", has_selection, |s, p| {
let ids = s.targets(p);
let ids = s.targets(p)?;
let ops = ids
.iter()
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -304,7 +304,7 @@ pub fn specs() -> Vec<CommandSpec> {
let targets = match named_photos(s, p, C)? {
Some(ids) if !ids.iter().any(|id| outdated(s, *id)) => return Err(crate::EngineError::Disabled(C.into(), already_current())),
Some(ids) => ids,
None => s.targets(p),
None => s.targets(p)?,
};
let ops: Vec<Op> = targets
.into_iter()
@@ -811,7 +811,7 @@ pub fn specs() -> Vec<CommandSpec> {
s.previous_active.filter(|id| s.catalog.photo(*id).is_some_and(|ph| !ph.deleted)).ok_or_else(|| bad(c, "no previous photo"))?;
let groups = groups_param(p, c)?.unwrap_or_else(|| s.copy_groups.clone());
let partial = lightcraft_develop::extract_groups(&s.develop_of(prev).unwrap_or_default(), &groups);
let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|id| *id != prev).collect();
let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|id| *id != prev).collect();
let ops = ids
.iter()
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -838,7 +838,7 @@ pub fn specs() -> Vec<CommandSpec> {
let full = lightcraft_develop::apply_partial(&DevelopSettings::default(), &clip, 1.0);
clip = lightcraft_develop::extract_groups(&full, &only);
}
let ids = s.targets(p);
let ids = s.targets(p)?;
let ops = ids
.iter()
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
@@ -890,7 +890,7 @@ pub fn specs() -> Vec<CommandSpec> {
let delta = f64_req(p, "delta", c)?;
let label = format!("Quick Develop: {}", spec.label);
let ops: Vec<Op> = s
.targets(p)
.targets(p)?
.into_iter()
.filter_map(|id| s.develop_of(id).map(|d| (id, d)))
.filter_map(|(id, d)| {
@@ -943,7 +943,7 @@ pub fn specs() -> Vec<CommandSpec> {
};
let r_ev = gathered(s, reference).ok_or_else(|| bad(c, "the active photo has no shutter / aperture / ISO"))?;
let r_slider = s.develop_of(reference).map(|d| d.light.exposure).unwrap_or(0.0);
let ids: Vec<PhotoId> = s.targets(p).into_iter().filter(|x| *x != reference).collect();
let ids: Vec<PhotoId> = s.targets(p)?.into_iter().filter(|x| *x != reference).collect();
let (mut ops, mut skipped) = (Vec::new(), Vec::new());
for id in ids {
let (Some(ev), Some(d)) = (gathered(s, id), s.develop_of(id)) else {
@@ -978,7 +978,8 @@ pub fn specs() -> Vec<CommandSpec> {
let pid = str_param(p, "id").ok_or_else(|| bad("preset.apply", "missing id"))?;
let preset = s.presets.iter().find(|x| x.id == pid).cloned().ok_or_else(|| bad("preset.apply", format!("unknown preset `{pid}`")))?;
let amount = f64_or(p, "amount", 100.0).clamp(0.0, 200.0) / 100.0;
let ids = s.targets(p);
// `id` names the preset here: the photos are `ids`, else the selection
let ids = s.targets(&json!({"ids": p.get("ids")}))?;
let ops = ids
.iter()
.filter_map(|id| s.develop_of(*id).map(|d| (*id, d)))
crates/engine/src/cmd/face_detect.rs
+1 -1
View File
@@ -132,7 +132,7 @@ fn detect(s: &mut Session, p: &Value) -> Result<Value> {
let detector = detector(s).map_err(|e| e.into_error(C))?;
let started = Instant::now();
let (mut results, mut ops) = (Vec::new(), Vec::new());
for id in s.targets(p) {
for id in s.targets(p)? {
let (rgb, width, height) = match render_rgb(s, id, LOOK_EDGE) {
Ok(r) => r,
Err(e) => {
crates/engine/src/cmd/face_recognize.rs
+1 -1
View File
@@ -841,7 +841,7 @@ mod imp {
const C: &str = "faces.suggest";
let embedder = current(s, C)?;
collect(s);
let targets = s.targets(p);
let targets = s.targets(p)?;
let model_threshold = embedder.manifest().thresholds.match_cosine;
let num = |k: &str| p.get(k).and_then(Value::as_f64).filter(|v| v.is_finite()).map(|v| v as f32);
let threshold = num("threshold").or(model_threshold).unwrap_or(DEFAULT_THRESHOLD);
crates/engine/src/cmd/geocode.rs
+1 -1
View File
@@ -115,7 +115,7 @@ fn fill(meta: &mut lightcraft_catalog::Meta, place: &lightcraft_geocode::Place,
fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "metadata.reverseGeocode";
let overwrite = bool_or(p, "overwrite", false);
let ids = s.targets(p);
let ids = s.targets(p)?;
let todo: Vec<(PhotoId, LatLon)> = ids
.iter()
.filter_map(|id| {
crates/engine/src/cmd/keywords.rs
+3 -3
View File
@@ -256,7 +256,7 @@ pub fn specs() -> Vec<CommandSpec> {
let Some(k) = current_keywords(s).get(i as usize - 1).cloned().filter(|k| !k.is_empty()) else {
return Ok(json!({"changed": 0}));
};
let ids = s.targets(p);
let ids = s.targets(p)?;
let all = !ids.is_empty()
&& ids.iter().all(|id| {
s.catalog
@@ -288,7 +288,7 @@ pub fn specs() -> Vec<CommandSpec> {
always,
|s, p| {
let mut current: Vec<String> = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
if let Some(ph) = s.catalog.photo(id) {
current.extend(ph.meta.keywords.iter().cloned());
}
@@ -336,7 +336,7 @@ pub fn specs() -> Vec<CommandSpec> {
Some(parent) => format!("{parent}|{name}"),
None => name,
});
let photos = if bool_or(p, "addToSelected", false) { s.targets(p) } else { Vec::new() };
let photos = if bool_or(p, "addToSelected", false) { s.targets(p)? } else { Vec::new() };
let op = s
.catalog
.create_keyword_ops(&path, info_from(p, KeywordInfo::default()), &photos)
crates/engine/src/cmd/library.rs
+12 -13
View File
@@ -182,7 +182,7 @@ fn mark_deleted(s: &mut Session, ids: &[PhotoId], label: &str) -> Result<usize>
/// Apply one op per target as a single undo step.
fn for_targets(s: &mut Session, p: &Value, label: &str, f: impl Fn(PhotoId) -> Option<Op>) -> Result<Value> {
let targets = s.targets(p);
let targets = s.targets(p)?;
let ops: Vec<Op> = targets.iter().filter_map(|id| f(*id)).collect();
let n = ops.len();
if n > 0 {
@@ -194,7 +194,7 @@ fn for_targets(s: &mut Session, p: &Value, label: &str, f: impl Fn(PhotoId) -> O
/// Adjust each target's own rating; unchanged bounds do not create an undo step.
fn change_rating(s: &mut Session, p: &Value, delta: i8, label: &str) -> Result<Value> {
let mut ops = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
let old = s.catalog.photo(id).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?.rating;
let rating = (i16::from(old) + i16::from(delta)).clamp(0, 5) as u8;
if rating != old {
@@ -213,7 +213,7 @@ fn change_rating(s: &mut Session, p: &Value, delta: i8, label: &str) -> Result<V
/// raising/lowering clamps on reject < unflagged < pick. Unchanged photos make no history step.
fn change_flag(s: &mut Session, p: &Value, label: &str, step: impl Fn(Flag) -> Flag) -> Result<Value> {
let mut ops = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
let old = s.catalog.photo(id).ok_or(lightcraft_catalog::CatalogError::NoPhoto(id))?.flag;
let flag = step(old);
if flag != old {
@@ -258,7 +258,7 @@ fn row_step(p: &Value) -> isize {
}
fn rotate(s: &mut Session, p: &Value, cw: bool) -> Result<Value> {
let targets = s.targets(p);
let targets = s.targets(p)?;
let mut ops = Vec::new();
for id in targets {
if let Some(d) = s.develop_of(id) {
@@ -277,7 +277,7 @@ fn rotate(s: &mut Session, p: &Value, cw: bool) -> Result<Value> {
}
fn flip(s: &mut Session, p: &Value, horizontal: bool) -> Result<Value> {
let targets = s.targets(p);
let targets = s.targets(p)?;
let mut ops = Vec::new();
for id in targets {
if let Some(d) = s.develop_of(id) {
@@ -883,7 +883,7 @@ pub fn specs() -> Vec<CommandSpec> {
// ---- delete / restore
cmd!("photo.delete", "Delete Photo", ["Photo"], Some("Delete"), "{ids?} — moves to Recently Deleted", has_selection, |s, p| {
let before = s.visible_cloned();
let targets = s.targets(p);
let targets = s.targets(p)?;
let n = mark_deleted(s, &targets, "Delete")?;
select_after_deletion(s, &before);
Ok(json!({"changed": n}))
@@ -912,7 +912,7 @@ pub fn specs() -> Vec<CommandSpec> {
),
cmd!("photo.deletePermanently", "Delete Permanently", ["Photo"], None, "{ids?}", has_selection, |s, p| {
let before = s.visible_cloned();
let t = s.targets(p);
let t = s.targets(p)?;
let op = s.catalog.delete_photos_permanently_ops(&t);
s.commit("Delete Permanently", op)?;
select_after_deletion(s, &before);
@@ -929,7 +929,7 @@ pub fn specs() -> Vec<CommandSpec> {
|s, p| {
let strs =
|k: &str| p.get(k).and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect::<Vec<_>>());
let targets = s.targets(p);
let targets = s.targets(p)?;
// GPS: "lat, lon" (decimal or 51°30'26"N 0°7'39"W), [lat, lon], or null / "" to clear
let gps: Option<Option<(f64, f64)>> = match p.get("gps") {
None => None,
@@ -1027,8 +1027,7 @@ pub fn specs() -> Vec<CommandSpec> {
}
let folder = bool_or(p, "folder", false);
let parent = p.get("parent").and_then(Value::as_u64).map(AlbumId);
let (photos, skipped) =
if !folder && bool_or(p, "addSelected", false) { album_targets(s, s.targets(&Value::Null)) } else { (vec![], 0) };
let (photos, skipped) = if !folder && bool_or(p, "addSelected", false) { album_targets(s, s.selected()) } else { (vec![], 0) };
let id = s.catalog.alloc_album_id();
let cover = photos.first().copied();
s.commit(
@@ -1268,7 +1267,7 @@ pub fn specs() -> Vec<CommandSpec> {
has_library_selection,
|s, p| {
let id = album_param(p, "id", "album.addPhotos")?;
let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null));
let targets = ids_param(p).unwrap_or_else(|| s.selected());
let al = s.catalog.album(id).ok_or_else(|| bad("album.addPhotos", "no such album"))?;
if al.is_smart() || al.folder {
return Err(bad("album.addPhotos", "smart albums and folders can't hold photos"));
@@ -1295,7 +1294,7 @@ pub fn specs() -> Vec<CommandSpec> {
"{ids?} — B: adds the photos to the target album (the Quick Collection unless one is set), or removes them when they're all in it → {album, added, skipped}; Local photos are skipped",
has_library_selection,
|s, p| {
let targets = album_targets_or_err(s, "album.toggleTarget", ids_param(p).unwrap_or_else(|| s.targets(&Value::Null)))?;
let targets = album_targets_or_err(s, "album.toggleTarget", ids_param(p).unwrap_or_else(|| s.selected()))?;
let (targets, skipped) = targets;
let target = s.target_album.filter(|a| s.catalog.album(*a).is_some_and(|al| !al.is_smart() && !al.folder));
let id = match target.or_else(|| s.catalog.quick_collection()) {
@@ -1454,7 +1453,7 @@ pub fn specs() -> Vec<CommandSpec> {
_ => return Err(bad("album.removePhotos", "no album: pass `id` or show an album")),
},
};
let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null));
let targets = ids_param(p).unwrap_or_else(|| s.selected());
let al = s.catalog.album(id).ok_or_else(|| bad("album.removePhotos", "no such album"))?;
if al.is_smart() {
return Err(bad("album.removePhotos", "smart albums update automatically: change their rules"));
crates/engine/src/cmd/manage.rs
+3 -3
View File
@@ -10,7 +10,7 @@ use crate::Result;
fn rename_args(s: &crate::Session, p: &Value, c: &str) -> Result<(Vec<lightcraft_catalog::PhotoId>, String, usize)> {
let template = str_param(p, "template").ok_or_else(|| bad(c, "missing `template`"))?.to_string();
let start = p.get("start").and_then(Value::as_u64).unwrap_or(1) as usize;
Ok((s.targets(p), template, start))
Ok((s.targets(p)?, template, start))
}
/// A named set of colour-label names (red, yellow, green, blue, purple; empty = the colour's name).
@@ -96,7 +96,7 @@ fn auto_tag_tracklog(s: &mut crate::Session, p: &Value) -> Result<Value> {
let (mut no_time, mut outside, mut has_gps, mut assumed_utc, mut interpolated) = (0, 0, 0, 0, 0);
let mut ops = Vec::new();
let mut photos = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
let Some(ph) = s.catalog.photo(id) else { continue };
if ph.meta.gps.is_some() && !replace {
has_gps += 1;
@@ -183,7 +183,7 @@ pub fn specs() -> Vec<CommandSpec> {
|s, p| {
use lightcraft_catalog::dates::{iso_seconds, normalize_iso, shift_iso};
let c = "photo.setCaptureTime";
let targets: Vec<_> = s.targets(p).into_iter().filter(|id| s.catalog.photo(*id).is_some()).collect();
let targets: Vec<_> = s.targets(p)?.into_iter().filter(|id| s.catalog.photo(*id).is_some()).collect();
if targets.is_empty() {
return Err(bad(c, "no photos"));
}
crates/engine/src/cmd/merge.rs
+2 -2
View File
@@ -20,7 +20,7 @@ fn can_merge(s: &Session) -> std::result::Result<(), String> {
fn run(s: &mut Session, id: &str, p: &Value) -> Result<Value> {
let (kind, finish) = crate::merge::parse(id, p)?;
let ids: Vec<PhotoId> = s.targets(p);
let ids: Vec<PhotoId> = s.targets(p)?;
if !bool_or(p, "preview", false) {
return s.merge_now(kind, finish, &ids);
}
@@ -45,7 +45,7 @@ fn run(s: &mut Session, id: &str, p: &Value) -> Result<Value> {
fn run_batch(s: &mut Session, p: &Value) -> Result<Value> {
const ID: &str = "merge.hdrBatch";
let (kind, finish) = crate::merge::parse("merge.hdr", p)?;
let ids: Vec<PhotoId> = s.targets(p);
let ids: Vec<PhotoId> = s.targets(p)?;
let sets = s.catalog.bracket_sets(&ids);
if sets.is_empty() {
return Err(bad(ID, crate::merge::NO_BRACKET_SETS));
crates/engine/src/cmd/mod.rs
+1 -1
View File
@@ -109,7 +109,7 @@ pub fn has_selection(s: &Session) -> std::result::Result<(), String> {
/// can take (issue #684; Add to My Photos first).
pub fn has_library_selection(s: &Session) -> std::result::Result<(), String> {
has_selection(s)?;
if s.targets(&serde_json::Value::Null).iter().any(|id| s.catalog.photo(*id).is_none_or(|p| !p.local)) { Ok(()) } else { Err(LOCAL_ONLY.into()) }
if s.selected().iter().any(|id| s.catalog.photo(*id).is_none_or(|p| !p.local)) { Ok(()) } else { Err(LOCAL_ONLY.into()) }
}
/// Why an album command refused a Local-only selection.
pub const LOCAL_ONLY: &str = "only Local photos: albums hold library photos (Add to My Photos first)";
crates/engine/src/cmd/organize.rs
+7 -7
View File
@@ -22,7 +22,7 @@ fn stacks_of(s: &Session, targets: &[PhotoId]) -> Vec<StackId> {
fn in_stack(s: &Session) -> std::result::Result<(), String> {
has_selection(s)?;
if stacks_of(s, &s.targets(&Value::Null)).is_empty() { Err("no stacked photo selected".into()) } else { Ok(()) }
if stacks_of(s, &s.selected()).is_empty() { Err("no stacked photo selected".into()) } else { Ok(()) }
}
fn has_stacks(s: &Session) -> std::result::Result<(), String> {
@@ -111,7 +111,7 @@ pub fn specs() -> Vec<CommandSpec> {
"{ids?, name?} → {ids: [new photo ids]} — a new catalog entry sharing the original file, with independent settings; stacked with its original and named `name` or \"Copy N\"",
has_selection,
|s, p| {
let targets = s.targets(p);
let targets = s.targets(p)?;
let name = str_param(p, "name").filter(|n| !n.trim().is_empty()).map(str::to_string);
let mut made = Vec::new();
for t in targets {
@@ -134,7 +134,7 @@ pub fn specs() -> Vec<CommandSpec> {
"{ids?, top?: photoId (default: the active photo), collapsed?: bool (default true)} — stacks the photos are already in are merged",
has_selection,
|s, p| {
let targets = s.targets(p);
let targets = s.targets(p)?;
let top = p
.get("top")
.and_then(Value::as_u64)
@@ -153,11 +153,11 @@ pub fn specs() -> Vec<CommandSpec> {
}
),
cmd!("stack.ungroup", "Ungroup Stack", ["Photo", "Stack"], Some("Cmd+Shift+G"), "{ids?}", in_stack, |s, p| {
let ops = s.catalog.ungroup_ops(&s.targets(p));
let ops = s.catalog.ungroup_ops(&s.targets(p)?);
Ok(json!({"ungrouped": commit_ops(s, "Ungroup Stack", ops)?}))
}),
cmd!("stack.remove", "Remove from Stack", ["Photo", "Stack"], None, "{ids?}", in_stack, |s, p| {
let ops = s.catalog.remove_from_stacks_ops(&s.targets(p));
let ops = s.catalog.remove_from_stacks_ops(&s.targets(p)?);
Ok(json!({"changed": commit_ops(s, "Remove from Stack", ops)?}))
}),
cmd!("stack.setTop", "Set as Top of Stack", ["Photo", "Stack"], Some("Shift+S"), "{id?} (default: the active photo)", in_stack, |s, p| {
@@ -189,7 +189,7 @@ pub fn specs() -> Vec<CommandSpec> {
}
),
cmd!("stack.toggle", "Expand/Collapse Stack", ["Photo", "Stack"], Some("S"), "{ids?, collapsed?: bool}", in_stack, |s, p| {
let stacks = stacks_of(s, &s.targets(p));
let stacks = stacks_of(s, &s.targets(p)?);
let first = stacks.first().and_then(|id| s.catalog.stack(*id)).map(|st| st.collapsed).unwrap_or(false);
let collapsed = bool_or(p, "collapsed", !first);
set_collapsed(s, stacks, collapsed, if collapsed { "Collapse Stack" } else { "Expand Stack" })
@@ -212,7 +212,7 @@ pub fn specs() -> Vec<CommandSpec> {
|s, p| {
let gap = f64_or(p, "gap", 60.0).max(0.0);
let explicit = p.get("ids").is_some();
let pool = if explicit || s.selection.ids.len() > 1 { s.targets(p) } else { s.visible_cloned() };
let pool = if explicit || s.selection.ids.len() > 1 { s.targets(p)? } else { s.visible_cloned() };
let groups = s.catalog.auto_stack_groups(&pool, gap);
let photos: usize = groups.iter().map(Vec::len).sum();
if bool_or(p, "preview", false) {
crates/engine/src/cmd/previews.rs
+3 -3
View File
@@ -152,7 +152,7 @@ fn build(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad(C, "a preview build is already running"));
}
// explicit ids, else the selection, else everything in view
let ids = if p.get("ids").is_some() || p.get("id").is_some() || !s.selection.ids.is_empty() { s.targets(p) } else { s.visible_cloned() };
let ids = if p.get("ids").is_some() || p.get("id").is_some() || !s.selection.ids.is_empty() { s.targets(p)? } else { s.visible_cloned() };
let jobs: Vec<Vec<RenderJob>> = ids.iter().map(|id| jobs_for(s, *id, edge)).filter(|j| !j.is_empty()).collect();
let state = Arc::new(PreviewBuild { total: jobs.len(), ..Default::default() });
s.preview_build = Some(state.clone());
@@ -324,7 +324,7 @@ fn smart(s: &mut Session, p: &Value) -> Result<Value> {
if background && s.preview_build.as_ref().is_some_and(|b| !b.finished.load(Ordering::Relaxed)) {
return Err(bad(C, "a preview build is already running"));
}
let ids = if p.get("ids").is_some() || !s.selection.ids.is_empty() { s.targets(p) } else { s.visible_cloned() };
let ids = if p.get("ids").is_some() || !s.selection.ids.is_empty() { s.targets(p)? } else { s.visible_cloned() };
let mut jobs: Vec<SmartJob> = Vec::new();
for id in ids {
let Some(ph) = s.catalog.photo(id).cloned() else { continue };
@@ -503,7 +503,7 @@ pub fn specs() -> Vec<CommandSpec> {
smart_location
),
cmd!(query "photo.smartPreview", "Smart Preview Status", [], None, "{id?} → {smartPreview: bool, originalOnline: bool}", always, |s, p| {
let id = s.targets(p).first().copied().ok_or_else(|| bad("photo.smartPreview", "no photo"))?;
let id = s.targets(p)?.first().copied().ok_or_else(|| bad("photo.smartPreview", "no photo"))?;
let online = match s.catalog.photo(id).map(|p| p.source.clone()) {
Some(lightcraft_catalog::Source::File { path }) => std::path::Path::new(&path).exists(),
_ => true,
crates/engine/src/cmd/xmp.rs
+2 -2
View File
@@ -12,7 +12,7 @@ fn save(s: &mut Session, p: &Value) -> Result<Value> {
let mut merged = Vec::new();
let mut backups = Vec::new();
let mut failed = Vec::new();
let ids = s.targets(p);
let ids = s.targets(p)?;
let owners = crate::sidecar::StemOwners::among(&s.catalog, &ids);
for id in ids {
match s.save_sidecar_with(id, &owners) {
@@ -37,7 +37,7 @@ fn read(s: &mut Session, p: &Value) -> Result<Value> {
let mut read = Vec::new();
let mut failed = Vec::new();
let mut warnings = Vec::new();
for id in s.targets(p) {
for id in s.targets(p)? {
match s.read_sidecar_op_with_warnings(id) {
Ok(Some((op, from, mut photo_warnings))) => {
ops.push(op);
crates/engine/src/contact_sheet.rs
+1 -1
View File
@@ -83,7 +83,7 @@ pub fn prepare(session: &mut Session, p: &Value) -> Result<Prepared, String> {
if p.get("id").is_some_and(|v| v.as_u64().is_none()) {
return Err("id must be a photo id".into());
}
let ids = session.targets(p);
let ids = session.targets(p).map_err(|e| e.to_string())?;
if ids.is_empty() || ids.len() > 1000 {
return Err("select between 1 and 1000 photos".into());
}
crates/engine/src/lib.rs
+21 -7
View File
@@ -500,7 +500,11 @@ impl Session {
}
let empty = Value::Object(Default::default());
let params = if params.is_null() { &empty } else { params };
self.run_command(id, spec.journal.then_some(params), |s| (spec.run)(s, params))
self.run_command(id, spec.journal.then_some(params), |s| (spec.run)(s, params)).map_err(|e| match e {
// a shared parameter check ([`Session::targets`]) names the command it ran for
EngineError::BadParams { cmd, msg } if cmd.is_empty() => EngineError::BadParams { cmd: id.to_string(), msg },
e => e,
})
}
/// Run `f` as command `id` with what [`Session::execute`] does around every command: the
@@ -1167,14 +1171,24 @@ impl Session {
(self.visible_gen, self.visible.clone())
}
/// Targets of photo commands: explicit `ids`/`id` param, else the selection.
pub fn targets(&self, p: &Value) -> Vec<PhotoId> {
if let Some(a) = p.get("ids").and_then(Value::as_array) {
return a.iter().filter_map(Value::as_u64).map(PhotoId).collect();
/// Targets of photo commands: the explicit `ids` (photo ids) or `id` (a photo id) param, else [`Self::selected`].
/// An `ids` or `id` that is there but isn't photo ids is an error, never the selection or part of the list
/// (issue #1506): a mistyped target mustn't turn a command meant for one photo into one for every selected photo.
pub fn targets(&self, p: &Value) -> Result<Vec<PhotoId>> {
let invalid = |what: &str| EngineError::BadParams { cmd: String::new(), msg: format!("`{what}` must be photo ids (whole numbers)") };
match p.get("ids") {
None | Some(Value::Null) => {}
Some(Value::Array(a)) => return a.iter().map(|v| v.as_u64().map(PhotoId).ok_or_else(|| invalid("ids"))).collect(),
Some(_) => return Err(invalid("ids")),
}
if let Some(id) = p.get("id").and_then(Value::as_u64) {
return vec![PhotoId(id)];
match p.get("id") {
None | Some(Value::Null) => Ok(self.selected()),
Some(v) => v.as_u64().map(|id| vec![PhotoId(id)]).ok_or_else(|| invalid("id")),
}
}
/// The photos commands act on when a call names none: the selection, else the active photo.
pub fn selected(&self) -> Vec<PhotoId> {
if self.selection.ids.is_empty() { self.selection.active.into_iter().collect() } else { self.selection.ids.clone() }
}
}
crates/engine/src/tests.rs
+32
View File
@@ -1583,3 +1583,35 @@ fn rotate_carries_the_crop_with_the_image() {
assert_eq!(c.geometry.rect, left_half);
assert_eq!(c.aspect, Some((2, 3)));
}
/// A call that names its photos with a malformed `id` or `ids` (a string, a mixed list) is refused and changes
/// nothing; it used to act on the selection, or on the valid part of the list (issue #1506). Leaving the targets
/// out still means the selection.
#[test]
fn malformed_photo_targets_are_refused_not_widened() {
use lightcraft_catalog::PhotoId;
let mut s = Session::with_demo();
let ids: Vec<u64> = s.catalog.photos().take(3).map(|p| p.id.0).collect();
s.execute("library.select", &json!({"ids": [ids[0], ids[1]]})).unwrap();
let rating = |s: &Session, i: u64| s.catalog.photo(PhotoId(i)).unwrap().rating;
let before: Vec<u8> = ids.iter().map(|i| rating(&s, *i)).collect();
for bad in [
json!({"rating": 5, "id": "5"}),
json!({"rating": 5, "ids": [ids[2], "x"]}),
json!({"rating": 5, "ids": ids[2].to_string()}),
json!({"rating": 5, "id": -1}),
] {
let err = s.execute("photo.rate", &bad).unwrap_err();
assert!(matches!(&err, crate::EngineError::BadParams { cmd, .. } if cmd == "photo.rate"), "{bad}: {err}");
assert_eq!(ids.iter().map(|i| rating(&s, *i)).collect::<Vec<_>>(), before, "{bad} changed a rating");
}
let snapshot = s.catalog.to_snapshot();
assert!(s.execute("photo.deletePermanently", &json!({"id": "5"})).is_err());
assert_eq!(s.catalog.to_snapshot(), snapshot, "nothing deleted");
// no targets, or a null one: the selection
s.execute("photo.rate", &json!({"rating": 4, "id": null})).unwrap();
assert_eq!((rating(&s, ids[0]), rating(&s, ids[1]), rating(&s, ids[2])), (4, 4, before[2]));
// a preset's id is not a photo target
let preset = s.presets.first().unwrap().id.clone();
s.execute("preset.apply", &json!({"id": preset})).unwrap();
}
crates/ui-egui/src/menubar.rs
+1 -1
View File
@@ -706,7 +706,7 @@ pub(crate) fn expanded(app: &LightcraftApp, name: &str) -> Option<Vec<MenuNode>>
v
}
"Add to Album" => {
let targets = app.session.targets(&Value::Null);
let targets = app.session.selected();
let sel = !targets.is_empty();
// albums hold library photos (issue #684): a Local-only selection is told what to do first
let local_only = crate::panels::grid::local_only(app, &targets);
crates/ui-egui/src/menus.rs
+4 -4
View File
@@ -2124,7 +2124,7 @@ pub fn ui_enabled(app: &LightcraftApp, id: &str) -> bool {
"compare.swap" | "compare.makeSelect" | "compare.set" => app.ui.view == ViewMode::Compare,
"slideshow.togglePause" | "slideshow.next" | "slideshow.previous" | "slideshow.end" => app.ui.slideshow.is_some(),
s if s.starts_with("dialog.merge") || (s.starts_with("merge.") && s.ends_with("Last")) => {
app.session.targets(&serde_json::json!({})).len() >= 2 && app.merge.final_task.is_none()
app.session.selected().len() >= 2 && app.merge.final_task.is_none()
}
_ => true,
}
@@ -2176,7 +2176,7 @@ pub fn delete_in_album(app: &mut LightcraftApp, ctx: &egui::Context) -> bool {
app.toast(ctx, crate::i18n::tr("Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo"));
return true;
}
if app.session.targets(&json!({})).is_empty() {
if app.session.selected().is_empty() {
return true;
}
match app.run("album.removePhotos", json!({"id": aid.0})) {
@@ -2197,7 +2197,7 @@ pub fn confirm_delete(app: &mut LightcraftApp) -> bool {
if !app.ui.settings.confirm_delete {
return false;
}
let count = app.session.targets(&json!({})).len();
let count = app.session.selected().len();
if count == 0 {
return false;
}
@@ -2315,7 +2315,7 @@ fn open_original_in(app: &mut LightcraftApp, ctx: &egui::Context, p: &Value) ->
}
fn open_originals(app: &mut LightcraftApp, p: &Value) -> Result<Value, String> {
let mut ids = app.session.targets(p);
let mut ids = app.session.targets(p).map_err(|e| e.to_string())?;
ids.dedup();
if ids.is_empty() {
return Err("no photo selected".into());
crates/ui-egui/src/merge.rs
+3 -3
View File
@@ -150,7 +150,7 @@ fn spawn(app: &LightcraftApp, options: &MergeDialog, ids: &[PhotoId], preview: b
/// Open the merge dialog for the selection.
pub fn open(app: &mut LightcraftApp, command: &str) -> Result<Value, String> {
let ids = app.session.targets(&json!({}));
let ids = app.session.selected();
if ids.len() < 2 {
return Err("select at least 2 photos to merge".into());
}
@@ -187,7 +187,7 @@ pub fn start_final(app: &mut LightcraftApp, opts: &MergeDialog) -> Result<Value,
/// Merge the selection without the dialog, with the options last used for `command` (the
/// defaults the first time).
pub fn start_last(app: &mut LightcraftApp, command: &str) -> Result<Value, String> {
let ids = app.session.targets(&json!({}));
let ids = app.session.selected();
if ids.len() < 2 {
return Err("select at least 2 photos to merge".into());
}
@@ -202,7 +202,7 @@ pub fn start_batch(app: &mut LightcraftApp, p: &Value) -> Result<Value, String>
if app.merge.final_task.is_some() {
return Err("a merge is already running".into());
}
let ids = app.session.targets(p);
let ids = app.session.targets(p).map_err(|e| e.to_string())?;
let sets = app.session.catalog.bracket_sets(&ids);
if sets.is_empty() {
return Err(lightcraft_engine::merge::NO_BRACKET_SETS.into());
crates/ui-egui/src/panels/dialogs.rs
+7 -7
View File
@@ -22,7 +22,7 @@ pub(crate) fn rename_preview(
) -> (Option<Vec<lightcraft_engine::rename::RenamePlan>>, usize) {
use std::hash::{Hash, Hasher};
type Rows = std::sync::Arc<std::sync::Mutex<Option<Vec<lightcraft_engine::rename::RenamePlan>>>>;
let ids = app.session.targets(&json!({}));
let ids = app.session.selected();
let photos = lightcraft_engine::rename::rename_photos(&app.session.catalog, &ids);
let total = photos.len();
let mut h = std::collections::hash_map::DefaultHasher::new();
@@ -71,7 +71,7 @@ fn watermark_preview(
type Base = std::sync::Arc<std::sync::Mutex<(u8, Option<Result<lightcraft_raster::Rgba8, String>>)>>;
type Shown = (u64, u8, lightcraft_engine::export::Watermark, usize, egui::TextureHandle);
let t = Tokens::get(ui.ctx());
let Some(&id) = app.session.targets(&json!({})).first() else { return };
let Some(&id) = app.session.selected().first() else { return };
let Some(photo) = app.session.catalog.photo(id).cloned() else { return };
let (out_w, out_h) = lightcraft_engine::export::output_size(&photo, &lightcraft_engine::export::ExportOptions { resize, ..Default::default() });
let export_short = out_w.min(out_h);
@@ -471,7 +471,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
);
}
Dialog::CaptureTime { mode, time, days, hours, minutes, zone } => {
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
field(ui, "Change", |ui| {
ui.spacing_mut().item_spacing.x = 4.0;
for (i, (label, m)) in [("Set date & time", "set"), ("Shift by", "shift"), ("Time zone", "zone")].iter().enumerate() {
@@ -555,7 +555,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
);
}
Dialog::Rename { template, start } => {
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
ui.label(egui::RichText::new(crate::i18n::tr_format!("{n} photo{}", if n == 1 { "" } else { "s" }, n = n)).color(t.text_dim));
let template_id = egui::Id::new("rename-template");
let tags_open = field(ui, "Template", |ui| {
@@ -677,7 +677,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
confirm = true;
}
// as in Lightroom, the option is only there when something is selected
if !*folder && !app.session.targets(&json!({})).is_empty() {
if !*folder && !app.session.selected().is_empty() {
ui.add_space(6.0);
let r = ui.checkbox(include_selected, crate::i18n::tr("Include selected photos"));
crate::widgets::register(ui.ctx(), "check:albumIncludeSelected", r.rect);
@@ -696,7 +696,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
}
Dialog::CopySettings { groups } => group_checklist(ui, "copyGroup", groups),
Dialog::PasteSettings { groups } => {
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
ui.label(
egui::RichText::new(crate::i18n::tr_format!("Paste into {n} photo{} — only settings that were copied are pasted", if n == 1 { "" } else { "s" }, n = n))
.color(t.text_dim),
@@ -707,7 +707,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
Dialog::ContactSheet { options } => {
options.columns = options.columns.clamp(1, 8);
options.rows = options.rows.clamp(1, 10);
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
ui.label(format!("{n} selected photos · {} pages", n.div_ceil(options.columns * options.rows)));
field(ui, "Paper", |ui| {
crate::widgets::combo_box("contactSheetPaper").selected_text(&options.paper).show_ui(ui, |ui| {
crates/ui-egui/src/panels/grid.rs
+2 -2
View File
@@ -1090,7 +1090,7 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
}
});
crate::widgets::menu_button(ui, crate::i18n::tr("Set Color Label"), |ui| label_menu(app, ui));
if local_only(app, &app.session.targets(&serde_json::Value::Null)) {
if local_only(app, &app.session.selected()) {
// albums hold library photos (issue #684): nothing here to add until they join My Photos
let add = ui.add_enabled(false, egui::Button::new(crate::i18n::tr("Add to Album"))).on_disabled_hover_text(albums_hint());
register(ui.ctx(), "button:contextAddToAlbum", add.rect);
@@ -1196,7 +1196,7 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
let _ = app.run("develop.reset", json!({}));
}
crate::widgets::menu_button(ui, crate::i18n::tr("Photo Merge"), |ui| {
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
for (id, label) in [("dialog.mergeHdr", "HDR…"), ("dialog.mergePanorama", "Panorama…"), ("dialog.mergeHdrPanorama", "HDR Panorama…")] {
if ui.add_enabled(n >= 2, egui::Button::new(crate::i18n::tr(label))).clicked() {
let _ = app.run(id, json!({}));
crates/ui-egui/src/panels/keyword_list.rs
+1 -1
View File
@@ -77,7 +77,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
ui.label(egui::RichText::new(crate::i18n::tr(none)).color(t.text_dim));
});
}
let selection = app.session.targets(&serde_json::Value::Null);
let selection = app.session.selected();
let ticks = app.caches.keyword_ticks(&app.session.catalog, &selection);
for r in &rows {
row(app, ui, r, &selection, &ticks, filter.trim().is_empty());
crates/ui-egui/src/panels/keywording.rs
+1 -1
View File
@@ -30,7 +30,7 @@ pub fn view_switch(app: &mut LightcraftApp, ui: &mut egui::Ui) {
/// exported files will carry. One only some of the photos have is marked with an asterisk.
pub fn names_row(app: &mut LightcraftApp, ui: &mut egui::Ui) {
let t = Tokens::get(ui.ctx());
let selection = app.session.targets(&serde_json::Value::Null);
let selection = app.session.selected();
let export = app.ui.keywording_view == crate::state::KeywordingView::WillExport;
let names = app.caches.keyword_names(&app.session.catalog, &selection, export);
if names.is_empty() {
crates/ui-egui/src/panels/left.rs
+1 -1
View File
@@ -1565,7 +1565,7 @@ fn folder_menu(app: &mut LightcraftApp, resp: &egui::Response, a: &Album) {
ui.separator();
}
if !a.folder && !a.is_smart() {
let local_only = super::grid::local_only(app, &app.session.targets(&serde_json::Value::Null));
let local_only = super::grid::local_only(app, &app.session.selected());
let b = ui
.add_enabled(!local_only, egui::Button::new(crate::i18n::tr("Add Selected Photos")))
.on_disabled_hover_text(super::grid::albums_hint());
crates/ui-egui/src/panels/right.rs
+2 -2
View File
@@ -890,7 +890,7 @@ fn keywording(app: &mut LightcraftApp, ui: &mut egui::Ui) {
let kid = egui::Id::new("kw-input");
let mut text = ui.data_mut(|d| d.get_temp::<String>(kid).unwrap_or_default());
// the keywords every selected photo has: those only some have are still offered and suggested
let selection = app.session.targets(&serde_json::Value::Null);
let selection = app.session.selected();
let have: Vec<String> =
app.caches.keyword_chips(&app.session.catalog, &selection).iter().filter(|c| c.on_all()).map(|c| c.path.clone()).collect();
// while the box has the cursor it offers the library's keywords for what is being typed,
@@ -1067,7 +1067,7 @@ fn keywording(app: &mut LightcraftApp, ui: &mut egui::Ui) {
/// selected photos; "Save as Set…" keeps the current nine under a name.
fn keyword_set(app: &mut LightcraftApp, ui: &mut egui::Ui) {
// on: every selected photo has it (⌥1–⌥9 then take it off them); partly on: only some do
let selection = app.session.targets(&serde_json::Value::Null);
let selection = app.session.selected();
let chips = app.caches.keyword_chips(&app.session.catalog, &selection);
let t = Tokens::get(ui.ctx());
let sets = lightcraft_engine::cmd::keywords::keyword_sets_json(&app.session);
crates/ui-egui/src/shortcuts.rs
+1 -1
View File
@@ -573,7 +573,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
// B: the brush while editing; in the grids, add to the target album (Quick Collection)
if f == "tool.brush" && matches!(app.ui.view, crate::state::ViewMode::PhotoGrid | crate::state::ViewMode::SquareGrid) {
if let Ok(r) = app.run("album.toggleTarget", json!({})) {
let n = app.session.targets(&json!({})).len();
let n = app.session.selected().len();
let what = crate::i18n::tr_format!("{n} photo{}", if n == 1 { "" } else { "s" }, n = n);
let name = r["name"].as_str().unwrap_or("Quick Collection").to_string();
app.toast(
crates/ui-egui/src/tests_keywording.rs
+1 -1
View File
@@ -191,7 +191,7 @@ fn the_active_photo_alone_is_the_selection() {
assert_eq!(h.app.session.selection.active.map(|p| p.0), Some(ids[0]));
assert!(h.app.session.selection.ids.is_empty());
assert!(has(&h, "keywordChip:Weddings") && !has(&h, "keywordChipPartial:Weddings"), "the active photo's keywords");
let ticks = crate::panels::keyword_list::Ticks::of(&h.app.session.catalog, &h.app.session.targets(&json!({})));
let ticks = crate::panels::keyword_list::Ticks::of(&h.app.session.catalog, &h.app.session.selected());
assert_eq!(ticks.tick("Weddings"), crate::panels::keyword_list::Tick::All);
assert!(has(&h, "keywordCheck:beach"));
}
docs/whats-new.md
+5
View File
@@ -117,6 +117,11 @@
"Canon CRX adaptive QP value 172" or "175". They now decode from the sensor data. Photos imported before this
switch over with Photo ▸ Reload from Disk.
### Reliability
- A command given photos it can't read as photo ids (`"id": "5"`, or a list with something else in it) from a
script, the control channel or MCP is now refused (issue #1506). It used to act on the selected photos instead,
or on the part of the list it could read, so a mistyped id could rate, move or delete the wrong photos.
### Hard-edged Heal and Clone spots
- A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity
changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged.