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

Merge PR #2327: #2312: culling goes on from a photo rated out of the view

Browse Source 
# Conflicts:
#	docs/parity-checklist.md
#	docs/whats-new.md
This commit is contained in:
francoisgn
2026-10-22 22:28:40 +03:00
parent be5735abf4 aa729d28e8
commit 4e75723da5
7 changed files with 273 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/library.rs
+110 -42
View File
@@ -201,7 +201,6 @@ fn change_rating(s: &mut Session, p: &Value, delta: i8, label: &str) -> Result<V
if n > 0 {
s.commit(label, Op::Batch { ops })?;
}
advance_if(s, p);
Ok(json!({"changed": n}))
}
@@ -220,10 +219,31 @@ fn change_flag(s: &mut Session, p: &Value, label: &str, step: impl Fn(Flag) -> F
if n > 0 {
s.commit(label, Op::Batch { ops })?;
}
advance_if(s, p);
Ok(json!({"changed": n}))
}
/// Rate, flag or label (`edit`), then `advance`. When the edit took the active photo out of the view (the filter bar,
/// Picks or a smart album shows it no more), culling goes on from its place, as after a delete: the photo after it is
/// active, or the one before it at the end (#2312), not the view's first photo. That photo is the next one, so
/// `advance` doesn't step past it. Photos named by `ids` / `id` leave the selection to the caller: Compare and Survey
/// keep showing the photo they rate.
fn cull(s: &mut Session, p: &Value, edit: impl FnOnce(&mut Session) -> Result<Value>) -> Result<Value> {
let named = [p.get("ids"), p.get("id")].into_iter().any(|t| t.is_some_and(|t| !t.is_null()));
let before = (!named).then(|| s.visible_shared().1);
let active = s.selection.active;
let v = edit(s)?;
if let Some(before) = before
&& active.is_some_and(|a| before.contains(&a) && !s.visible().contains(&a))
{
select_after_deletion(s, &before);
s.active_mask = None;
s.active_spot = None;
} else {
advance_if(s, p);
}
Ok(v)
}
fn advance_if(s: &mut Session, p: &Value) {
if bool_or(p, "advance", false) {
let _ = step(s, 1);
@@ -732,40 +752,66 @@ pub fn specs() -> Vec<CommandSpec> {
|s, p| { step(s, -row_step(p)) }
),
// ---- rating / flags / labels
cmd!("photo.rate", "Set Rating", ["Photo", "Set Rating"], None, "{rating: 0..5, ids?, advance?: bool}", has_selection, |s, p| {
let r = super::f64_req(p, "rating", "photo.rate")? as i64;
if !(0..=5).contains(&r) {
return Err(bad("photo.rate", "rating must be 0..5"));
cmd!(
"photo.rate",
"Set Rating",
["Photo", "Set Rating"],
None,
"{rating: 0..5, ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| {
let r = super::f64_req(p, "rating", "photo.rate")? as i64;
if !(0..=5).contains(&r) {
return Err(bad("photo.rate", "rating must be 0..5"));
}
cull(s, p, |s| for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 })))
}
let v = for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 }))?;
advance_if(s, p);
Ok(v)
}),
cmd!("photo.decreaseRating", "Decrease Rating", [], None, "{ids?, advance?: bool}", has_selection, |s, p| {
change_rating(s, p, -1, "Decrease Rating")
}),
cmd!("photo.increaseRating", "Increase Rating", [], None, "{ids?, advance?: bool}", has_selection, |s, p| {
change_rating(s, p, 1, "Increase Rating")
}),
cmd!("photo.flag", "Set Flag", ["Photo", "Set Flag"], None, "{flag: pick|reject|none, ids?, advance?: bool}", has_selection, |s, p| {
let f = str_param(p, "flag").and_then(Flag::parse).ok_or_else(|| bad("photo.flag", "flag must be pick|reject|none"))?;
let v = for_targets(s, p, "Set Flag", |id| Some(Op::SetFlag { id, flag: f }))?;
advance_if(s, p);
Ok(v)
}),
),
cmd!(
"photo.decreaseRating",
"Decrease Rating",
[],
None,
"{ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { cull(s, p, |s| change_rating(s, p, -1, "Decrease Rating")) }
),
cmd!(
"photo.increaseRating",
"Increase Rating",
[],
None,
"{ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { cull(s, p, |s| change_rating(s, p, 1, "Increase Rating")) }
),
cmd!(
"photo.flag",
"Set Flag",
["Photo", "Set Flag"],
None,
"{flag: pick|reject|none, ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| {
let f = str_param(p, "flag").and_then(Flag::parse).ok_or_else(|| bad("photo.flag", "flag must be pick|reject|none"))?;
cull(s, p, |s| for_targets(s, p, "Set Flag", |id| Some(Op::SetFlag { id, flag: f })))
}
),
// Classic: ` cycles the flag; Cmd+Up / Cmd+Down raise / lower it (Ctrl on Windows and Linux)
cmd!(
"photo.cycleFlag",
"Cycle Flag",
["Photo", "Set Flag"],
Some("`"),
"{ids?, advance?: bool} — each photo's own flag: unflagged -> pick -> reject -> unflagged",
"{ids?, advance?: bool} — each photo's own flag: unflagged -> pick -> reject -> unflagged; without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| {
change_flag(s, p, "Cycle Flag", |f| match f {
Flag::None => Flag::Pick,
Flag::Pick => Flag::Reject,
Flag::Reject => Flag::None,
cull(s, p, |s| {
change_flag(s, p, "Cycle Flag", |f| match f {
Flag::None => Flag::Pick,
Flag::Pick => Flag::Reject,
Flag::Reject => Flag::None,
})
})
}
),
@@ -774,34 +820,52 @@ pub fn specs() -> Vec<CommandSpec> {
"Increase Flag Status",
["Photo", "Set Flag"],
Some("Cmd+Up"),
"{ids?, advance?: bool} — reject -> unflagged -> pick (stops at pick)",
"{ids?, advance?: bool} — reject -> unflagged -> pick (stops at pick); without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { change_flag(s, p, "Increase Flag Status", |f| if f == Flag::Reject { Flag::None } else { Flag::Pick }) }
|s, p| { cull(s, p, |s| change_flag(s, p, "Increase Flag Status", |f| if f == Flag::Reject { Flag::None } else { Flag::Pick })) }
),
cmd!(
"photo.decreaseFlag",
"Decrease Flag Status",
["Photo", "Set Flag"],
Some("Cmd+Down"),
"{ids?, advance?: bool} — pick -> unflagged -> reject (stops at reject)",
"{ids?, advance?: bool} — pick -> unflagged -> reject (stops at reject); without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { change_flag(s, p, "Decrease Flag Status", |f| if f == Flag::Pick { Flag::None } else { Flag::Reject }) }
|s, p| { cull(s, p, |s| change_flag(s, p, "Decrease Flag Status", |f| if f == Flag::Pick { Flag::None } else { Flag::Reject })) }
),
cmd!(
"photo.pick",
"Flag as Pick",
["Photo", "Set Flag"],
Some("P"),
"{ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { cull(s, p, |s| for_targets(s, p, "Flag as Pick", |id| Some(Op::SetFlag { id, flag: Flag::Pick }))) }
),
cmd!(
"photo.reject",
"Flag as Reject",
["Photo", "Set Flag"],
Some("X"),
"{ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { cull(s, p, |s| for_targets(s, p, "Flag as Reject", |id| Some(Op::SetFlag { id, flag: Flag::Reject }))) }
),
cmd!(
"photo.unflag",
"Unflag",
["Photo", "Set Flag"],
Some("U"),
"{ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| { cull(s, p, |s| for_targets(s, p, "Unflag", |id| Some(Op::SetFlag { id, flag: Flag::None }))) }
),
cmd!("photo.pick", "Flag as Pick", ["Photo", "Set Flag"], Some("P"), "{ids?}", has_selection, |s, p| {
for_targets(s, p, "Flag as Pick", |id| Some(Op::SetFlag { id, flag: Flag::Pick }))
}),
cmd!("photo.reject", "Flag as Reject", ["Photo", "Set Flag"], Some("X"), "{ids?}", has_selection, |s, p| {
for_targets(s, p, "Flag as Reject", |id| Some(Op::SetFlag { id, flag: Flag::Reject }))
}),
cmd!("photo.unflag", "Unflag", ["Photo", "Set Flag"], Some("U"), "{ids?}", has_selection, |s, p| {
for_targets(s, p, "Unflag", |id| Some(Op::SetFlag { id, flag: Flag::None }))
}),
cmd!(
"photo.label",
"Set Color Label",
["Photo", "Set Color Label"],
None,
"{label: red|yellow|green|blue|purple|none, ids?}",
"{label: red|yellow|green|blue|purple|none, ids?, advance?: bool} — without `ids`, the selection moves on when this takes the active photo out of the view",
has_selection,
|s, p| {
// `none` (or null) clears; a missing label or one that isn't text is an error, not a clear
@@ -812,7 +876,7 @@ pub fn specs() -> Vec<CommandSpec> {
None => return Err(bad("photo.label", "missing `label` (red|yellow|green|blue|purple|none)")),
Some(_) => return Err(bad("photo.label", "label must be red|yellow|green|blue|purple|none")),
};
for_targets(s, p, "Set Color Label", |id| Some(Op::SetLabel { id, label: l }))
cull(s, p, |s| for_targets(s, p, "Set Color Label", |id| Some(Op::SetLabel { id, label: l })))
}
),
// ---- orientation
@@ -1771,3 +1835,7 @@ mod relative_rating_tests;
#[cfg(test)]
#[path = "library/tests_flag_cycle.rs"]
mod flag_cycle_tests;
#[cfg(test)]
#[path = "library/tests_rated_out.rs"]
mod rated_out_tests;
crates/engine/src/cmd/library/tests_rated_out.rs
+112
View File
@@ -0,0 +1,112 @@
//! A rating, flag or label that takes the active photo out of the view (the filter bar, Picks) goes on from its place:
//! the photo that took it, not the view's first photo (#2312).
use lightcraft_catalog::{ColorLabel, Flag, Op, Photo, PhotoId, Source};
use serde_json::json;
use crate::{Result, Selection, Session};
/// 100 photos with one star each, in file-name order (photo `n` is the `n`th), the filter showing one-star photos.
fn one_star() -> Result<Session> {
let mut s = Session::new();
for id in 1..=100 {
let mut photo = Photo::new(
PhotoId(id),
Source::File { path: format!("/lightcraft-rated-out/{id:03}.jpg") },
&format!("{id:03}.jpg"),
"JPEG",
40,
30,
"2026-10-19",
);
photo.rating = 1;
photo.flag = Flag::Pick;
photo.label = Some(ColorLabel::Red);
s.catalog.apply(Op::AddPhoto { photo: Box::new(photo) })?;
}
s.execute("library.sort", &json!({"key": "fileName", "ascending": true}))?;
s.execute("library.filter", &json!({"rating": 1, "ratingOp": "exactly"}))?;
Ok(s)
}
#[test]
fn a_star_taken_off_goes_on_to_the_next_photo() {
// Given photo 82 of 100 one-star photos, the filter showing one-star photos
let mut s = one_star().unwrap();
s.execute("library.select", &json!({"ids": [82]})).unwrap();
// When its star is taken off, Then it leaves the view and the photo after it is the one selected
s.execute("photo.rate", &json!({"rating": 0})).unwrap();
assert!(!s.visible_cloned().contains(&PhotoId(82)));
assert_eq!(s.selection, Selection::single(PhotoId(83)));
// and the next photo is the one after that, not the first one of the view
s.execute("library.next", &json!({})).unwrap();
assert_eq!(s.active(), Some(PhotoId(84)));
// With `advance`, the photo that took its place is the next one: no photo is skipped
s.execute("photo.rate", &json!({"rating": 0, "advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(85)));
s.execute("photo.decreaseRating", &json!({"advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(86)));
// The last photo of the view hands on to the one before it
s.execute("library.select", &json!({"ids": [100]})).unwrap();
s.execute("photo.rate", &json!({"rating": 0, "advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(99)));
// A rating that keeps the photo in the view advances as before
s.execute("photo.rate", &json!({"rating": 1, "advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(99)), "the last photo of the view stays");
s.execute("library.select", &json!({"ids": [10]})).unwrap();
s.execute("photo.rate", &json!({"rating": 1, "advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(11)));
}
#[test]
fn flags_and_labels_that_leave_the_view_go_on_to_the_next_photo_too() {
for (filter, command, params) in [
(json!({"flag": "pick"}), "photo.reject", json!({})),
(json!({"flag": "pick"}), "photo.flag", json!({"flag": "none"})),
(json!({"flag": "pick"}), "photo.cycleFlag", json!({})),
(json!({"flag": "pick"}), "photo.decreaseFlag", json!({})),
(json!({"label": "red"}), "photo.label", json!({"label": "none"})),
(json!({}), "photo.increaseRating", json!({})),
] {
let mut s = one_star().unwrap();
s.execute("library.filter", &filter).unwrap();
s.execute("library.select", &json!({"ids": [40]})).unwrap();
s.execute(command, &params).unwrap();
assert!(!s.visible_cloned().contains(&PhotoId(40)), "{command} {params}");
assert_eq!(s.selection, Selection::single(PhotoId(41)), "{command} {params}");
}
}
#[test]
fn several_photos_rated_out_go_on_after_the_active_one() {
let mut s = one_star().unwrap();
// three selected, the middle one active: all leave, the photo after the active one is selected
s.execute("library.select", &json!({"ids": [20, 21, 22], "active": 21})).unwrap();
s.execute("photo.rate", &json!({"rating": 0})).unwrap();
assert_eq!(s.selection, Selection::single(PhotoId(23)));
// the active one leaves and another selected photo stays: that one is active
s.execute("photo.rate", &json!({"ids": [30], "rating": 3})).unwrap();
s.execute("library.filter", &json!({"rating": 1, "ratingOp": "atLeast"})).unwrap();
s.execute("library.select", &json!({"ids": [29, 30], "active": 29})).unwrap();
s.execute("photo.decreaseRating", &json!({})).unwrap();
assert_eq!(s.selection, Selection { ids: vec![PhotoId(30)], active: Some(PhotoId(30)) });
}
#[test]
fn named_photos_and_a_held_view_keep_the_selection() {
// Photos named by `ids` leave the selection to the caller (Compare and Survey keep the photo they show)
let mut s = one_star().unwrap();
s.execute("library.select", &json!({"ids": [5, 6], "active": 6})).unwrap();
s.execute("photo.rate", &json!({"ids": [6], "rating": 0})).unwrap();
assert_eq!(s.selection, Selection { ids: vec![PhotoId(5), PhotoId(6)], active: Some(PhotoId(6)) });
// A held view keeps the photo, so the selection stays with it
let mut s = one_star().unwrap();
s.execute("library.holdView", &json!({"on": true})).unwrap();
// (what the grid shows is what is held)
assert_eq!(s.visible_cloned().len(), 100);
s.execute("library.select", &json!({"ids": [7]})).unwrap();
s.execute("photo.rate", &json!({"rating": 0})).unwrap();
assert!(s.visible_cloned().contains(&PhotoId(7)));
assert_eq!(s.selection, Selection::single(PhotoId(7)));
s.execute("photo.rate", &json!({"rating": 0, "advance": true})).unwrap();
assert_eq!(s.active(), Some(PhotoId(8)));
}