From c782bf6a856eace4c21053ceeb540e8aa0134408 Mon Sep 17 00:00:00 2001 From: Alvaro Gonzalez-Jimenez Date: Mon, 19 Oct 2026 16:33:22 +0200 Subject: [PATCH 1/2] #2312: culling goes on from a photo rated out of the view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With a filter showing one-star photos, taking the star off a photo took it out of the view but left it the active photo: the grid highlighted nothing, Detail kept showing it, and the next step (→, Shift+0–5 or Auto Advance) started from the view's first photo, because `library.next` finds no place in the view for an active photo the view doesn't show. A rating, flag or label applied to the selection now does what a delete already does when it takes the active photo out of the view: the photo after it becomes active (the one before it at the end). That photo is the next one, so `advance`, Shift+key and Auto Advance (keys and the Photo menu) don't step past it. Commands that name their photos with `ids` leave the selection alone, so Compare and Survey still show the photo they rate; a held view keeps the photo, and the selection with it. photo.pick, photo.reject, photo.unflag and photo.label now take `advance` like photo.rate and photo.flag. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 70 +++++++---- .../engine/src/cmd/library/tests_rated_out.rs | 112 ++++++++++++++++++ crates/ui-egui/src/menubar.rs | 3 +- crates/ui-egui/src/panels/compare.rs | 6 +- crates/ui-egui/src/shortcuts.rs | 35 +++++- docs/parity-checklist.md | 5 +- docs/whats-new.md | 9 ++ 7 files changed, 210 insertions(+), 30 deletions(-) create mode 100644 crates/engine/src/cmd/library/tests_rated_out.rs diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 860cf089f..57185e664 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -205,7 +205,6 @@ fn change_rating(s: &mut Session, p: &Value, delta: i8, label: &str) -> Result 0 { s.commit(label, Op::Batch { ops })?; } - advance_if(s, p); Ok(json!({"changed": n})) } @@ -224,10 +223,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) -> Result { + 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); @@ -741,21 +761,17 @@ pub fn specs() -> Vec { if !(0..=5).contains(&r) { return Err(bad("photo.rate", "rating must be 0..5")); } - let v = for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 }))?; - advance_if(s, p); - Ok(v) + cull(s, p, |s| for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 }))) }), cmd!("photo.decreaseRating", "Decrease Rating", [], None, "{ids?, advance?: bool}", has_selection, |s, p| { - change_rating(s, p, -1, "Decrease Rating") + cull(s, p, |s| 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") + 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}", 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) + 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!( @@ -766,10 +782,12 @@ pub fn specs() -> Vec { "{ids?, advance?: bool} — each photo's own flag: unflagged -> pick -> reject -> unflagged", 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, + }) }) } ), @@ -780,7 +798,7 @@ pub fn specs() -> Vec { Some("Cmd+Up"), "{ids?, advance?: bool} — reject -> unflagged -> pick (stops at pick)", 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", @@ -789,30 +807,30 @@ pub fn specs() -> Vec { Some("Cmd+Down"), "{ids?, advance?: bool} — pick -> unflagged -> reject (stops at reject)", 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?}", has_selection, |s, p| { - for_targets(s, p, "Flag as Pick", |id| Some(Op::SetFlag { id, flag: Flag::Pick })) + cmd!("photo.pick", "Flag as Pick", ["Photo", "Set Flag"], Some("P"), "{ids?, advance?: bool}", 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?}", has_selection, |s, p| { - for_targets(s, p, "Flag as Reject", |id| Some(Op::SetFlag { id, flag: Flag::Reject })) + cmd!("photo.reject", "Flag as Reject", ["Photo", "Set Flag"], Some("X"), "{ids?, advance?: bool}", 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?}", has_selection, |s, p| { - for_targets(s, p, "Unflag", |id| Some(Op::SetFlag { id, flag: Flag::None })) + cmd!("photo.unflag", "Unflag", ["Photo", "Set Flag"], Some("U"), "{ids?, advance?: bool}", has_selection, |s, p| { + cull(s, p, |s| 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}", has_selection, |s, p| { let l = match str_param(p, "label") { None | Some("none") => None, Some(x) => Some(ColorLabel::parse(x).ok_or_else(|| bad("photo.label", "unknown label"))?), }; - 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 @@ -1758,3 +1776,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; diff --git a/crates/engine/src/cmd/library/tests_rated_out.rs b/crates/engine/src/cmd/library/tests_rated_out.rs new file mode 100644 index 000000000..af11bab8d --- /dev/null +++ b/crates/engine/src/cmd/library/tests_rated_out.rs @@ -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 { + 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, ¶ms).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))); +} diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index c5e4e45c7..27ab9e289 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -858,9 +858,10 @@ pub fn run_item(app: &mut LightcraftApp, id: &str, params: Value) -> Result Result { } /// After rating/flagging with Auto Advance: next candidate (Compare), next photo in the survey, -/// else the next photo in the view. -pub fn advance(app: &mut LightcraftApp) { +/// else the next photo in the view. `was` is the active photo before the rating: a rating that took +/// it out of the view made the photo after it active already, and that is the next one (#2312). +pub fn advance(app: &mut LightcraftApp, was: Option) { let _ = match app.ui.view { ViewMode::Compare => compare_step(app, 1), ViewMode::Survey => survey_step(app, 1), + _ if app.session.active() != was => Ok(Value::Null), _ => app.session.execute("library.next", &json!({})).map_err(|e| e.to_string()), }; } diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index b9d1874a1..d009e55ef 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -437,9 +437,10 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { // Auto Advance then moves on (next candidate in Compare, next photo elsewhere) let cull = |app: &mut LightcraftApp, id: &str, mut params: serde_json::Value, advance: bool| { compare::target_active(app, &mut params); + let was = app.session.active(); let ok = app.run(id, params).is_ok(); if ok && (advance || app.ui.auto_advance) { - compare::advance(app); + compare::advance(app, was); } }; for (id, params) in aliased { @@ -877,6 +878,38 @@ mod tests { } } + #[test] + fn culling_keys_go_on_from_a_photo_that_leaves_the_filter() { + // #2312: the filter shows one-star photos; taking photo 2's star off makes photo 3 active, once, with or + // without Shift and Auto Advance, and → goes on from there instead of from the first photo + use lightcraft_catalog::PhotoId; + let one_star = || { + let mut h = library(); + h.app.session.execute("library.selectAll", &json!({})).unwrap(); + h.app.session.execute("photo.rate", &json!({"rating": 1})).unwrap(); + h.app.session.execute("library.filter", &json!({"rating": 1, "ratingOp": "exactly"})).unwrap(); + h.app.session.execute("library.select", &json!({"ids": [2]})).unwrap(); + h + }; + for view in [crate::state::ViewMode::PhotoGrid, crate::state::ViewMode::Detail] { + for (shift, auto) in [(false, false), (true, false), (false, true), (true, true)] { + let mut h = one_star(); + h.app.ui.view = view; + h.app.ui.auto_advance = auto; + key(&mut h, "0", shift); + assert_eq!(h.app.session.catalog.photo(PhotoId(2)).unwrap().rating, 0); + assert_eq!(h.app.session.active(), Some(PhotoId(3)), "{view:?} Shift={shift} Auto Advance={auto}"); + key(&mut h, "right", false); + assert_eq!(h.app.session.active(), Some(PhotoId(4)), "{view:?} Shift={shift} Auto Advance={auto}"); + } + } + // the Photo menu with Auto Advance too + let mut h = one_star(); + h.app.ui.auto_advance = true; + crate::menubar::run_item(&mut h.app, "photo.rate", json!({"rating": 0})).unwrap(); + assert_eq!(h.app.session.active(), Some(PhotoId(3))); + } + #[test] fn color_label_actions_show_feedback_only_after_success() { use lightcraft_catalog::PhotoId; diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 4e0531793..c7ee4f53b 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-15 · **Change:** minor (LRC-PRINT-PAGE: Print Page Options) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-19 · **Change:** patch (LR-LIB-RATING: culling goes on from a photo rated out of the view, #2312) · **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 @@ -102,7 +102,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service | | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library | | LR-LIB-TARGET | Target album | P2 | ✅ | `cmd:album.setTarget`, `cmd:album.toggleTarget` | see LRC-LIB-COLLECTIONS | -| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs` | 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | +| LR-LIB-RATING | Star ratings | P0 | ✅ | `cmd:photo.rate` (`advance`), `cmd:photo.decreaseRating`, `cmd:photo.increaseRating`, `crates/ui-egui/src/shortcuts.rs` | 0 clears; 1–5 set; Shift+0–5 applies and advances once, including with Auto Advance. A rating, flag or label that takes the active photo out of the view (filter bar, Picks, smart album) makes the photo after it active (the one before at the end), and Shift / Auto Advance doesn't step past it (#2312). Default grid brackets adjust each photo by one, clamped to 0–5; one batch undo and AutoWrite. Batch grid and undo/redo regressions; macOS native ownership corrected (PR #261) | | LR-LIB-FLAG | Pick / reject flags | P0 | ✅ | `cmd:photo.pick`, `cmd:photo.reject`, `cmd:photo.unflag`, `cmd:photo.flag` | P pick, X reject, U clear; Shift+P picks and advances in both grids (Presets elsewhere); Shift+X/U and legacy Shift+Z advance; no flag cycle; rejection does not delete the file | | LR-LIB-LABEL | Colour labels | P1 | ✅ | `cmd:photo.label`, `cmd:label.setNames`, `cmd:label.names`, `cmd:dialog.labelNames`, keys 6–9 in `crates/ui-egui/src/shortcuts.rs` | Photo menu and grid context menu (coloured, named), Info-panel swatches, label dot plus translucent colour on square-grid/filmstrip surrounds and photo-grid footers (photo pixels unchanged); colour-label toasts have pale matching backgrounds; setting/clearing through keys (including Shift), menus or Info swatches shows the same bottom toast as ratings, with the colour/custom name; editable label names (undoable, journaled); no purple key (as in Lightroom), no custom extra labels | | LR-LIB-KEYWORD | Keywords | P0 | ✅ | `cmd:panel.keywords`, `cmd:photo.setMeta` (`addKeywords`/`removeKeywords`), `cmd:keyword.list`, `cmd:keyword.rename`, `cmd:keyword.delete`, `cmd:keyword.merge`, `cmd:keyword.suggest`, `crates/catalog/src/keywords.rs`, `crates/ui-egui/src/panels/left.rs` | left-panel keyword tree with counts (click filters, children included; context menu rename / merge / delete / add / remove); library-wide ops are one undo step and replay from the op log; suggestions (co-occurring / most used / completions) in the Keywords panel; keywords reorganize by drag and drop in the Keyword List (LRC-LIB-KEYWORDS) | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-19 | patch | LR-LIB-RATING: a rating, flag or label that takes the active photo out of the view makes the photo after it active instead of leaving an unseen photo active (→ then went to the view's first photo), and Shift / Auto Advance doesn't step past it (#2312); status unchanged (✅) | | 2026-10-14 | patch | LR-EXP-TYPE: opaque 8-bit PNG/TIFF/WebP use RGB; transparent exports retain alpha (#1670). Statuses and feature counts unchanged. | | 2026-10-15 | minor | LRC-PRINT-PAGE: Page Options (Page Numbers, Page Info, Crop Marks) on Single Image and Contact Sheet pages, in the Print dialog, `print.set` and templates; still 🟡 (no identity plate, watermark, photo info or Cut Guides) | | 2026-10-15 | minor | LR-IMP-FORMATS: Hasselblad 3FR and FFF show their uncompressed RGB preview (no JPEG in these files) in the grid and loupe until developed, and as the stand-in when a file fails to decode; status unchanged (🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index 2f02ffb91..d08d1eade 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,14 @@ # What's new in LightCraft +## 2026-10-19 + +### Rating in a filtered view keeps your place +- When a filter shows only one-star photos and you take the star off a photo, it leaves the view and the photo after + it is now selected, so you carry on from where you were (issue #2312). The same goes for flags and colour labels, + in Picks and in smart albums. Before, the photo stayed selected although the grid no longer showed it, and → or + Auto Advance jumped back to the first photo of the view. With Auto Advance on, or with Shift+0–5, you land on the + photo after it: no photo is skipped. + ## 2026-10-14 ### Model downloads time out on a name that doesn't resolve From aa729d28e8db1bde965931b508aa3c1629e70e10 Mon Sep 17 00:00:00 2001 From: Alvaro Gonzalez-Jimenez Date: Mon, 19 Oct 2026 18:22:06 +0200 Subject: [PATCH 2/2] #2312: the culling commands say the selection moves on From review on #2327: the move is in the engine, so a script, the CLI or an MCP client that rates the active photo out of a filtered view without `ids` finds the next photo selected afterwards. The parameter text of the eleven rating, flag and label commands now says so, where commands.list and the MCP tools show it: without `ids`, the selection moves on when the edit takes the active photo out of the view. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 104 ++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 29 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 57185e664..20f3560ae 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -756,30 +756,58 @@ pub fn specs() -> Vec { |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 }))) } - cull(s, p, |s| for_targets(s, p, "Set Rating", |id| Some(Op::SetRating { id, rating: r as u8 }))) - }), - cmd!("photo.decreaseRating", "Decrease Rating", [], None, "{ids?, advance?: bool}", has_selection, |s, p| { - cull(s, p, |s| change_rating(s, p, -1, "Decrease Rating")) - }), - cmd!("photo.increaseRating", "Increase Rating", [], None, "{ids?, advance?: bool}", 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}", 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 }))) - }), + ), + 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| { cull(s, p, |s| { @@ -796,7 +824,7 @@ pub fn specs() -> Vec { "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| { cull(s, p, |s| change_flag(s, p, "Increase Flag Status", |f| if f == Flag::Reject { Flag::None } else { Flag::Pick })) } ), @@ -805,25 +833,43 @@ pub fn specs() -> Vec { "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| { 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}", 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}", 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}", 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?, 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.label", "Set Color Label", ["Photo", "Set Color Label"], None, - "{label: red|yellow|green|blue|purple|none, ids?, advance?: bool}", + "{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| { let l = match str_param(p, "label") {