diff options
| author | Paul Buetow <paul@buetow.org> | 2026-06-17 21:35:21 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-06-17 21:35:21 +0300 |
| commit | 25aff490d739290602b28bdc53936dd3ea6ed939 (patch) | |
| tree | db6b521d3e5281d5edec1aa8588b48192e3a0f4d | |
| parent | 6109e2a6f5bc45c3c4080259bd09a1e3dc48ea4d (diff) | |
mn0 share camera Make+Model dedup helper
The rule that joins a camera's EXIF Make + Model into one label while
avoiding a duplicated manufacturer prefix (e.g. "Canon Canon EOS 5D" ->
"Canon EOS 5D") was implemented independently in the album tooltip builder
and the stats leaderboard tally. Extract it into a single shared helper
camera_label_from_make_model in the new src/lib/metadata-label.source.sh,
sourced before both callers.
Both prior implementations were behavior-identical (empty model -> make,
empty make -> model, exact/prefix dedup, case-sensitive), so this is a pure
DRY refactor with no observable output change. Added a focused unit test
covering dedup, plain concatenation and the empty-field edge cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| -rw-r--r-- | Justfile | 2 | ||||
| -rwxr-xr-x | bin/shuriken | 76 | ||||
| -rw-r--r-- | src/lib/album-metadata.source.sh | 18 | ||||
| -rw-r--r-- | src/lib/metadata-label.source.sh | 31 | ||||
| -rw-r--r-- | src/lib/stats-aggregate.source.sh | 25 | ||||
| -rwxr-xr-x | tests/cli.sh | 42 |
6 files changed, 121 insertions, 73 deletions
@@ -7,7 +7,7 @@ PREFIX := env_var_or_default("PREFIX", "/usr") BINDIR := env_var_or_default("BINDIR", PREFIX + "/bin") DATADIR := env_var_or_default("DATADIR", PREFIX + "/share") SYSCONFDIR := env_var_or_default("SYSCONFDIR", "/etc/default") -LIB_SOURCES := "src/lib/logging.source.sh src/lib/bootstrap.source.sh src/lib/paths.source.sh src/lib/imagemagick.source.sh src/lib/process.source.sh src/lib/archive.source.sh src/lib/template.source.sh src/lib/job-pool.source.sh src/lib/image.source.sh src/lib/random.source.sh src/lib/image-pipeline.source.sh src/lib/album-metadata.source.sh src/lib/album-render.source.sh src/lib/album.source.sh src/lib/stats-aggregate.source.sh src/lib/stats-render.source.sh src/lib/stats-filter-album.source.sh src/lib/config.source.sh src/lib/config.print.source.sh src/lib/config.sync.source.sh src/lib/config.staging.source.sh src/lib/config.validate.source.sh src/lib/config.cli.source.sh src/lib/action.source.sh" +LIB_SOURCES := "src/lib/logging.source.sh src/lib/bootstrap.source.sh src/lib/paths.source.sh src/lib/imagemagick.source.sh src/lib/process.source.sh src/lib/archive.source.sh src/lib/template.source.sh src/lib/job-pool.source.sh src/lib/image.source.sh src/lib/random.source.sh src/lib/metadata-label.source.sh src/lib/image-pipeline.source.sh src/lib/album-metadata.source.sh src/lib/album-render.source.sh src/lib/album.source.sh src/lib/stats-aggregate.source.sh src/lib/stats-render.source.sh src/lib/stats-filter-album.source.sh src/lib/config.source.sh src/lib/config.print.source.sh src/lib/config.sync.source.sh src/lib/config.staging.source.sh src/lib/config.validate.source.sh src/lib/config.cli.source.sh src/lib/action.source.sh" default: build diff --git a/bin/shuriken b/bin/shuriken index 979f06a..3025da3 100755 --- a/bin/shuriken +++ b/bin/shuriken @@ -1666,6 +1666,39 @@ maybe_shuffle() { fi } +# Inlined from src/lib/metadata-label.source.sh +# Shared EXIF metadata label helpers. Extracted (task mn0) so the rule for +# turning a camera's Make + Model into one human-readable label lives in a +# single place instead of being duplicated in album-metadata.source.sh's +# tooltip builder and stats-aggregate.source.sh's leaderboard tally. This file +# is sourced before both callers (see LIB_SOURCES in the Justfile). All library +# modules are sourced before any code runs, so definition order only documents +# the dependency, it does not affect availability. + +# Join a camera's EXIF Make + Model into one label, avoiding a duplicated +# manufacturer prefix. Many cameras already repeat the make inside the model +# (e.g. Make="Canon", Model="Canon EOS 5D"), so when the model equals the make +# or starts with "<make> " we keep the model alone ("Canon EOS 5D" rather than +# "Canon Canon EOS 5D"). Either field may be empty: an empty model yields the +# make, an empty make yields the model, and both empty yields an empty string. +camera_label_from_make_model() { + local -r make="$1"; shift + local -r model="$1"; shift + + if [ -z "$model" ]; then + printf '%s' "$make" + return + fi + if [ -z "$make" ]; then + printf '%s' "$model" + return + fi + case "$model" in + "$make"|"$make "*) printf '%s' "$model" ;; + *) printf '%s %s' "$make" "$model" ;; + esac +} + # Inlined from src/lib/image-pipeline.source.sh create_photo_derivatives() { local -r photos_dir="$1"; shift @@ -1924,21 +1957,9 @@ _photo_exif_tooltip_text_from_values() { make="${values_ref[Make]:-}" model="${values_ref[Model]:-}" - camera="$make" - if [ -n "$model" ]; then - if [ -n "$make" ]; then - case "$model" in - "$make"|"$make "*) - camera="$model" - ;; - *) - camera="$make $model" - ;; - esac - else - camera="$model" - fi - fi + # Dedup the manufacturer prefix via the shared helper (task mn0) so this + # tooltip and the stats leaderboard derive identical camera labels. + camera=$(camera_label_from_make_model "$make" "$model") _first_exif_value_to aperture "$exif_name" FNumber ApertureValue _first_exif_value_to iso "$exif_name" \ @@ -3490,34 +3511,17 @@ _stats_slug() { printf '%s' "$slug" } -# Join Make + Model into a single camera label, reusing the same dedup logic as -# album.source.sh's tooltip builder (handles "Model already includes Make"). -_stats_camera_label() { - local -r make="$1"; shift - local -r model="$1"; shift - - if [ -z "$model" ]; then - printf '%s' "$make" - return - fi - if [ -z "$make" ]; then - printf '%s' "$model" - return - fi - case "$model" in - "$make"|"$make "*) printf '%s' "$model" ;; - *) printf '%s %s' "$make" "$model" ;; - esac -} - # Tally a photo's camera (Make+Model) and, when present, its lens into their # leaderboard counts and filter mini-albums. Skips photos with no Make/Model. +# The Make+Model dedup now lives in camera_label_from_make_model +# (metadata-label.source.sh, task mn0), shared with the album tooltip builder. _stats_record_camera() { local -n values_ref="$1"; shift local -r photo="$1"; shift local label - label=$(_stats_camera_label "${values_ref[Make]:-}" "${values_ref[Model]:-}") + label=$(camera_label_from_make_model \ + "${values_ref[Make]:-}" "${values_ref[Model]:-}") _stats_tally STATS_CAMERAS camera "$label" "$label" "$photo" if [ -n "${values_ref[LensModel]:-}" ]; then _stats_tally STATS_LENSES lens \ diff --git a/src/lib/album-metadata.source.sh b/src/lib/album-metadata.source.sh index 2078a85..cc500ae 100644 --- a/src/lib/album-metadata.source.sh +++ b/src/lib/album-metadata.source.sh @@ -169,21 +169,9 @@ _photo_exif_tooltip_text_from_values() { make="${values_ref[Make]:-}" model="${values_ref[Model]:-}" - camera="$make" - if [ -n "$model" ]; then - if [ -n "$make" ]; then - case "$model" in - "$make"|"$make "*) - camera="$model" - ;; - *) - camera="$make $model" - ;; - esac - else - camera="$model" - fi - fi + # Dedup the manufacturer prefix via the shared helper (task mn0) so this + # tooltip and the stats leaderboard derive identical camera labels. + camera=$(camera_label_from_make_model "$make" "$model") _first_exif_value_to aperture "$exif_name" FNumber ApertureValue _first_exif_value_to iso "$exif_name" \ diff --git a/src/lib/metadata-label.source.sh b/src/lib/metadata-label.source.sh new file mode 100644 index 0000000..4d36b3c --- /dev/null +++ b/src/lib/metadata-label.source.sh @@ -0,0 +1,31 @@ +# Shared EXIF metadata label helpers. Extracted (task mn0) so the rule for +# turning a camera's Make + Model into one human-readable label lives in a +# single place instead of being duplicated in album-metadata.source.sh's +# tooltip builder and stats-aggregate.source.sh's leaderboard tally. This file +# is sourced before both callers (see LIB_SOURCES in the Justfile). All library +# modules are sourced before any code runs, so definition order only documents +# the dependency, it does not affect availability. + +# Join a camera's EXIF Make + Model into one label, avoiding a duplicated +# manufacturer prefix. Many cameras already repeat the make inside the model +# (e.g. Make="Canon", Model="Canon EOS 5D"), so when the model equals the make +# or starts with "<make> " we keep the model alone ("Canon EOS 5D" rather than +# "Canon Canon EOS 5D"). Either field may be empty: an empty model yields the +# make, an empty make yields the model, and both empty yields an empty string. +camera_label_from_make_model() { + local -r make="$1"; shift + local -r model="$1"; shift + + if [ -z "$model" ]; then + printf '%s' "$make" + return + fi + if [ -z "$make" ]; then + printf '%s' "$model" + return + fi + case "$model" in + "$make"|"$make "*) printf '%s' "$model" ;; + *) printf '%s %s' "$make" "$model" ;; + esac +} diff --git a/src/lib/stats-aggregate.source.sh b/src/lib/stats-aggregate.source.sh index ebb75cf..46592c4 100644 --- a/src/lib/stats-aggregate.source.sh +++ b/src/lib/stats-aggregate.source.sh @@ -300,34 +300,17 @@ _stats_slug() { printf '%s' "$slug" } -# Join Make + Model into a single camera label, reusing the same dedup logic as -# album.source.sh's tooltip builder (handles "Model already includes Make"). -_stats_camera_label() { - local -r make="$1"; shift - local -r model="$1"; shift - - if [ -z "$model" ]; then - printf '%s' "$make" - return - fi - if [ -z "$make" ]; then - printf '%s' "$model" - return - fi - case "$model" in - "$make"|"$make "*) printf '%s' "$model" ;; - *) printf '%s %s' "$make" "$model" ;; - esac -} - # Tally a photo's camera (Make+Model) and, when present, its lens into their # leaderboard counts and filter mini-albums. Skips photos with no Make/Model. +# The Make+Model dedup now lives in camera_label_from_make_model +# (metadata-label.source.sh, task mn0), shared with the album tooltip builder. _stats_record_camera() { local -n values_ref="$1"; shift local -r photo="$1"; shift local label - label=$(_stats_camera_label "${values_ref[Make]:-}" "${values_ref[Model]:-}") + label=$(camera_label_from_make_model \ + "${values_ref[Make]:-}" "${values_ref[Model]:-}") _stats_tally STATS_CAMERAS camera "$label" "$label" "$photo" if [ -n "${values_ref[LensModel]:-}" ]; then _stats_tally STATS_LENSES lens \ diff --git a/tests/cli.sh b/tests/cli.sh index 2ca65ce..5a58384 100755 --- a/tests/cli.sh +++ b/tests/cli.sh @@ -5862,6 +5862,45 @@ test_stats_collect_reads_cached_identify_output() { test::teardown } +# Unit-test the shared Make+Model dedup helper (task mn0). The album tooltip and +# stats leaderboard both rely on this, so cover dedup, plain concatenation and +# the empty-field edge cases here in one place. +test_camera_label_from_make_model() { + local actual + + # shellcheck source=src/lib/metadata-label.source.sh + source "$TEST_REPO_ROOT/src/lib/metadata-label.source.sh" + + _assert_camera_label() { + local -r expected="$1"; shift + local -r make="$1"; shift + local -r model="$1"; shift + + actual=$(camera_label_from_make_model "$make" "$model") + if [ "$actual" != "$expected" ]; then + printf 'FAIL: camera_label_from_make_model %q %q => %q, want %q\n' \ + "$make" "$model" "$actual" "$expected" >&2 + exit 1 + fi + } + + # Model repeats the make as a prefix: dedup to the model alone. + _assert_camera_label 'Canon EOS 5D' 'Canon' 'Canon EOS 5D' + # Model equals the make exactly: still just the model. + _assert_camera_label 'Canon' 'Canon' 'Canon' + # No duplication: make and model are concatenated. + _assert_camera_label 'NIKON CORPORATION Z 6' 'NIKON CORPORATION' 'Z 6' + # A make that is a substring but not a prefix is not deduped. + _assert_camera_label 'Canon PowerShot Canon' 'Canon' 'PowerShot Canon' + # Empty model yields the make; empty make yields the model. + _assert_camera_label 'Apple' 'Apple' '' + _assert_camera_label 'iPhone 12' '' 'iPhone 12' + # Both empty yields an empty label. + _assert_camera_label '' '' '' + # The prefix match is case-sensitive: differing case is not deduped. + _assert_camera_label 'canon Canon EOS 5D' 'canon' 'Canon EOS 5D' +} + main() { trap test::teardown EXIT @@ -6200,6 +6239,9 @@ main() { 'stats collect reads cached identify output' \ test_stats_collect_reads_cached_identify_output test::run_case \ + 'camera label dedups make and model' \ + test_camera_label_from_make_model + test::run_case \ '--generate metadata escapes JSON and custom tarball suffix' \ test_generate_metadata_escapes_json_and_custom_tarball_suffix test::run_case \ |
