From 0ae4e85bd2f80747171eb6eb9f5a37e2b84b54d7 Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 22 Sep 2026 17:55:02 -0700 Subject: [PATCH] Stop warning about slave-only content in layout checks A singles library naturally has artists and songs the albums library lacks, so "only N of M top-level folders exist in the master" and "N of M slave files have no counterpart" were false alarms. Layout warnings now only flag real layout problems: - Folders that are the same but named differently (case, punctuation, accents, "&"/"and", leading/trailing "The"), at the top level and one level down inside shared folders, with examples. - Songs found in both libraries that mostly live in a different folder in the master (e.g. Artist/Song vs Artist/Album/Song). Slave-only songs are ignored, as are singles/EPs/compilations matched to their album version. Co-Authored-By: Claude Opus 5.5 --- internal/engine/engine.go | 43 +++------- internal/engine/layout.go | 148 +++++++++++++++++++++++++++++++++ internal/engine/layout_test.go | 104 +++++++++++++++++++++++ internal/engine/scan.go | 11 +-- 4 files changed, 269 insertions(+), 37 deletions(-) create mode 100644 internal/engine/layout.go create mode 100644 internal/engine/layout_test.go diff --git a/internal/engine/engine.go b/internal/engine/engine.go index a97edf0..e39efc0 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -298,43 +298,26 @@ func (e *Engine) NormalizePair(p *store.Pair) ([]string, error) { return layoutWarnings(l), nil } -// layoutWarnings does a cheap top-level comparison of the two libraries. -// Any folder layout works; what matters is that both libraries use the same -// one, so matching files sit at the same relative path. +// layoutWarnings is the quick check run when a pair is saved and at the +// start of each scan. Content that exists in only one library is normal and +// never warned about; only folders that are the same but named differently are. func layoutWarnings(l *libs) []string { - var warns []string - list := func(root string) (dirs map[string]bool, files int) { - dirs = map[string]bool{} - ents, _ := os.ReadDir(root) + empty := true + if ents, _ := os.ReadDir(l.slave.Path); len(ents) > 0 { for _, en := range ents { - if strings.HasPrefix(en.Name(), ".") { - continue - } - if en.IsDir() { - dirs[strings.ToLower(en.Name())] = true - } else if l.hasExt(en.Name()) { - files++ + if !strings.HasPrefix(en.Name(), ".") && (en.IsDir() || l.hasExt(en.Name())) { + empty = false + break } } - return } - md, _ := list(l.master.Path) - sd, sf := list(l.slave.Path) - if len(sd) == 0 && sf == 0 { - warns = append(warns, "The slave library is empty. Use Build to pick songs from the master library to hardlink into it.") - return warns + if empty { + return []string{"The slave library is empty. Use Build to pick songs from the master library to hardlink into it."} } - shared := 0 - for d := range sd { - if md[d] { - shared++ - } + if w := renamedWarning(renamedFolders(l)); w != "" { + return []string{w} } - if len(sd) > 0 && shared*2 < len(sd) { - warns = append(warns, fmt.Sprintf("Only %d of %d top-level folders in the slave library also exist in the master library. "+ - "Both libraries should use the same folder layout, or most files will not match.", shared, len(sd))) - } - return warns + return nil } // ---- helpers ---- diff --git a/internal/engine/layout.go b/internal/engine/layout.go new file mode 100644 index 0000000..534b389 --- /dev/null +++ b/internal/engine/layout.go @@ -0,0 +1,148 @@ +package engine + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/cdnutter/playlink/internal/store" +) + +// Layout warnings flag real layout problems only. Content that exists in just +// one library is normal (a singles library naturally has artists and songs +// the albums library lacks), so it is never counted against the layout. + +var accentFold = strings.NewReplacer( + "á", "a", "à", "a", "â", "a", "ä", "a", "ã", "a", "å", "a", "é", "e", "è", "e", "ê", "e", "ë", "e", + "í", "i", "ì", "i", "î", "i", "ï", "i", "ó", "o", "ò", "o", "ô", "o", "ö", "o", "õ", "o", "ø", "o", + "ú", "u", "ù", "u", "û", "u", "ü", "u", "ñ", "n", "ç", "c", "ý", "y", "ÿ", "y", "œ", "oe", "æ", "ae", "ß", "ss", +) + +// folderKey identifies a folder name regardless of case, punctuation, accents, +// "&" vs "and" and a leading/trailing "The": "AC/DC", "AC-DC" and "ACDC", or +// "The Beatles" and "Beatles, The", get the same key. +func folderKey(name string) string { + s := accentFold.Replace(strings.ToLower(strings.TrimSpace(name))) + s = strings.ReplaceAll(s, "&", " and ") + s = strings.TrimPrefix(s, "the ") + s = strings.TrimSuffix(s, ", the") + var b strings.Builder + for _, w := range words(s) { + b.WriteString(w) + } + return b.String() +} + +func subdirs(dir string) []string { + ents, _ := os.ReadDir(dir) + var out []string + for _, en := range ents { + if en.IsDir() && !strings.HasPrefix(en.Name(), ".") { + out = append(out, en.Name()) + } + } + return out +} + +// renamedFolders finds slave folders that are the same folder as a master +// folder under a different name. It checks top-level folders and, for +// top-level folders both libraries share, the folders inside them. +func renamedFolders(l *libs) [][2]string { + var out [][2]string + compare := func(prefix string, master, slave []string) { + exact := map[string]bool{} + byKey := map[string]string{} + for _, m := range master { + exact[m] = true + if k := folderKey(m); k != "" { + byKey[k] = m + } + } + for _, s := range slave { + if exact[s] { + continue + } + if m, ok := byKey[folderKey(s)]; ok { + out = append(out, [2]string{filepath.Join(prefix, s), filepath.Join(prefix, m)}) + } + } + } + mTop, sTop := subdirs(l.master.Path), subdirs(l.slave.Path) + compare("", mTop, sTop) + shared := map[string]bool{} + for _, m := range mTop { + shared[m] = true + } + for _, s := range sTop { + if shared[s] { + compare(s, subdirs(filepath.Join(l.master.Path, s)), subdirs(filepath.Join(l.slave.Path, s))) + } + } + sort.Slice(out, func(i, j int) bool { return out[i][0] < out[j][0] }) + return out +} + +func renamedWarning(pairs [][2]string) string { + if len(pairs) == 0 { + return "" + } + var ex []string + for _, p := range pairs[:min(3, len(pairs))] { + ex = append(ex, fmt.Sprintf("%q (slave) vs %q (master)", p[0], p[1])) + } + more := "" + if len(pairs) > 3 { + more = fmt.Sprintf(" and %d more", len(pairs)-3) + } + noun := "folder is" + if len(pairs) > 1 { + noun = "folders are" + } + return fmt.Sprintf("%d %s named differently in the two libraries: %s%s. Songs inside them only match by path "+ + "once the names agree.", len(pairs), noun, strings.Join(ex, ", "), more) +} + +// layoutStats watches, during a scan, where songs found in both libraries +// live. Songs only in the slave library are ignored. +type layoutStats struct { + shared int // slave files with a master counterpart + moved int // ...that live in a different folder in the master + example [][2]string // a few moved examples +} + +func (ls *layoutStats) add(relSlave, relMaster, kind, reason string) { + if kind == store.KindNone || relMaster == "" { + return + } + ls.shared++ + if kind != store.KindElsewhere && kind != store.KindApprox { + return + } + if albumKey(relSlave) == albumKey(relMaster) { + return // only a disc folder differs + } + // A single, EP or compilation matched to its album version is the + // intended behaviour, not a layout problem. + if st := parseTitle(relSlave); st.single || st.compilation || strings.Contains(reason, "album version preferred") { + return + } + ls.moved++ + if len(ls.example) < 2 { + ls.example = append(ls.example, [2]string{relSlave, relMaster}) + } +} + +func (ls *layoutStats) warning() string { + if ls.shared < 20 || ls.moved*4 <= ls.shared { + return "" + } + var ex []string + for _, e := range ls.example { + ex = append(ex, fmt.Sprintf("%q is at %q", e[0], e[1])) + } + return fmt.Sprintf("Of %d songs found in both libraries, %d are in a different folder in the master library (for example %s). "+ + "The libraries seem to use different folder layouts, so these songs only match through the fallbacks.", + ls.shared, ls.moved, strings.Join(ex, "; ")) +} diff --git a/internal/engine/layout_test.go b/internal/engine/layout_test.go new file mode 100644 index 0000000..808807a --- /dev/null +++ b/internal/engine/layout_test.go @@ -0,0 +1,104 @@ +package engine + +import ( + "fmt" + "strings" + "testing" +) + +func scanWarnings(t *testing.T, ev *env) []string { + t.Helper() + ev.scan() + p, err := ev.st.Pair(ctx, ev.pair.ID) + if err != nil { + t.Fatal(err) + } + var out []string + for _, w := range p.LastScanWarnings { + if !strings.Contains(w, "ffprobe") { + out = append(out, w) + } + } + return out +} + +// The user's situation: a singles library with far more artists and songs +// than the albums library. Slave-only content must not trigger warnings. +func TestSlaveOnlyContentIsNotALayoutProblem(t *testing.T) { + ev := setup(t, false) + for i := 0; i < 40; i++ { + ev.write(ev.slave, fmt.Sprintf("Singles Artist %02d/Some Single/Song.mp3", i), "s") + } + for i := 0; i < 10; i++ { + rel := fmt.Sprintf("Shared Artist/Album/%02d Track.flac", i) + ev.write(ev.master, rel, "same") + ev.write(ev.slave, rel, "same") + } + ev.write(ev.master, "Master Only/Album/01 X.flac", "m") + if w := scanWarnings(t, ev); len(w) != 0 { + t.Fatalf("slave-only content should not produce warnings, got %q", w) + } +} + +func TestRenamedFoldersAreReported(t *testing.T) { + ev := setup(t, false) + ev.write(ev.master, "AC_DC/Back in Black/01 Hells Bells.flac", "a") + ev.write(ev.slave, "AC-DC/Back in Black/01 Hells Bells.mp3", "b") + ev.write(ev.master, "The Beatles/Abbey Road/01 Come Together.flac", "a") + ev.write(ev.slave, "Beatles, The/Abbey Road/01 Come Together.mp3", "b") + ev.write(ev.master, "Avenged Sevenfold/City of Evil/03 Bat Country.flac", "a") + ev.write(ev.slave, "Avenged Sevenfold/City Of Evil/03 Bat Country.mp3", "b") + ev.write(ev.slave, "Beyoncé/Lemonade/Formation.mp3", "b") + ev.write(ev.master, "Beyonce/Lemonade/Formation.flac", "a") + w := scanWarnings(t, ev) + if len(w) != 1 { + t.Fatalf("want one renamed-folders warning, got %q", w) + } + for _, want := range []string{`"AC-DC" (slave) vs "AC_DC" (master)`, `"Avenged Sevenfold/City Of Evil" (slave) vs "Avenged Sevenfold/City of Evil" (master)`, "4 folders", "1 more"} { + if !strings.Contains(w[0], want) { + t.Errorf("warning %q should contain %q", w[0], want) + } + } +} + +// A slave library without album folders is a genuine layout difference. +func TestDifferentLayoutIsReported(t *testing.T) { + ev := setup(t, false) + for i := 0; i < 24; i++ { + ev.write(ev.master, fmt.Sprintf("Band/Album/%02d Song Number %d.flac", i+1, i), "m") + ev.write(ev.slave, fmt.Sprintf("Band/Song Number %d.mp3", i), "s") + } + w := scanWarnings(t, ev) + if len(w) != 1 || !strings.Contains(w[0], "Of 24 songs found in both libraries, 24 are in a different folder") { + t.Fatalf("want a different-layout warning, got %q", w) + } +} + +// Singles matched to their album version are intended, not a layout problem. +func TestSinglesMatchedToAlbumsAreNotALayoutProblem(t *testing.T) { + ev := setup(t, false) + enableApprox(t, ev) + for i := 0; i < 24; i++ { + ev.write(ev.master, fmt.Sprintf("Band/Album/%02d Song Number %d.flac", i+1, i), "m") + ev.write(ev.slave, fmt.Sprintf("Band/Song Number %d - Single/Song Number %d.mp3", i, i), "s") + } + if w := scanWarnings(t, ev); len(w) != 0 { + t.Fatalf("singles matched to album versions should not warn, got %q", w) + } +} + +func TestFolderKey(t *testing.T) { + same := [][2]string{{"AC/DC", "AC-DC"}, {"The Beatles", "Beatles, The"}, {"Simon & Garfunkel", "Simon and Garfunkel"}, + {"Beyoncé", "Beyonce"}, {"City Of Evil", "City of Evil"}, {"Sigur Rós", "Sigur Ros"}} + for _, p := range same { + if folderKey(p[0]) != folderKey(p[1]) { + t.Errorf("%q and %q should share a key (%q vs %q)", p[0], p[1], folderKey(p[0]), folderKey(p[1])) + } + } + if folderKey("Theory of a Deadman") == folderKey("ory of a Deadman") { + t.Error(`"The" must only be stripped as a whole word`) + } + if folderKey("Nightmare") == folderKey("Nightmare (Deluxe)") { + t.Error("different folders must keep different keys") + } +} diff --git a/internal/engine/scan.go b/internal/engine/scan.go index 4c614fa..4f742e6 100644 --- a/internal/engine/scan.go +++ b/internal/engine/scan.go @@ -314,7 +314,7 @@ func (e *Engine) scan(ctx context.Context, j *Job, pairID int64, trigger string) j.phase("Comparing files", int64(len(slaveFiles))) keep := map[string]bool{} - kinds := map[string]int{} + var layout layoutStats for _, rel := range sortedKeys(slaveFiles) { if ctx.Err() != nil { return ctx.Err() @@ -331,7 +331,7 @@ func (e *Engine) scan(ctx context.Context, j *Job, pairID int64, trigger string) return err } keep[rel] = true - kinds[m.Kind]++ + layout.add(m.RelSlave, m.RelMaster, m.Kind, m.Reason) j.count(m.Status) j.tick() } @@ -340,11 +340,8 @@ func (e *Engine) scan(ctx context.Context, j *Job, pairID int64, trigger string) } // Layout warnings based on what was actually found. - if ss.files > 0 { - if n := kinds[store.KindElsewhere] + kinds[store.KindApprox] + kinds[store.KindNone]; n*4 > ss.files { - warns = append(warns, fmt.Sprintf("%d of %d slave files have no counterpart at the same path in the master library. "+ - "Check that both libraries use the same folder layout.", n, ss.files)) - } + if w := layout.warning(); w != "" { + warns = append(warns, w) } if ms.symlinks+ss.symlinks > 0 { warns = append(warns, fmt.Sprintf("Ignored %d symlinks (playlink never follows or replaces symlinks).", ms.symlinks+ss.symlinks))