Show scan warnings as structured cards instead of long sentences
Warnings are now stored and served as structured data (title, detail, slave/master items, total) and rendered as cards with a Slave -> Master table: the first five rows, with "Show all" for the rest. - Renamed folders highlight the characters that differ; non-ASCII characters get their code point in a tooltip, so look-alikes such as "-" (U+002D) and "‐" (U+2010) can be told apart. - Songs in a different folder are grouped by folder pair with a song count, most affected first, instead of two long examples. - Folders that only differ by name are no longer also reported as songs in a different folder. - Warnings saved as plain text by earlier scans still display. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1d31999793
commit
9e848cd765
13 files changed
+305
-92
No files matched your search
@@ -255,7 +255,7 @@ func (e *Engine) open(p store.Pair) (*libs, error) {
|
||||
|
||||
// NormalizePair fills defaults and validates a pair. It returns non-fatal
|
||||
// warnings (such as layout mismatches) for the UI.
|
||||
func (e *Engine) NormalizePair(p *store.Pair) ([]string, error) {
|
||||
func (e *Engine) NormalizePair(p *store.Pair) ([]store.Warning, error) {
|
||||
p.Name = strings.TrimSpace(p.Name)
|
||||
if p.Name == "" {
|
||||
return nil, errors.New("name is required")
|
||||
@@ -301,7 +301,7 @@ func (e *Engine) NormalizePair(p *store.Pair) ([]string, error) {
|
||||
// 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 {
|
||||
func layoutWarnings(l *libs) []store.Warning {
|
||||
empty := true
|
||||
if ents, _ := os.ReadDir(l.slave.Path); len(ents) > 0 {
|
||||
for _, en := range ents {
|
||||
@@ -312,10 +312,11 @@ func layoutWarnings(l *libs) []string {
|
||||
}
|
||||
}
|
||||
if empty {
|
||||
return []string{"The slave library is empty. Use Build to pick songs from the master library to hardlink into it."}
|
||||
return []store.Warning{{Title: "The slave library is empty",
|
||||
Detail: "Use Build to pick songs from the master library to hardlink into it."}}
|
||||
}
|
||||
if w := renamedWarning(renamedFolders(l)); w != "" {
|
||||
return []string{w}
|
||||
if w := renamedWarning(renamedFolders(l)); w != nil {
|
||||
return []store.Warning{*w}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -502,8 +502,8 @@ func TestAnyLayoutMatchesWithoutWarnings(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, w := range p.LastScanWarnings {
|
||||
if strings.Contains(w, "layout") || strings.Contains(w, "folder") || strings.Contains(w, "depth") {
|
||||
t.Fatalf("matching layouts must not produce layout warnings, got %q", w)
|
||||
if text := w.Title + " " + w.Detail; strings.Contains(text, "layout") || strings.Contains(text, "folder") || strings.Contains(text, "depth") {
|
||||
t.Fatalf("matching layouts must not produce layout warnings, got %+v", w)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+51
-27
@@ -35,6 +35,15 @@ func folderKey(name string) string {
|
||||
return b.String()
|
||||
}
|
||||
|
||||
// pathKey applies folderKey to every component of a relative folder path.
|
||||
func pathKey(dir string) string {
|
||||
parts := strings.Split(dir, string(filepath.Separator))
|
||||
for i, p := range parts {
|
||||
parts[i] = folderKey(p)
|
||||
}
|
||||
return strings.Join(parts, "/")
|
||||
}
|
||||
|
||||
func subdirs(dir string) []string {
|
||||
ents, _ := os.ReadDir(dir)
|
||||
var out []string
|
||||
@@ -84,32 +93,32 @@ func renamedFolders(l *libs) [][2]string {
|
||||
return out
|
||||
}
|
||||
|
||||
func renamedWarning(pairs [][2]string) string {
|
||||
// maxWarningItems caps how many rows a warning carries.
|
||||
const maxWarningItems = 200
|
||||
|
||||
func renamedWarning(pairs [][2]string) *store.Warning {
|
||||
if len(pairs) == 0 {
|
||||
return ""
|
||||
return nil
|
||||
}
|
||||
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]))
|
||||
w := &store.Warning{Total: len(pairs),
|
||||
Detail: "Songs inside these folders can only match by path once the names agree. Rename one side so they match."}
|
||||
if len(pairs) == 1 {
|
||||
w.Title = "1 folder is named differently in the two libraries"
|
||||
} else {
|
||||
w.Title = fmt.Sprintf("%d folders are named differently in the two libraries", len(pairs))
|
||||
}
|
||||
more := ""
|
||||
if len(pairs) > 3 {
|
||||
more = fmt.Sprintf(" and %d more", len(pairs)-3)
|
||||
for _, p := range pairs[:min(maxWarningItems, len(pairs))] {
|
||||
w.Items = append(w.Items, store.WarningItem{Slave: p[0], Master: p[1]})
|
||||
}
|
||||
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)
|
||||
return w
|
||||
}
|
||||
|
||||
// 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
|
||||
shared int // slave files with a master counterpart
|
||||
moved int // ...that live in a different folder in the master
|
||||
pairs map[[2]string]int // (slave album folder, master album folder) -> songs
|
||||
}
|
||||
|
||||
func (ls *layoutStats) add(relSlave, relMaster, kind, reason string) {
|
||||
@@ -123,26 +132,41 @@ func (ls *layoutStats) add(relSlave, relMaster, kind, reason string) {
|
||||
if albumKey(relSlave) == albumKey(relMaster) {
|
||||
return // only a disc folder differs
|
||||
}
|
||||
if pathKey(AlbumFolder(relSlave)) == pathKey(AlbumFolder(relMaster)) {
|
||||
return // same folder under another name: the renamed-folders warning covers it
|
||||
}
|
||||
// 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})
|
||||
if ls.pairs == nil {
|
||||
ls.pairs = map[[2]string]int{}
|
||||
}
|
||||
ls.pairs[[2]string{AlbumFolder(relSlave), AlbumFolder(relMaster)}]++
|
||||
}
|
||||
|
||||
func (ls *layoutStats) warning() string {
|
||||
func (ls *layoutStats) warning() *store.Warning {
|
||||
if ls.shared < 20 || ls.moved*4 <= ls.shared {
|
||||
return ""
|
||||
return nil
|
||||
}
|
||||
var ex []string
|
||||
for _, e := range ls.example {
|
||||
ex = append(ex, fmt.Sprintf("%q is at %q", e[0], e[1]))
|
||||
items := make([]store.WarningItem, 0, len(ls.pairs))
|
||||
for p, n := range ls.pairs {
|
||||
items = append(items, store.WarningItem{Slave: p[0], Master: p[1], Count: n})
|
||||
}
|
||||
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, "; "))
|
||||
sort.Slice(items, func(i, j int) bool {
|
||||
if items[i].Count != items[j].Count {
|
||||
return items[i].Count > items[j].Count
|
||||
}
|
||||
return items[i].Slave < items[j].Slave
|
||||
})
|
||||
w := &store.Warning{
|
||||
Title: fmt.Sprintf("%d of %d songs found in both libraries are in a different folder in the master library", ls.moved, ls.shared),
|
||||
Detail: "The libraries seem to use different folder layouts here, so these songs only match through the fallbacks. " +
|
||||
"Folders are listed with the most affected songs first.",
|
||||
Total: len(items),
|
||||
Items: items[:min(maxWarningItems, len(items))],
|
||||
}
|
||||
return w
|
||||
}
|
||||
@@ -4,18 +4,20 @@ import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/cdnutter/playlink/internal/store"
|
||||
)
|
||||
|
||||
func scanWarnings(t *testing.T, ev *env) []string {
|
||||
func scanWarnings(t *testing.T, ev *env) []store.Warning {
|
||||
t.Helper()
|
||||
ev.scan()
|
||||
p, err := ev.st.Pair(ctx, ev.pair.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var out []string
|
||||
var out []store.Warning
|
||||
for _, w := range p.LastScanWarnings {
|
||||
if !strings.Contains(w, "ffprobe") {
|
||||
if !strings.Contains(w.Title, "ffprobe") {
|
||||
out = append(out, w)
|
||||
}
|
||||
}
|
||||
@@ -36,7 +38,7 @@ func TestSlaveOnlyContentIsNotALayoutProblem(t *testing.T) {
|
||||
}
|
||||
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)
|
||||
t.Fatalf("slave-only content should not produce warnings, got %+v", w)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,13 +53,23 @@ func TestRenamedFoldersAreReported(t *testing.T) {
|
||||
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)
|
||||
if len(w) != 1 || w[0].Title != "4 folders are named differently in the two libraries" || w[0].Total != 4 {
|
||||
t.Fatalf("want one renamed-folders warning, got %+v", 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)
|
||||
want := map[store.WarningItem]bool{
|
||||
{Slave: "AC-DC", Master: "AC_DC"}: true,
|
||||
{Slave: "Beatles, The", Master: "The Beatles"}: true,
|
||||
{Slave: "Beyoncé", Master: "Beyonce"}: true,
|
||||
{Slave: "Avenged Sevenfold/City Of Evil", Master: "Avenged Sevenfold/City of Evil"}: true,
|
||||
}
|
||||
for _, it := range w[0].Items {
|
||||
if !want[it] {
|
||||
t.Errorf("unexpected item %+v", it)
|
||||
}
|
||||
delete(want, it)
|
||||
}
|
||||
if len(want) != 0 {
|
||||
t.Errorf("missing items %v", want)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -69,8 +81,11 @@ func TestDifferentLayoutIsReported(t *testing.T) {
|
||||
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)
|
||||
if len(w) != 1 || w[0].Title != "24 of 24 songs found in both libraries are in a different folder in the master library" {
|
||||
t.Fatalf("want a different-layout warning, got %+v", w)
|
||||
}
|
||||
if len(w[0].Items) != 1 || w[0].Items[0] != (store.WarningItem{Slave: "Band", Master: "Band/Album", Count: 24}) {
|
||||
t.Fatalf("songs should be grouped by folder pair, got %+v", w[0].Items)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,7 +98,7 @@ func TestSinglesMatchedToAlbumsAreNotALayoutProblem(t *testing.T) {
|
||||
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)
|
||||
t.Fatalf("singles matched to album versions should not warn, got %+v", w)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,3 +117,15 @@ func TestFolderKey(t *testing.T) {
|
||||
t.Error("different folders must keep different keys")
|
||||
}
|
||||
}
|
||||
|
||||
// A renamed folder is reported once, as a renamed folder, not again as songs
|
||||
// living in a different folder.
|
||||
func TestRenamedFolderNotCountedAsMoved(t *testing.T) {
|
||||
var ls layoutStats
|
||||
for i := 0; i < 30; i++ {
|
||||
ls.add(fmt.Sprintf("ABBA/Voulez-Vous/%02d x.flac", i), fmt.Sprintf("ABBA/Voulez‐vous/%02d x.flac", i), store.KindElsewhere, "")
|
||||
}
|
||||
if ls.shared != 30 || ls.moved != 0 || ls.warning() != nil {
|
||||
t.Fatalf("renamed folder counted as moved: shared=%d moved=%d", ls.shared, ls.moved)
|
||||
}
|
||||
}
|
||||
@@ -340,14 +340,16 @@ func (e *Engine) scan(ctx context.Context, j *Job, pairID int64, trigger string)
|
||||
}
|
||||
|
||||
// Layout warnings based on what was actually found.
|
||||
if w := layout.warning(); w != "" {
|
||||
warns = append(warns, w)
|
||||
if w := layout.warning(); w != nil {
|
||||
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))
|
||||
warns = append(warns, store.Warning{Title: fmt.Sprintf("Ignored %d symlinks", ms.symlinks+ss.symlinks),
|
||||
Detail: "playlink never follows or replaces symlinks."})
|
||||
}
|
||||
if e.prober.Bin == "" {
|
||||
warns = append(warns, "ffprobe was not found, so codec and quality details are unavailable for manual review.")
|
||||
warns = append(warns, store.Warning{Title: "ffprobe was not found",
|
||||
Detail: "Codec and quality details are unavailable for manual review."})
|
||||
}
|
||||
if err := e.st.FinishScan(ctx, pairID, time.Now().Unix(), "", warns); err != nil {
|
||||
return err
|
||||
|
||||
Reference in new issue
Block a user