Keep colour labels in the sidecar so they survive and travel
A rating and a flag are written to DarkRoom's sidecar as well as the catalog, because the catalog is a disposable index and the sidecar is how a judgement reaches the photographer's other devices. A label had no place there, so once labels could be set, one would have lived only in the catalog of the device it was set on and gone with it. The sidecar version now carries `label` (0 none, 1-5 as the catalog codes it), written only when set. It merges under the rating's rule, so a device that never labelled a frame cannot clear another device's label, and a code this build does not know reads as none rather than as some other colour. A judgement write carries the catalog's label with the stars, and the scan takes a sidecar's label into the catalog when it has one. An older build keeps the line as an unknown key and writes it back.
This commit is contained in:
@@ -93,6 +93,9 @@ pub const MAX_RATING: u8 = 5;
|
||||
/// Highest flag code: 0 unflagged, 1 pick, 2 reject.
|
||||
pub const MAX_FLAG: u8 = 2;
|
||||
|
||||
/// Highest colour-label code: purple. Mirrors `dr_catalog::rating::label_code`.
|
||||
pub const MAX_LABEL: u8 = 5;
|
||||
|
||||
/// TRACES: FR-CAT-8 | FR-NC-8
|
||||
/// One image's sidecar: a keyed set of versions.
|
||||
///
|
||||
@@ -187,6 +190,17 @@ pub struct Version {
|
||||
/// so a value moving between the two stores needs no translation table
|
||||
/// that could drift.
|
||||
pub flag: u8,
|
||||
/// TRACES: FR-CAT-5
|
||||
/// The colour label, `0` for none and `1..=5` red, yellow, green, blue,
|
||||
/// purple — the catalog's `versions.label` codes, for the reason
|
||||
/// [`Self::flag`] shares its encoding.
|
||||
///
|
||||
/// Here for the reason the rating is: a label that lived only in the
|
||||
/// catalog would go with the catalog, and would never reach the
|
||||
/// photographer's other devices, which learn judgements from this file.
|
||||
/// A build that predates the key keeps it as an unknown line and writes
|
||||
/// it back, so an older device passes it on rather than erasing it.
|
||||
pub label: u8,
|
||||
/// The edit itself: `(op, param) -> value`, non-default values only.
|
||||
pub params: BTreeMap<(String, String), f32>,
|
||||
/// TRACES: FR-DEV-3 | FR-NC-9
|
||||
@@ -264,6 +278,7 @@ impl Version {
|
||||
// it in the "not yet looked at" state a cull resumes from.
|
||||
rating: 0,
|
||||
flag: 0,
|
||||
label: 0,
|
||||
params,
|
||||
masks,
|
||||
film,
|
||||
@@ -572,6 +587,10 @@ impl Version {
|
||||
// that never had it.
|
||||
self.rating = merge_judgement(self.rating, remote.rating, remote_wins);
|
||||
self.flag = merge_judgement(self.flag, remote.flag, remote_wins);
|
||||
// TRACES: FR-CAT-5
|
||||
// A label under the same rule: 0 is "none given", so a device that
|
||||
// never labelled a frame cannot clear another's label.
|
||||
self.label = merge_judgement(self.label, remote.label, remote_wins);
|
||||
|
||||
// TRACES: FR-DEV-3f
|
||||
// The film resolves wholesale to the higher revision, like a mask
|
||||
@@ -873,6 +892,9 @@ impl Sidecar {
|
||||
if v.flag > 0 {
|
||||
let _ = writeln!(out, "flag = {}", v.flag);
|
||||
}
|
||||
if v.label > 0 {
|
||||
let _ = writeln!(out, "label = {}", v.label);
|
||||
}
|
||||
// TRACES: FR-DEV-3f
|
||||
// Before the parameters, because it decides what they mean: the
|
||||
// film's exposure slider is a slider on *that stock's* curve.
|
||||
@@ -1068,6 +1090,17 @@ impl Sidecar {
|
||||
Some(value.to_string());
|
||||
}
|
||||
"flag" => version.flag = value.parse::<u8>().unwrap_or(0).min(MAX_FLAG),
|
||||
// TRACES: FR-CAT-5
|
||||
// A code this build does not know reads as none rather than
|
||||
// being clamped onto purple: a wrong colour is a claim, and
|
||||
// no colour is only a gap.
|
||||
"label" => {
|
||||
version.label = value
|
||||
.parse::<u8>()
|
||||
.ok()
|
||||
.filter(|l| *l <= MAX_LABEL)
|
||||
.unwrap_or(0)
|
||||
}
|
||||
// TRACES: FR-DEV-8
|
||||
// Ahead of the `op.param` arm below, which would otherwise try
|
||||
// to read eight numbers as one float and drop the repair with a
|
||||
@@ -2828,6 +2861,37 @@ mod tests {
|
||||
assert_eq!(back.flag, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_label_survives_the_round_trip_and_an_unknown_one_reads_as_none() {
|
||||
// TRACES: FR-CAT-5
|
||||
let mut v = version_of(&EditGraph::default_chain());
|
||||
v.label = 3;
|
||||
let mut sidecar = Sidecar::new();
|
||||
sidecar.put(v);
|
||||
let text = sidecar.to_text();
|
||||
assert!(text.contains("label = 3"), "{text}");
|
||||
let parsed = Sidecar::parse(&text).expect("valid");
|
||||
assert_eq!(parsed.default_version().expect("a version").label, 3);
|
||||
|
||||
let odd = "drsc 1\n\n[version u1]\nname = Default\nrevision = 1\nmodified = 0\n\
|
||||
label = 9\n";
|
||||
let parsed = Sidecar::parse(odd).expect("valid");
|
||||
assert_eq!(parsed.default_version().expect("a version").label, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unlabelled_device_cannot_clear_another_devices_label() {
|
||||
// TRACES: FR-CAT-5
|
||||
let mut local = version_of(&EditGraph::default_chain());
|
||||
local.label = 0;
|
||||
local.revision = 9;
|
||||
let mut remote = version_of(&EditGraph::default_chain());
|
||||
remote.label = 1;
|
||||
remote.revision = 1;
|
||||
local.merge(&remote, None);
|
||||
assert_eq!(local.label, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unrated_image_writes_no_judgement_lines() {
|
||||
// The non-default rule applied to judgement: a library that has never
|
||||
@@ -2838,6 +2902,7 @@ mod tests {
|
||||
let text = sidecar.to_text();
|
||||
assert!(!text.contains("rating"), "{text}");
|
||||
assert!(!text.contains("flag"), "{text}");
|
||||
assert!(!text.contains("label"), "{text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -380,7 +380,14 @@ pub(super) async fn pull_sidecars(
|
||||
continue;
|
||||
};
|
||||
|
||||
match apply_judgement(conn, root_id, path, version.rating, version.flag) {
|
||||
match apply_judgement(
|
||||
conn,
|
||||
root_id,
|
||||
path,
|
||||
version.rating,
|
||||
version.flag,
|
||||
version.label,
|
||||
) {
|
||||
Ok(n) => {
|
||||
applied += n;
|
||||
record_sidecar_read(conn, root_id, path, &entry.validator);
|
||||
@@ -464,6 +471,7 @@ pub(super) fn apply_judgement(
|
||||
sidecar: &str,
|
||||
rating: u8,
|
||||
flag: u8,
|
||||
label: u8,
|
||||
) -> Result<usize, String> {
|
||||
let stem = sidecar
|
||||
.rsplit_once('.')
|
||||
@@ -513,14 +521,25 @@ pub(super) fn apply_judgement(
|
||||
// of caution, as `dr_pipeline::sidecar::merge_judgement`. The cost is
|
||||
// the one that rule always carries — clearing a rating does not
|
||||
// propagate.
|
||||
//
|
||||
// TRACES: FR-CAT-5
|
||||
// The label under the same rule: a sidecar with none leaves this
|
||||
// device's label alone.
|
||||
let changed = conn
|
||||
.execute(
|
||||
"UPDATE versions
|
||||
SET rating = CASE WHEN ?2 > 0 THEN ?2 ELSE rating END,
|
||||
flag = CASE WHEN ?3 > 0 THEN ?3 ELSE flag END
|
||||
flag = CASE WHEN ?3 > 0 THEN ?3 ELSE flag END,
|
||||
label = CASE WHEN ?4 > 0 THEN ?4 ELSE label END
|
||||
WHERE id = ?1
|
||||
AND ((?2 > 0 AND rating <> ?2) OR (?3 > 0 AND flag <> ?3))",
|
||||
rusqlite::params![version, rating.min(5) as i64, flag.min(2) as i64],
|
||||
AND ((?2 > 0 AND rating <> ?2) OR (?3 > 0 AND flag <> ?3)
|
||||
OR (?4 > 0 AND label IS NOT ?4))",
|
||||
rusqlite::params![
|
||||
version,
|
||||
rating.min(5) as i64,
|
||||
flag.min(2) as i64,
|
||||
if label <= 5 { label as i64 } else { 0 }
|
||||
],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
applied += changed;
|
||||
@@ -891,12 +910,27 @@ mod tests {
|
||||
v
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_label_from_another_device_reaches_the_catalog_and_none_clears_nothing() {
|
||||
// TRACES: FR-CAT-5
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
assert_eq!(apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0, 4).unwrap(), 1);
|
||||
let label = |cat: &Catalog| -> Option<i64> {
|
||||
cat.connection()
|
||||
.query_row("SELECT label FROM versions WHERE is_default = 1", [], |r| r.get(0))
|
||||
.unwrap()
|
||||
};
|
||||
assert_eq!(label(&cat), Some(4));
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0, 0).unwrap();
|
||||
assert_eq!(label(&cat), Some(4), "a sidecar with no label left it alone");
|
||||
}
|
||||
|
||||
/// The regression, in one line: a rating in a sidecar reaches the grid.
|
||||
/// Before this there was no path by which it could.
|
||||
#[test]
|
||||
fn a_rating_from_another_device_reaches_the_catalog() {
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap();
|
||||
let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap();
|
||||
|
||||
assert_eq!(n, 1);
|
||||
assert_eq!(judgements(&cat), vec![("2026/a.CR2".to_string(), 4, 1)]);
|
||||
@@ -908,7 +942,7 @@ mod tests {
|
||||
#[test]
|
||||
fn both_halves_of_a_raw_and_jpeg_pair_are_judged() {
|
||||
let cat = library(&["2026/a.CR2", "2026/a.jpg"]);
|
||||
let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0).unwrap();
|
||||
let n = apply_judgement(cat.connection(), 1, "2026/a.drsc", 3, 0, 0).unwrap();
|
||||
|
||||
assert_eq!(n, 2);
|
||||
assert_eq!(
|
||||
@@ -926,8 +960,8 @@ mod tests {
|
||||
#[test]
|
||||
fn a_lowered_rating_travels() {
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 0).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 1, 0).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 0, 0).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 1, 0, 0).unwrap();
|
||||
|
||||
assert_eq!(judgements(&cat)[0].1, 1);
|
||||
}
|
||||
@@ -937,10 +971,10 @@ mod tests {
|
||||
#[test]
|
||||
fn an_unjudged_sidecar_does_not_erase_a_local_rating() {
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 2).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 2, 0).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0).unwrap(),
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 0, 0, 0).unwrap(),
|
||||
0
|
||||
);
|
||||
assert_eq!(judgements(&cat), vec![("2026/a.CR2".to_string(), 5, 2)]);
|
||||
@@ -953,11 +987,11 @@ mod tests {
|
||||
fn applying_the_same_judgement_twice_changes_nothing() {
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
assert_eq!(
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap(),
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1).unwrap(),
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 4, 1, 0).unwrap(),
|
||||
0
|
||||
);
|
||||
}
|
||||
@@ -970,7 +1004,7 @@ mod tests {
|
||||
// `_` is LIKE's single-character wildcard, so an unescaped `a_b` stem
|
||||
// would also match `axb`.
|
||||
let cat = library(&["2026/a_b.CR2", "2026/axb.CR2"]);
|
||||
apply_judgement(cat.connection(), 1, "2026/a_b.drsc", 5, 0).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a_b.drsc", 5, 0, 0).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
judgements(&cat),
|
||||
@@ -986,7 +1020,7 @@ mod tests {
|
||||
#[test]
|
||||
fn a_shared_prefix_is_not_a_shared_sidecar() {
|
||||
let cat = library(&["2026/a.CR2", "2026/ab.CR2"]);
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 0).unwrap();
|
||||
apply_judgement(cat.connection(), 1, "2026/a.drsc", 5, 0, 0).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
judgements(&cat),
|
||||
@@ -1004,7 +1038,7 @@ mod tests {
|
||||
fn a_sidecar_with_no_photograph_here_is_not_a_failure() {
|
||||
let cat = library(&["2026/a.CR2"]);
|
||||
assert_eq!(
|
||||
apply_judgement(cat.connection(), 1, "2026/elsewhere.drsc", 5, 0).unwrap(),
|
||||
apply_judgement(cat.connection(), 1, "2026/elsewhere.drsc", 5, 0, 0).unwrap(),
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
@@ -34,8 +34,8 @@ pub struct SidecarWrite {
|
||||
/// amendment rather than a replacement.
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum Amendment {
|
||||
/// A star rating and a pick/reject flag — the cull.
|
||||
Judgement { rating: u8, flag: u8 },
|
||||
/// A star rating, a pick/reject flag and a colour label — the cull.
|
||||
Judgement { rating: u8, flag: u8, label: u8 },
|
||||
/// TRACES: FR-DEV-6
|
||||
/// Copied develop settings, applied within `scope`.
|
||||
///
|
||||
@@ -302,9 +302,14 @@ pub(super) fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline
|
||||
// paste must spare, the unknown keys of an operation this build lacks —
|
||||
// survives because it was read from the file and is written back.
|
||||
match &w.amendment {
|
||||
Amendment::Judgement { rating, flag } => {
|
||||
Amendment::Judgement {
|
||||
rating,
|
||||
flag,
|
||||
label,
|
||||
} => {
|
||||
version.rating = *rating;
|
||||
version.flag = *flag;
|
||||
version.label = *label;
|
||||
}
|
||||
Amendment::Settings {
|
||||
preset,
|
||||
@@ -624,7 +629,11 @@ mod tests {
|
||||
SidecarWrite {
|
||||
image_path: "PhotosRaw/incoming/a.CR2".to_string(),
|
||||
version_uuid: uuid.to_string(),
|
||||
amendment: Amendment::Judgement { rating, flag: 0 },
|
||||
amendment: Amendment::Judgement {
|
||||
rating,
|
||||
flag: 0,
|
||||
label: 0,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -492,7 +492,7 @@ fn collect_sidecar_writes(
|
||||
.collect::<Vec<_>>()
|
||||
.join(",");
|
||||
let sql = format!(
|
||||
"SELECT i.source_ref, v.uuid, v.rating, v.flag
|
||||
"SELECT i.source_ref, v.uuid, v.rating, v.flag, coalesce(v.label, 0)
|
||||
FROM images i
|
||||
JOIN versions v ON v.image_id = i.id AND v.is_default = 1
|
||||
WHERE i.id IN ({placeholders})"
|
||||
@@ -512,6 +512,7 @@ fn collect_sidecar_writes(
|
||||
amendment: library::Amendment::Judgement {
|
||||
rating: r.get::<_, i64>(2)? as u8,
|
||||
flag: r.get::<_, i64>(3)? as u8,
|
||||
label: r.get::<_, i64>(4)?.clamp(0, 5) as u8,
|
||||
},
|
||||
})
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user