diff --git a/internal/library/genre.go b/internal/library/genre.go index 1ea2f902..2c903f13 100644 --- a/internal/library/genre.go +++ b/internal/library/genre.go @@ -89,7 +89,7 @@ func normaliseGenreValue(v string) []string { for strings.HasPrefix(v, "(") { // "((" is the spec's escape for a literal "(" — the rest is plain text. if strings.HasPrefix(v, "((") { - return append(out, strings.TrimSpace(v[1:])) + return append(out, trueUpCasing(strings.TrimSpace(v[1:]))) } end := strings.IndexByte(v, ')') if end < 0 { @@ -106,7 +106,7 @@ func normaliseGenreValue(v string) []string { if err != nil { // Parenthesised but not a reference, e.g. "(Live)". Keep the // whole remainder as written. - return append(out, v) + return append(out, trueUpCasing(v)) } if name, ok := id3v1GenreName(n); ok { out = append(out, name) @@ -120,11 +120,104 @@ func normaliseGenreValue(v string) []string { } if n, err := strconv.Atoi(v); err == nil { if name, ok := id3v1GenreName(n); ok { + // Canonical table name, deliberately NOT re-cased — see id3v1Genres. return append(out, name) } return out } - return append(out, v) + return append(out, trueUpCasing(v)) +} + +// genreAcronyms are tokens that belong in caps. Tag editors that title-case the +// whole genre field turn "EDM" into "Edm" and "UK Garage" into "Uk Garage", and +// the operator's library carries all of these (#2468). +// +// Matched case-INSENSITIVELY, so "edm", "Edm" and "EDM" all land on "EDM". That +// is a deliberate, narrow exception to the project's rule that genre case is +// exposed as the file says it: "Rock" and "rock" still stay separate rows, +// because folding those would be a judgement about labels. Fixing a token that +// is unambiguously an initialism is not — there is no genre named "Edm". +// +// Kept short and evidence-led. Add a token here only when a real library shows +// it damaged; a speculative list risks mangling a word that legitimately looks +// like an acronym. +var genreAcronyms = map[string]string{ + "edm": "EDM", + "idm": "IDM", + "aor": "AOR", + "uk": "UK", + "us": "US", + "ebm": "EBM", +} + +// contractionSuffixes are the word-endings that follow an apostrophe in normal +// English. Title-casing capitalises the letter after ANY non-letter, which is +// how "Children's Music" became "Children'S Music". +// +// Deliberately a fixed list rather than "lowercase whatever follows an +// apostrophe": that broader rule would break "O'Brien" and "D'Angelo", which are +// correctly capitalised after the apostrophe. +var contractionSuffixes = map[string]bool{ + "s": true, "t": true, "re": true, "ll": true, "ve": true, "d": true, "m": true, +} + +// trueUpCasing repairs casing damage done by tag editors that title-case the +// genre field. It only ever changes case, never the letters — so it cannot +// silently turn one genre into a different one, which is what separates it from +// the label-remapping idea #2468 rejected. +func trueUpCasing(s string) string { + if s == "" { + return s + } + words := strings.Split(s, " ") + for i, w := range words { + if w == "" { + continue + } + // Match the word's letter core, not the raw word, so surrounding + // punctuation doesn't hide the acronym: "(Edm)" and "Edm," both need + // fixing, and re-attaching the trimmed edges keeps them intact. + lead, core, trail := splitWordCore(w) + if fixed, ok := genreAcronyms[strings.ToLower(core)]; ok { + words[i] = lead + fixed + trail + continue + } + words[i] = fixApostrophe(w) + } + return strings.Join(words, " ") +} + +// splitWordCore peels leading and trailing non-alphanumerics off a word. +// Interior punctuation stays in the core, so "Lo-Fi" and "R&B" are compared +// whole rather than being split into fragments that might match by accident. +func splitWordCore(w string) (lead, core, trail string) { + isCore := func(r rune) bool { + return (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') + } + start := 0 + for start < len(w) && !isCore(rune(w[start])) { + start++ + } + end := len(w) + for end > start && !isCore(rune(w[end-1])) { + end-- + } + return w[:start], w[start:end], w[end:] +} + +// fixApostrophe lowercases a capitalised contraction or possessive suffix: +// "Children'S" -> "Children's". Leaves "O'Brien" alone, since "Brien" is not a +// contraction suffix. +func fixApostrophe(w string) string { + idx := strings.LastIndexByte(w, '\'') + if idx <= 0 || idx == len(w)-1 { + return w + } + suffix := w[idx+1:] + if !contractionSuffixes[strings.ToLower(suffix)] { + return w + } + return w[:idx+1] + strings.ToLower(suffix) } func id3v1GenreName(n int) (string, bool) { diff --git a/internal/library/genre_test.go b/internal/library/genre_test.go index 0fa27b59..9be5da86 100644 --- a/internal/library/genre_test.go +++ b/internal/library/genre_test.go @@ -419,3 +419,102 @@ func equalStrings(a, b []string) bool { } return true } + +func TestTrueUpCasing(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + // The damage actually present in the operator's library (#2468). + {"edm acronym", "Edm", "EDM"}, + {"idm acronym", "Idm", "IDM"}, + {"aor acronym", "Aor", "AOR"}, + {"uk prefix", "Uk Garage", "UK Garage"}, + {"uk hardcore", "Uk Hardcore", "UK Hardcore"}, + {"acronym mid-phrase", "Trap Edm", "Trap EDM"}, + {"acronym at the end", "Glitch Hop Edm", "Glitch Hop EDM"}, + {"possessive", "Children'S Music", "Children's Music"}, + + // Already correct input must be left exactly alone. + {"correct acronym", "EDM", "EDM"}, + {"correct possessive", "Children's Music", "Children's Music"}, + + // Case-insensitive, so a lower-cased tag also lands on the canonical + // form rather than becoming a third variant. + {"lowercase acronym", "edm", "EDM"}, + + // Names with an apostrophe followed by a real word are NOT contractions + // and must keep their capital — this is why the suffix list is fixed + // rather than "lowercase anything after an apostrophe". + {"irish surname", "O'Brien Core", "O'Brien Core"}, + {"french elision", "D'Angelo Soul", "D'Angelo Soul"}, + + // Ordinary genres pass through untouched. Case is otherwise exposed as + // the file says it — "Rock" vs "rock" stays a real distinction. + {"plain", "Alternative Rock", "Alternative Rock"}, + {"lowercase plain", "rock", "rock"}, + {"hyphenated", "Lo-Fi Hip Hop", "Lo-Fi Hip Hop"}, + {"ampersand", "R&B", "R&B"}, + {"empty", "", ""}, + + // Punctuation around a word must not hide the acronym inside it. + {"parenthesised acronym", "Hip Hop (Edm)", "Hip Hop (EDM)"}, + {"acronym with comma", "Edm, Trap", "EDM, Trap"}, + + // Interior punctuation stays in the core, so these are compared whole + // and cannot match a fragment by accident. + {"hyphenated stays whole", "Lo-Fi", "Lo-Fi"}, + {"ampersand stays whole", "Drum & Bass", "Drum & Bass"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := trueUpCasing(tc.in); got != tc.want { + t.Errorf("trueUpCasing(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +// The casing repair must never touch a name resolved from the ID3v1 table. The +// operator chose to keep that table canonical, so entry 40's 1990s spelling +// "AlternRock" stays as-is even though it reads like damage. +func TestNormaliseGenreValue_CanonicalTableNamesNotRecased(t *testing.T) { + if got := normaliseGenreValue("40"); !equalStrings(got, []string{"AlternRock"}) { + t.Errorf("bare 40 = %q, want [AlternRock]", got) + } + if got := normaliseGenreValue("(40)"); !equalStrings(got, []string{"AlternRock"}) { + t.Errorf("(40) = %q, want [AlternRock]", got) + } +} + +// Casing runs on values that came from the file, including the parenthesised +// and refinement paths. +func TestNormaliseGenreValue_CasingAppliedToFileText(t *testing.T) { + tests := []struct { + in string + want []string + }{ + {"Edm", []string{"EDM"}}, + {"(17)Uk Garage", []string{"Rock", "UK Garage"}}, + // Punctuation around the acronym must not hide it — the letter core is + // what gets matched, and the trimmed edges are re-attached. + {"(Live Edm)", []string{"(Live EDM)"}}, + {"((Children'S Music", []string{"(Children's Music"}}, + } + for _, tc := range tests { + t.Run(tc.in, func(t *testing.T) { + if got := normaliseGenreValue(tc.in); !equalStrings(got, tc.want) { + t.Errorf("normaliseGenreValue(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +// Two spellings of one acronym in the same file collapse to a single tag rather +// than surviving as near-duplicates. +func TestNormaliseGenres_AcronymVariantsDedupe(t *testing.T) { + if got := normaliseGenres([]string{"Edm", "EDM", "edm"}); !equalStrings(got, []string{"EDM"}) { + t.Errorf("genres = %q, want [EDM]", got) + } +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 6d78fc20..634c45a4 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -52,7 +52,11 @@ var audioExtensions = map[string]bool{ // ("Alternative Rock" + "Rock" -> "Alternative RockRock"), which corrupted // the genre browse axis and polluted the taste profile's tag vocabulary, // and left bare ID3v1 numeric references unresolved (#2499). -const tagReadVersion int16 = 1 +// 2: acronym and apostrophe casing repaired on genre values (#2468) — "Edm" -> +// "EDM", "Children'S Music" -> "Children's Music". Bumped rather than left +// to new files only because taste_profile.sql reads tracks.genre directly, +// so a half-repaired library would carry both spellings as separate tags. +const tagReadVersion int16 = 2 type Stats struct { Scanned int `json:"scanned"`