feat: one album sheet, editable from every screen before filing
Everything intake is about to write is now editable in a single $EDITOR document: the album, the album artist, both folder names, the year, the cover, and per track the title, artist, disc, track number and an optional file name override. It opens on 'e' from every screen that has a plan loaded except the working screen, including the cover picker -- choosing a cover is when a wrong album artist tends to be noticed, and having to back out of the picker first would make the editor useless exactly then. It replaces the track-name editor, which could change only the two folder names and the text after each track number. The album title, album artist, year, disc and track numbers and a single track's artist were all decided from the tags and could only be put right by retagging the files. Two things are new rather than carried over: - Deleting a track block really does leave that file out of the album. The old editor said a deleted line skipped the file and then filed it anyway, under the name intake had already chosen for it. The per-disc track totals are counted again after a drop or a renumber, so a stale TRACKTOTAL cannot make a player show a finished album as incomplete. - A folder that still holds the name intake derived it from follows a rename of the album or artist. One typed on purpose does not. Track identity is the number after '##' -- the track position when the sheet was written -- and deliberately not the file name (editable), the track number (editable) or the row order (sortable). A value may contain '#' and ':' freely because a comment is only ever a whole line; the old editor stripped an inline comment and quietly truncated any title containing ' #'. An empty sheet leaves the album alone, an album of mp3s never reaches the editor because the sheet cannot make a lossy file lossless, and --edit-names forces it open. Closes 6c17ef4
diff --git a/README.md b/README.md
@@ -44,6 +44,7 @@ intake scan # list staged albums
intake scan --fast # list without reading metadata
intake album <name> # review one album, then archive it
intake album <name> --dry-run # show the full plan, change nothing
+intake album <name> --edit-names # open the album sheet first
intake resume # continue the first unarchived album
intake library verify # placeholder covers and missing libraries
intake library audit # compare archive and library for drift
@@ -59,8 +60,16 @@ Every command accepts `--root`, `--config`, `--state-dir`, `--dry-run`, `--yes`,
`--yes` approves automatically, but refuses when the plan has hard issues (unknown
album or artist, colliding names), so it can never silently invent metadata.
-When a name cannot be normalised — a Japanese title, a symbol-only title — intake opens
-`$EDITOR` on a small file instead of guessing.
+Everything about an album is editable before it is filed. Pressing `e` on any review
+screen — including the cover picker — opens the album sheet in `$EDITOR`: the album and
+album artist, both folder names, the year, the cover, and every track's title, artist,
+disc, track number and file name. Deleting a track block leaves that file out of the
+album. Nothing is guessed from a name that cannot be normalised — a Japanese title, a
+symbol-only title — without opening the sheet.
+
+The sheet is one document, and intake uses exactly what it says rather than what the tags
+said, so a wrong album artist or a missing year is fixed there instead of by retagging
+files.
## Layout
diff --git a/cmd/intake/main.go b/cmd/intake/main.go
@@ -40,7 +40,7 @@ Flags (all commands):
--dry-run show what would happen, change nothing
--yes approve automatically (refused if there are hard issues)
--json machine-readable output (scan)
- --edit-names always open the track-name editor, even when names are fine
+ --edit-names always open the album sheet, even when nothing needs fixing
--fast scan: list staging without reading file metadata
--no-cover skip the cover art phase
--no-library skip building the streaming library copy
@@ -88,7 +88,7 @@ func register(fs *flag.FlagSet) *commonFlags {
dryRun: fs.Bool("dry-run", false, "show what would happen without changing anything"),
yes: fs.Bool("yes", false, "approve automatically (refused if there are hard issues)"),
jsonOut: fs.Bool("json", false, "machine-readable output"),
- editNames: fs.Bool("edit-names", false, "always open the track-name editor"),
+ editNames: fs.Bool("edit-names", false, "always open the album sheet"),
fastScan: fs.Bool("fast", false, "list staging without reading file metadata (scan only)"),
noCover: fs.Bool("no-cover", false, "skip the cover art phase entirely"),
offline: fs.Bool("offline", false, "do not query any network cover source"),
diff --git a/docs/DESIGN.md b/docs/DESIGN.md
@@ -183,6 +183,26 @@ renamed away.
A record is only deleted when the album it points at has gone. A run that fails is not
evidence about an album recorded earlier.
+## The album sheet
+
+Everything intake is about to write is editable in one `$EDITOR` document, reachable from
+every screen that has a plan loaded except the one that is filing it. That includes the
+cover picker, because choosing a cover is when a wrong album artist tends to be noticed.
+
+The format is `field: value` lines, whole-line `#` comments, and `## n` to begin a track.
+Nothing is positional: a block carries its own numbers, so a block can be moved or deleted
+without the rest shifting, and a value may contain `#` and `:` freely. It replaced an
+editor that could change only the two folder names and the text after each track number —
+and whose "delete a line to skip the file" did not actually skip it.
+
+Track identity is the number after `##`, which is the track's position in the plan when the
+sheet was written. It is not the file name, which the user may change, nor the track
+number, which they may change, nor the row order, which they may sort.
+
+Deleting a block drops that file from the album and the totals are counted again. A folder
+that still holds the name intake derived it from follows a rename of the album or artist;
+one that was typed on purpose does not.
+
## Interface
Bubble Tea v2 (`charm.land/bubbletea/v2`), Bubbles v2 and Lipgloss v2. Screens: browse,
diff --git a/internal/cli/artist_test.go b/internal/cli/artist_test.go
@@ -9,7 +9,9 @@ import (
"intake/internal/artist"
"intake/internal/config"
+ "intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/tags"
)
func artistTestApp(t *testing.T, root string, yes bool) *App {
@@ -128,3 +130,115 @@ func TestAliasRoundTripsThroughQuoting(t *testing.T) {
})
}
}
+
+// The album sheet is where a wrong album artist, a missing year or a bad track
+// number is fixed. This drives the whole path: build a plan, hand it to $EDITOR,
+// read back what the editor saved, and check it landed.
+func TestEditingTheAlbumSheetAppliesEverything(t *testing.T) {
+ root := t.TempDir()
+ script := filepath.Join(root, "editor.sh")
+ body := `#!/bin/sh
+sed -i \
+ -e 's/^albumartist:.*/albumartist: The Right Artist/' \
+ -e 's/^album:.*/album: The Right Album/' \
+ -e 's/^year:.*/year: 1997/' \
+ -e 's/^title: One$/title: One, Reprised/' \
+ "$1"
+`
+ if err := os.WriteFile(script, []byte(body), 0o755); err != nil {
+ t.Fatal(err)
+ }
+
+ a, _ := cliApp(t, root)
+ a.Cfg.Editor = script
+ p := plan.Plan{
+ Album: scan.Album{Dir: filepath.Join(root, "staging", "x")},
+ AlbumName: "An Album",
+ AlbumArt: "An Artist",
+ ArtistDir: "an-artist",
+ AlbumDir: "an-album",
+ ArchiveDir: filepath.Join(root, "archive", "an-artist", "an-album"),
+ LibraryDir: filepath.Join(root, "library", "an-artist", "an-album"),
+ Tracks: []plan.TrackPlan{{
+ Source: filepath.Join(root, "staging", "x", "01 one.flac"),
+ Title: "One",
+ Disc: 1,
+ Number: 1,
+ Target: "1-01_one.flac",
+ Ext: ".flac",
+ Tags: []tags.Note{
+ {Field: tags.FieldTitle, Value: "One"},
+ {Field: tags.FieldAlbum, Value: "An Album"},
+ {Field: tags.FieldArtist, Value: "An Artist"},
+ {Field: tags.FieldAlbumArtist, Value: "An Artist"},
+ {Field: tags.FieldTrackNumber, Value: "1"},
+ },
+ }},
+ }
+
+ before := p.Sheet()
+ got, err := a.editSheet(&p)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got == nil {
+ t.Fatal("the editor returned nothing")
+ }
+ if p.AlbumArt != "The Right Artist" || p.AlbumName != "The Right Album" {
+ t.Errorf("album = %q by %q", p.AlbumName, p.AlbumArt)
+ }
+ if p.Year != "1997" {
+ t.Errorf("year = %q", p.Year)
+ }
+ if got.Tracks[0].Title != "One, Reprised" {
+ t.Errorf("title = %q", got.Tracks[0].Title)
+ }
+ if p.Tracks[0].Target != "1-01_one-reprised.flac" {
+ t.Errorf("target = %q", p.Tracks[0].Target)
+ }
+ if v, _ := tags.Get(p.Tracks[0].Tags, tags.FieldAlbumArtist); v != "The Right Artist" {
+ t.Errorf("the ALBUMARTIST tag was not updated: %q", v)
+ }
+ if v, _ := tags.Get(p.Tracks[0].Tags, tags.FieldDate); v != "1997" {
+ t.Errorf("the DATE tag was not added: %q", v)
+ }
+ // The artists.toml-worthy question is whether the folder moved.
+ changes := got.ChangedFrom(before)
+ if !changes.AlbumArtist || changes.ArtistDir {
+ t.Errorf("changes = %+v; the folder was not touched by the editor", changes)
+ }
+
+ // And an editor that saves the file unchanged changes nothing.
+ if _, err := a.editSheet(&p); err != nil {
+ t.Fatal(err)
+ }
+ if p.AlbumArt != "The Right Artist" {
+ t.Errorf("a no-op edit changed the album: %q", p.AlbumArt)
+ }
+}
+
+// An album of mp3s is blocked too, and the sheet cannot turn a lossy file into a
+// lossless one, so opening an editor for it would only waste a round trip.
+func TestTheSheetOnlyOffersToFixWhatItCanFix(t *testing.T) {
+ mp3 := plan.Plan{
+ AlbumName: "An Album",
+ AlbumArt: "An Artist",
+ Tracks: []plan.TrackPlan{{Source: "/s/01 one.mp3", Ext: ".mp3", Title: "One"}},
+ Issues: []plan.Issue{{Hard: true, Text: "the archive holds flac only, and this album contains mp3; convert it to flac first, or file it by hand"}},
+ }
+ if sheetFixable(mp3) {
+ t.Error("an mp3 album should not send the user to the editor")
+ }
+
+ nameless := plan.Plan{
+ Tracks: []plan.TrackPlan{{Source: "/s/01 one.flac", Ext: ".flac", Title: "One"}},
+ Issues: []plan.Issue{{Hard: true, Text: "album artist is unknown: no ALBUMARTIST/ARTIST tag and the directory name gives no hint"}},
+ }
+ if !sheetFixable(nameless) {
+ t.Error("an album with no artist is exactly what the sheet is for")
+ }
+
+ if sheetFixable(plan.Plan{}) {
+ t.Error("a plan with no hard issues should not need the editor")
+ }
+}
diff --git a/internal/cli/cli.go b/internal/cli/cli.go
@@ -17,9 +17,9 @@ import (
"intake/internal/audio"
"intake/internal/config"
"intake/internal/library"
- "intake/internal/names"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/state"
"intake/internal/tags"
)
@@ -219,74 +219,72 @@ func (a *App) process(ctx context.Context, al scan.Album) error {
return fmt.Errorf("%s: %s (%s)", al.Name, al.Status, strings.Join(al.Notes, "; "))
}
- names0 := plan.Build(al, a.Cfg.Archive(), a.Cfg.Library(), plan.DirKey{}, a.Cfg.DiscPrefix)
- var resolved *plan.Plan
- var editSession *names.Session
- needsNames := nameIssues(names0) || strings.TrimSpace(names0.AlbumArt) == ""
- if a.Yes && needsNames {
+ p0 := plan.Build(al, a.Cfg.Archive(), a.Cfg.Library(), plan.DirKey{}, a.Cfg.DiscPrefix)
+ needsNames := sheetFixable(p0) || a.EditNames
+ if a.Yes && needsNames && p0.Blocked() {
// --yes is for unattended runs. The editor would sit waiting for a
// person who is not watching, which hangs the whole batch on the first
// album whose names cannot be worked out, so refuse and say what is
// wrong instead. Every case that reaches here is a hard issue.
a.logf("")
- a.logf("REFUSING to proceed: %d issue(s) need a name, and --yes cannot open an editor to supply one.", countHard(names0))
- for _, i := range names0.Issues {
+ a.logf("REFUSING to proceed: %d issue(s) need a name, and --yes cannot open an editor to supply one.", countHard(p0))
+ for _, i := range p0.Issues {
if i.Hard {
a.logf(" ! %s", i.Text)
}
}
return errors.New("names need editing, which --yes cannot do")
}
- if a.EditNames || needsNames {
- edited := names0
- session, err := a.editNames(&edited)
+
+ var edited *sheet.Sheet
+ var changes sheet.Changes
+ if needsNames {
+ before := p0.Sheet()
+ got, err := a.editSheet(&p0)
if err != nil {
- if a.Yes && edited.Blocked() {
+ if a.Yes && p0.Blocked() {
a.logf("")
a.logf("REFUSING to proceed: %v", err)
return errors.New("hard issues present; refusing --yes")
}
return err
}
- resolved = &edited
- editSession = session
- a.logf("")
- a.logf("NAMES")
- a.logf(" album : %s", edited.AlbumName)
- a.logf(" artist : %s", edited.AlbumArt)
+ if got != nil {
+ edited, changes = got, got.ChangedFrom(before)
+ a.logf("")
+ a.logf("ALBUM SHEET")
+ a.logf(" album : %s", p0.AlbumName)
+ a.logf(" album artist : %s", p0.AlbumArt)
+ a.logf(" artist folder: %s", p0.ArtistDir)
+ a.logf(" album folder : %s", p0.AlbumDir)
+ a.logf(" year : %s", orUnknown(p0.Year))
+ a.logf(" tracks : %d", len(p0.Tracks))
+ }
}
- var various bool
- if resolved != nil {
- various = resolved.VariousArt
- } else {
- various = names0.VariousArt
- }
- credit := names0.CreditFromTags
+ credit := p0.CreditFromTags
if strings.TrimSpace(credit) == "" {
- if resolved != nil && strings.TrimSpace(resolved.AlbumArt) != "" {
- credit = resolved.AlbumArt
- } else {
- credit = names0.AlbumArt
- }
+ credit = p0.AlbumArt
}
editorArtist := ""
- if resolved != nil && editSession != nil {
- switch {
- case strings.TrimSpace(names0.AlbumArt) == "":
- editorArtist = editSession.ArtistDir
- case editSession.ArtistUserEdited:
- editorArtist = editSession.ArtistDir
- }
+ if changes.ArtistDir {
+ editorArtist = p0.ArtistDir
}
- key, chosenCredit, err := a.resolveArtist(al, credit, various, editorArtist)
+ key, chosenCredit, err := a.resolveArtist(al, credit, p0.VariousArt, editorArtist)
if err != nil {
return err
}
if strings.TrimSpace(chosenCredit) != "" {
key.Credit = chosenCredit
}
- p := plan.BuildWithArtist(al, a.Cfg.Archive(), a.Cfg.Library(), key, a.Cfg.DiscPrefix, resolved, editSession)
+ p := plan.BuildWithArtist(al, a.Cfg.Archive(), a.Cfg.Library(), key, a.Cfg.DiscPrefix)
+ if edited != nil {
+ // Applied after the artist is resolved, because the sheet is the last
+ // word on what gets written.
+ if err := p.ApplySheet(*edited); err != nil {
+ return err
+ }
+ }
cover := coverChoice{}
if !a.Cfg.NoCover {
@@ -307,8 +305,8 @@ func (a *App) process(ctx context.Context, al scan.Album) error {
}
aliasCredit := key.Credit
- if strings.TrimSpace(names0.CreditFromTags) != "" {
- aliasCredit = names0.CreditFromTags
+ if strings.TrimSpace(p0.CreditFromTags) != "" {
+ aliasCredit = p0.CreditFromTags
}
if (key.Source == "entered" || key.Source == "editor") &&
strings.TrimSpace(aliasCredit) != "" && key.Dir != aliasCredit {
diff --git a/internal/cli/file.go b/internal/cli/file.go
@@ -222,7 +222,7 @@ func (a *App) PrepareAlbum(ctx context.Context, dir string) (plan.Plan, error) {
if strings.TrimSpace(chosenCredit) != "" {
key.Credit = chosenCredit
}
- return plan.BuildWithArtist(al, a.Cfg.Archive(), a.Cfg.Library(), key, a.Cfg.DiscPrefix, nil, nil), nil
+ return plan.BuildWithArtist(al, a.Cfg.Archive(), a.Cfg.Library(), key, a.Cfg.DiscPrefix), nil
}
func (a *App) FileAlbum(ctx context.Context, dir string, logf func(string, ...any)) (FileResult, error) {
diff --git a/internal/cli/interactive.go b/internal/cli/interactive.go
@@ -10,10 +10,10 @@ import (
"strings"
"intake/internal/artist"
- "intake/internal/names"
"intake/internal/normalize"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
)
func (a *App) artistIndex(credit string) (*artist.Index, error) {
@@ -331,111 +331,108 @@ func (a *App) rememberAlias(ix *artist.Index, credit, dir string) {
}
}
-func (a *App) editNames(p *plan.Plan) (*names.Session, error) {
- session := p.EditSession()
- if len(session.Tracks) == 0 {
- return nil, nil
+func (a *App) openEditor(path string) bool {
+ editor := a.Cfg.EditorCommand()
+ if editor == "" {
+ return false
}
- dir, err := os.MkdirTemp("", "intake-names-")
- if err != nil {
- return nil, err
+ cmd := exec.Command("sh", "-c", editor+" "+shellQuote(path))
+ cmd.Stdin = os.Stdin
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ if err := cmd.Run(); err != nil {
+ if _, ok := err.(*exec.Error); ok {
+ return false
+ }
}
- defer os.RemoveAll(dir)
+ return true
+}
- path := filepath.Join(dir, "names.txt")
- if err := os.WriteFile(path, []byte(names.Render(session)), 0o644); err != nil {
+func shellQuote(s string) string {
+ return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
+
+// editSheet opens the whole album in $EDITOR: every field intake is about to
+// write, in one file, and applies whatever comes back.
+//
+// This replaced an editor that could only change the two folder names and the
+// text after each track number. Everything else -- the album title, the album
+// artist, the year, the disc and track numbers, the artist on one track -- was
+// decided from the tags and could only be put right by retagging the files.
+func (a *App) editSheet(p *plan.Plan) (*sheet.Sheet, error) {
+ current := p.Sheet()
+ path := filepath.Join(a.Cfg.StateDir, "sheet.txt")
+ if a.Cfg.StateDir == "" {
+ dir, err := os.MkdirTemp("", "intake-sheet-")
+ if err != nil {
+ return nil, err
+ }
+ defer os.RemoveAll(dir)
+ path = filepath.Join(dir, "sheet.txt")
+ } else {
+ if err := os.MkdirAll(a.Cfg.StateDir, 0o755); err != nil {
+ return nil, err
+ }
+ }
+ if err := os.WriteFile(path, []byte(sheet.Render(current)), 0o644); err != nil {
return nil, err
}
a.logf("")
- if len(p.Issues) > 0 {
- a.logf("Some names need you:")
- for _, it := range p.Issues {
- a.logf(" ! %s", it.Text)
+ if p.Blocked() {
+ a.logf("This album needs you. Everything about it is editable:")
+ for _, i := range p.Issues {
+ if i.Hard {
+ a.logf(" ! %s", i.Text)
+ }
}
} else {
- a.logf("Editing names.")
+ a.logf("Editing the album sheet.")
}
a.logf("Opening your editor on:")
a.logf(" %s", path)
_ = os.Stdout.Sync()
if !a.openEditor(path) {
- return nil, fmt.Errorf("could not launch an editor (set $EDITOR or the editor setting in config); the names still need fixing")
+ return nil, fmt.Errorf("could not launch an editor (set $EDITOR or the editor setting in config); the album still needs you")
}
edited, err := os.ReadFile(path)
if err != nil {
return nil, err
}
- res, err := names.Parse(string(edited), session)
+ next, err := sheet.Parse(string(edited), current)
if err != nil {
- return nil, fmt.Errorf("the edited names could not be read: %w", err)
- }
- if dupes := names.Duplicates(res.Tracks); len(dupes) > 0 {
- for target, sources := range dupes {
- return nil, fmt.Errorf("two tracks would both be named %q (from %s)", target, strings.Join(sources, ", "))
- }
- }
- if res.AlbumDir == "" {
- return nil, fmt.Errorf("album folder name is still empty; it needs a value")
+ return nil, fmt.Errorf("%s: %w", path, err)
}
- if res.ArtistDir == "" {
- return nil, fmt.Errorf("artist folder name is still empty; it needs a value")
+ if err := p.ApplySheet(next); err != nil {
+ return nil, fmt.Errorf("%s: %w", path, err)
}
- p.ApplyNames(res)
-
- final := session
- final.Album, final.AlbumDir, final.ArtistDir = res.Album, res.AlbumDir, res.ArtistDir
- final.ArtistUserEdited = res.ArtistUserEdited
- final.AlbumUserEdited = res.AlbumUserEdited
- final.Tracks = make([]names.Item, 0, len(res.Tracks))
- for _, track := range res.Tracks {
- for _, before := range session.Tracks {
- if before.Source == track.Source {
- track.Suggested = before.Suggested
- track.Original = before.Original
- break
- }
- }
- final.Tracks = append(final.Tracks, track)
- }
- return &final, nil
-}
-
-func (a *App) openEditor(path string) bool {
- editor := a.Cfg.EditorCommand()
- if editor == "" {
- return false
- }
- cmd := exec.Command("sh", "-c", editor+" "+shellQuote(path))
- cmd.Stdin = os.Stdin
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
- if _, ok := err.(*exec.Error); ok {
- return false
- }
- }
- return true
+ return &next, nil
}
-func shellQuote(s string) string {
- return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
-}
-
-func nameIssues(p plan.Plan) bool {
+// sheetFixable reports whether everything blocking an album is something a
+// person can put right in the album sheet. An album of mp3s is blocked too, and
+// opening an editor for it would only waste a round trip: the sheet cannot turn
+// a lossy file into a lossless one.
+func sheetFixable(p plan.Plan) bool {
+ var blocked bool
for _, i := range p.Issues {
if !i.Hard {
continue
}
+ blocked = true
t := i.Text
- if strings.Contains(t, "normalise to the same name") ||
- strings.Contains(t, "no usable title") ||
- strings.Contains(t, "empty directory name") ||
- strings.Contains(t, "no ASCII folder name") {
- return true
+ switch {
+ case strings.Contains(t, "album name is unknown"),
+ strings.Contains(t, "album artist is unknown"),
+ strings.Contains(t, "no ASCII folder name"),
+ strings.Contains(t, "no usable title"),
+ strings.Contains(t, "normalise to the same name"),
+ strings.Contains(t, "would be written twice"):
+ default:
+ return false
}
}
- return false
+ return blocked
}
diff --git a/internal/names/names.go b/internal/names/names.go
@@ -1,212 +0,0 @@
-package names
-
-import (
- "fmt"
- "path/filepath"
- "strings"
-)
-
-type Item struct {
- Source string
- Prefix string
- Suffix string
- Original string
- KeepAsIs bool
- Number int
- Disc int
- Suggested string
-}
-
-func (i Item) Full() string {
- if i.KeepAsIs {
- return filepath.Base(i.Source)
- }
- return i.Prefix + i.Suffix
-}
-
-type Session struct {
- Album string
- AlbumDir string
- ArtistDir string
- Tracks []Item
- ArtistUserEdited bool
- AlbumUserEdited bool
-}
-
-type Result struct {
- Album string
- AlbumDir string
- ArtistDir string
- Tracks []Item
- AlbumSet bool
- AlbumUserEdited bool
- ArtistUserEdited bool
- TrackEdited map[string]bool
-}
-
-func Render(s Session) string {
- var b strings.Builder
- b.WriteString("# intake names for this album\n")
- b.WriteString("#\n")
- b.WriteString("# 'album' is the ALBUM tag and the folder name; 'artist' is the folder\n")
- b.WriteString("# under archive/ and library/. Both must be safe directory names:\n")
- b.WriteString("# lowercase, no slashes. Edit them if normalisation could not work\n")
- b.WriteString("# them out (a Japanese title, for example, has no ASCII form).\n")
- b.WriteString("#\n")
- b.WriteString("# Track lines: the \"disc-track_\" part is generated and must not change.\n")
- b.WriteString("# Edit what comes after it. Write '=' to keep the original filename.\n")
- b.WriteString("# Delete a track line to leave that file untouched (it is skipped).\n")
- b.WriteString("# An empty file aborts the whole album.\n")
- b.WriteString("\n")
- fmt.Fprintf(&b, "album: %s\n", s.AlbumDir)
- fmt.Fprintf(&b, "artist: %s\n", s.ArtistDir)
- if s.AlbumDir == "" || s.ArtistDir == "" {
- b.WriteString("#\n")
- if s.AlbumDir == "" {
- fmt.Fprintf(&b, "# The album title %q has no ASCII form. Type a folder name above.\n", s.Album)
- }
- if s.ArtistDir == "" {
- b.WriteString("# The artist has no ASCII form. Type a folder name above.\n")
- }
- }
- b.WriteString("\n")
- for _, it := range s.Tracks {
- name := it.Full()
- if it.Original != "" && it.Original != name {
- fmt.Fprintf(&b, "%-60s # %s\n", name, it.Original)
- continue
- }
- fmt.Fprintf(&b, "%s\n", name)
- }
- return b.String()
-}
-
-func Parse(text string, s Session) (Result, error) {
- res := Result{Album: s.Album, AlbumDir: s.AlbumDir, ArtistDir: s.ArtistDir}
-
- byPrefix := map[string]int{}
- for i, it := range s.Tracks {
- byPrefix[it.Prefix] = i
- }
- out := make([]Item, len(s.Tracks))
- copy(out, s.Tracks)
- seen := make([]bool, len(s.Tracks))
-
- lineNo := 0
- for _, raw := range strings.Split(text, "\n") {
- lineNo++
- line := strings.TrimRight(raw, " \t\r")
- trimmed := strings.TrimSpace(line)
- if trimmed == "" || strings.HasPrefix(trimmed, "#") {
- continue
- }
- body := trimmed
- if i := strings.Index(body, " #"); i >= 0 {
- body = strings.TrimSpace(body[:i])
- }
-
- if k, v, ok := strings.Cut(body, ":"); ok {
- key := strings.ToLower(strings.TrimSpace(k))
- if key == "album" || key == "artist" {
- val := cleanDirValue(v)
- if val == "" {
- return Result{}, fmt.Errorf("line %d: %s has no name", lineNo, key)
- }
- if key == "album" {
- res.AlbumUserEdited = val != s.AlbumDir
- res.Album, res.AlbumDir, res.AlbumSet = val, val, true
- } else {
- res.ArtistUserEdited = val != s.ArtistDir
- res.ArtistDir = val
- }
- continue
- }
- }
-
- if strings.HasPrefix(body, "=") {
- idx := -1
- for j := range out {
- if !seen[j] {
- idx = j
- break
- }
- }
- if idx < 0 {
- return Result{}, fmt.Errorf("line %d: more entries than tracks", lineNo)
- }
- out[idx].KeepAsIs = true
- seen[idx] = true
- continue
- }
-
- prefix, rest, ok := strings.Cut(body, "_")
- if !ok {
- return Result{}, fmt.Errorf("line %d: %q is missing the generated prefix (it should look like 1-01_name.flac)", lineNo, body)
- }
- idx, known := byPrefix[prefix+"_"]
- if !known {
- return Result{}, fmt.Errorf("line %d: unknown prefix %q (do not change the disc-track number)", lineNo, prefix+"_")
- }
- if seen[idx] {
- return Result{}, fmt.Errorf("line %d: %s appears twice", lineNo, prefix)
- }
- rest = strings.TrimSpace(rest)
- if rest == "" {
- return Result{}, fmt.Errorf("line %d: %s has no name after the prefix", lineNo, prefix)
- }
- if strings.ContainsAny(rest, "/\\") {
- return Result{}, fmt.Errorf("line %d: %q contains a path separator", lineNo, rest)
- }
- if rest == "." || rest == ".." {
- return Result{}, fmt.Errorf("line %d: %q is not a valid name", lineNo, rest)
- }
- if rest != out[idx].Suggested {
- if res.TrackEdited == nil {
- res.TrackEdited = map[string]bool{}
- }
- res.TrackEdited[out[idx].Source] = true
- }
- out[idx].Suffix = rest
- out[idx].KeepAsIs = false
- seen[idx] = true
- }
-
- for i := range out {
- if seen[i] {
- res.Tracks = append(res.Tracks, out[i])
- }
- }
- if len(res.Tracks) == 0 {
- return Result{}, fmt.Errorf("no track names remained; nothing to do")
- }
- return res, nil
-}
-
-func cleanDirValue(v string) string {
- v = strings.TrimSpace(v)
- v = strings.Trim(v, "/")
- v = strings.ReplaceAll(v, string(filepath.Separator), "-")
- if v == "." || v == ".." {
- return ""
- }
- return v
-}
-
-func Duplicates(items []Item) map[string][]string {
- byName := map[string][]string{}
- for _, it := range items {
- name := it.Suffix
- if it.KeepAsIs {
- name = filepath.Base(it.Source)
- }
- name = strings.ToLower(strings.TrimSuffix(name, filepath.Ext(name)))
- byName[name] = append(byName[name], filepath.Base(it.Source))
- }
- out := map[string][]string{}
- for k, v := range byName {
- if len(v) > 1 {
- out[k] = v
- }
- }
- return out
-}
diff --git a/internal/names/names_test.go b/internal/names/names_test.go
@@ -1,129 +0,0 @@
-package names
-
-import (
- "strings"
- "testing"
-)
-
-func sample() Session {
- return Session{
- Album: "Test Album",
- AlbumDir: "test-album",
- ArtistDir: "test-artist",
- Tracks: []Item{
- {Source: "/s/01 a.flac", Prefix: "1-01_", Suffix: "first-light.flac", Original: "First Light", Number: 1, Disc: 1},
- {Source: "/s/02 b.flac", Prefix: "1-02_", Suffix: "second-wind.flac", Original: "Second Wind", Number: 2, Disc: 1},
- },
- }
-}
-
-func TestRenderParseRoundTrip(t *testing.T) {
- s := sample()
- text := Render(s)
- if !strings.Contains(text, "album: test-album") || !strings.Contains(text, "artist: test-artist") {
- t.Fatalf("rendered file missing the folder lines:\n%s", text)
- }
- if !strings.Contains(text, "1-01_first-light.flac") {
- t.Fatalf("rendered file missing a track line:\n%s", text)
- }
- got, err := Parse(text, s)
- if err != nil {
- t.Fatal(err)
- }
- if len(got.Tracks) != 2 || got.Tracks[0].Full() != "1-01_first-light.flac" {
- t.Errorf("round trip changed the names: %+v", got.Tracks)
- }
- if got.AlbumDir != "test-album" || got.ArtistDir != "test-artist" {
- t.Errorf("round trip changed the folders: %+v", got)
- }
-}
-
-func TestParseEditedFolderNames(t *testing.T) {
- s := sample()
- edited := "album: yoake\nartist: shiina-ringo\n\n1-01_yoake.flac\n1-02_morning-light.flac\n"
- got, err := Parse(edited, s)
- if err != nil {
- t.Fatal(err)
- }
- if got.AlbumDir != "yoake" || got.ArtistDir != "shiina-ringo" {
- t.Errorf("folders not applied: %+v", got)
- }
- if got.Album != "yoake" || !got.AlbumSet {
- t.Errorf("album tag should follow the edited folder name, got %q set=%v", got.Album, got.AlbumSet)
- }
- if got.Tracks[0].Full() != "1-01_yoake.flac" {
- t.Errorf("track name not applied: %q", got.Tracks[0].Full())
- }
-}
-
-func TestParseEditedName(t *testing.T) {
- s := sample()
- got, err := Parse("1-01_夜明け.flac\n1-02_second-wind.flac\n", s)
- if err != nil {
- t.Fatal(err)
- }
- if got.Tracks[0].Full() != "1-01_夜明け.flac" {
- t.Errorf("CJK name not preserved: %q", got.Tracks[0].Full())
- }
-}
-
-func TestParseKeepsOriginalComment(t *testing.T) {
- s := sample()
- got, err := Parse("1-01_custom-name.flac # First Light\n1-02_second-wind.flac\n", s)
- if err != nil {
- t.Fatal(err)
- }
- if got.Tracks[0].Full() != "1-01_custom-name.flac" {
- t.Errorf("comment leaked into the name: %q", got.Tracks[0].Full())
- }
-}
-
-func TestParseKeepAsIs(t *testing.T) {
- s := sample()
- got, err := Parse("=\n1-02_second-wind.flac\n", s)
- if err != nil {
- t.Fatal(err)
- }
- if !got.Tracks[0].KeepAsIs || got.Tracks[0].Full() != "01 a.flac" {
- t.Errorf("'=' should keep the source filename, got %+v", got.Tracks[0])
- }
-}
-
-func TestParseDeletedLineSkipsTrack(t *testing.T) {
- s := sample()
- got, err := Parse("1-01_first-light.flac\n", s)
- if err != nil {
- t.Fatal(err)
- }
- if len(got.Tracks) != 1 {
- t.Errorf("a deleted line should drop that track, got %d items", len(got.Tracks))
- }
-}
-
-func TestParseErrors(t *testing.T) {
- s := sample()
- cases := []struct{ name, text string }{
- {"missing prefix", "first-light.flac\n"},
- {"unknown prefix", "1-99_nope.flac\n"},
- {"empty name", "1-01_\n"},
- {"path separator", "1-01_a/b.flac\n"},
- {"duplicate prefix", "1-01_x.flac\n1-01_y.flac\n"},
- {"all comments", "# nothing here\n"},
- {"empty album", "album:\n"},
- }
- for _, c := range cases {
- if _, err := Parse(c.text, s); err == nil {
- t.Errorf("%s: expected an error for %q", c.name, c.text)
- }
- }
-}
-
-func TestDuplicates(t *testing.T) {
- items := []Item{
- {Source: "/s/a.flac", Prefix: "1-01_", Suffix: "same.flac", Number: 1},
- {Source: "/s/b.flac", Prefix: "1-02_", Suffix: "same.flac", Number: 2},
- }
- if d := Duplicates(items); len(d) != 1 {
- t.Errorf("expected one duplicate group, got %v", d)
- }
-}
diff --git a/internal/plan/plan.go b/internal/plan/plan.go
@@ -1,14 +1,15 @@
package plan
import (
+ "errors"
"fmt"
"path/filepath"
"sort"
"strings"
- "intake/internal/names"
"intake/internal/normalize"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/tags"
)
@@ -46,10 +47,16 @@ type Plan struct {
LibraryDir string
ArchiveRoot string
LibraryRoot string
- Tracks []TrackPlan
- Issues []Issue
- Dropped []string
- Warnings []string
+ // DiscPrefix and Width are the naming shape the plan was built with. An
+ // edit has to keep it: recomputing the padding from the tracks that are left
+ // would silently rename every file in an album whose highest-numbered track
+ // was just deleted.
+ DiscPrefix string
+ Width int
+ Tracks []TrackPlan
+ Issues []Issue
+ Dropped []string
+ Warnings []string
}
func (p Plan) Blocked() bool {
@@ -289,6 +296,8 @@ func Build(a scan.Album, archiveRoot, libraryRoot string, artistKey DirKey, disc
}
width := normalize.PadWidth(maxTrack)
+ p.Width = width
+ p.DiscPrefix = discPrefix
tracksOnDisc := map[int]int{}
for _, it := range items {
@@ -411,101 +420,24 @@ func splitDirName(dir string) (artist, album string, ok bool) {
return "", "", false
}
-func (p Plan) EditItems() []names.Item {
- return p.EditSession().Tracks
-}
-
-func (p Plan) EditSession() names.Session {
- items := make([]names.Item, 0, len(p.Tracks))
- for _, t := range p.Tracks {
- prefix, suffix := splitTarget(t.Target)
- items = append(items, names.Item{
- Source: t.Source,
- Prefix: prefix,
- Suffix: suffix,
- Suggested: suffix,
- Original: t.Title,
- Number: t.Number,
- Disc: t.Disc,
- })
- }
- return names.Session{
- Album: p.AlbumTag,
- AlbumDir: p.AlbumDir,
- ArtistDir: p.ArtistDir,
- Tracks: items,
- }
-}
-
-func (p *Plan) ApplyNames(res names.Result) {
- byPath := map[string]names.Item{}
- for _, it := range res.Tracks {
- byPath[it.Source] = it
- }
- for i := range p.Tracks {
- it, ok := byPath[p.Tracks[i].Source]
- if !ok {
- continue
- }
- p.Tracks[i].Target = it.Full()
- renamed := res.TrackEdited[it.Source]
- if renamed && !it.KeepAsIs {
- p.Tracks[i].Title = titleFromSuffix(it.Suffix)
- }
- for j := range p.Tracks[i].Tags {
- switch p.Tracks[i].Tags[j].Field {
- case tags.FieldTitle:
- if renamed {
- p.Tracks[i].Tags[j].Value = p.Tracks[i].Title
- }
- case tags.FieldTrackNumber:
- p.Tracks[i].Tags[j].Value = fmt.Sprintf("%d", it.Number)
- }
- }
- }
-
- if res.AlbumSet && res.AlbumUserEdited {
- p.AlbumName = res.Album
- for i := range p.Tracks {
- for j := range p.Tracks[i].Tags {
- if p.Tracks[i].Tags[j].Field == tags.FieldAlbum {
- p.Tracks[i].Tags[j].Value = res.Album
- }
- }
- }
- } else if res.AlbumSet && p.AlbumName == "" {
- p.AlbumName = res.Album
- for i := range p.Tracks {
- for j := range p.Tracks[i].Tags {
- if p.Tracks[i].Tags[j].Field == tags.FieldAlbum {
- p.Tracks[i].Tags[j].Value = res.Album
- }
- }
- }
- }
- if res.ArtistUserEdited {
- p.AlbumArt = res.ArtistDir
- for i := range p.Tracks {
- for j := range p.Tracks[i].Tags {
- if p.Tracks[i].Tags[j].Field == tags.FieldAlbumArtist {
- p.Tracks[i].Tags[j].Value = res.ArtistDir
- } else if p.Tracks[i].Tags[j].Field == tags.FieldArtist &&
- strings.TrimSpace(p.Tracks[i].Tags[j].Value) == "" {
- p.Tracks[i].Tags[j].Value = res.ArtistDir
- }
- }
- }
- }
-
- if res.AlbumDir != "" {
- p.AlbumDir = res.AlbumDir
+// noteCollisions records the names in a plan that cannot coexist.
+//
+// The first kind is two different titles collapsing to the same normalised text,
+// which is worth stopping for because the title is the only human-readable part
+// of a file name. The second is two tracks whose whole target is the same string,
+// which the first check cannot see when they also share a track number, because
+// it groups by title and folds repeats of a title together. That one is not a
+// naming nuisance but a lost track: the copy truncates and replaces, so the album
+// is filed one track short and every later check still passes.
+func noteCollisions(p *Plan, titles, sources [][2]string) {
+ for _, c := range normalize.DetectCollisions(titles) {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("two different titles normalise to the same name %q (from %s); rename one before continuing",
+ c.Target, strings.Join(quoteAll(c.Causes), " and "))})
}
- if res.ArtistDir != "" {
- p.ArtistDir = res.ArtistDir
+ for _, c := range normalize.TargetClashes(sources) {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("%s would be written twice, from %s; give the tracks different numbers or titles, in their tags or in the staged file names",
+ c.Target, strings.Join(quoteAll(c.Causes), " and "))})
}
- p.rebuildPaths()
- p.reassessIssues(res.AlbumUserEdited, res.ArtistUserEdited)
- noteCollisions(p, titlePairs(p.Tracks), sourcePairs(p.Tracks))
}
// titlePairs and sourcePairs are the two views noteCollisions needs: what the
@@ -526,110 +458,255 @@ func sourcePairs(tracks []TrackPlan) [][2]string {
return out
}
-// noteCollisions records the names in a plan that cannot coexist.
-//
-// The first kind is two different titles collapsing to the same normalised text,
-// which is worth stopping for because the title is the only human-readable part
-// of a file name. The second is two tracks whose whole target is the same string,
-// which the first check cannot see when they also share a track number, because
-// it groups by title and folds repeats of a title together. That one is not a
-// naming nuisance but a lost track: the copy truncates and replaces, so the album
-// is filed one track short and every later check still passes.
-func noteCollisions(p *Plan, titles, sources [][2]string) {
- for _, c := range normalize.DetectCollisions(titles) {
- p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("two different titles normalise to the same name %q (from %s); rename one before continuing",
- c.Target, strings.Join(quoteAll(c.Causes), " and "))})
+// Sheet returns the editable description of this plan: everything intake is
+// about to write, in a form a person can change in $EDITOR.
+func (p Plan) Sheet() sheet.Sheet {
+ tracks := make([]sheet.Track, 0, len(p.Tracks))
+ for i, t := range p.Tracks {
+ artist := ""
+ if v, ok := tags.Get(t.Tags, tags.FieldArtist); ok && strings.TrimSpace(v) != strings.TrimSpace(p.AlbumArt) {
+ artist = strings.TrimSpace(v)
+ }
+ tracks = append(tracks, sheet.Track{
+ ID: i + 1,
+ Title: t.Title,
+ Artist: artist,
+ Disc: t.Disc,
+ Number: t.Number,
+ Source: filepath.Base(t.Source),
+ Derived: t.Target,
+ })
}
- for _, c := range normalize.TargetClashes(sources) {
- p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("%s would be written twice, from %s; give the tracks different numbers or titles, in their tags or in the staged file names",
- c.Target, strings.Join(quoteAll(c.Causes), " and "))})
+ return sheet.Sheet{
+ Album: p.AlbumName,
+ AlbumArtist: p.AlbumArt,
+ ArtistDir: p.ArtistDir,
+ AlbumDir: p.AlbumDir,
+ Year: p.Year,
+ Tracks: tracks,
}
}
-func (p *Plan) reassessIssues(albumEdited, artistEdited bool) {
- if albumEdited || p.AlbumName != "" {
- p.Issues = removeIssues(p.Issues, "album name is unknown")
+// ApplySheet rewrites the plan from an edited sheet and returns it to a state
+// that can be filed: targets rebuilt, totals recounted, and every issue that was
+// about a name worked out again from what the sheet now says.
+//
+// Tracks the sheet no longer mentions are dropped from the album. That is what
+// makes "delete the block" mean something -- the old editor's comment claimed a
+// deleted line left the file out, and it did not: the track was still filed,
+// under the name intake had already chosen for it.
+func (p *Plan) ApplySheet(s sheet.Sheet) error {
+ before := p.Tracks
+ kept := make([]TrackPlan, 0, len(s.Tracks))
+ for _, st := range s.Tracks {
+ if st.ID < 1 || st.ID > len(before) {
+ return fmt.Errorf("track %d is not in this album (it has %d tracks)", st.ID, len(before))
+ }
+ tp := before[st.ID-1]
+ tp.Disc = st.Disc
+ tp.Number = st.Number
+ tp.Title = strings.TrimSpace(st.Title)
+ tp.Artist = strings.TrimSpace(st.Artist)
+ if tp.Artist == "" {
+ tp.Artist = strings.TrimSpace(s.AlbumArtist)
+ }
+ kept = append(kept, tp)
+ }
+ if len(kept) == 0 {
+ return errors.New("no tracks left; the album would have nothing in it")
+ }
+
+ oldAlbum, oldArt := p.AlbumName, p.AlbumArt
+ p.AlbumName = strings.TrimSpace(s.Album)
+ p.AlbumTag = p.AlbumName
+ p.AlbumArt = strings.TrimSpace(s.AlbumArtist)
+ p.CreditFromTags = p.AlbumArt
+
+ // A folder that still holds the name intake derived it from follows a
+ // rename; one that was typed on purpose does not. Correcting "Boards of
+ // Canad" to "Boards of Canada" should not leave the album sitting in
+ // "boards-of-canad", and deliberately naming a folder something other than
+ // the album should not be undone a moment later.
+ albumDir := strings.TrimSpace(s.AlbumDir)
+ if albumDir == "" || normalize.DirName(albumDir) == normalize.DirName(oldAlbum) {
+ albumDir = p.AlbumName
+ }
+ artistDir := strings.TrimSpace(s.ArtistDir)
+ if artistDir == "" || normalize.DirName(artistDir) == normalize.DirName(oldArt) {
+ artistDir = p.AlbumArt
+ }
+ p.AlbumDir = normalize.DirName(albumDir)
+ if p.AlbumDir == "" {
+ p.AlbumDir = normalize.Name(albumDir)
}
- if artistEdited || p.AlbumArt != "" {
- p.Issues = removeIssues(p.Issues, "album artist is unknown")
+ p.ArtistDir = normalize.DirName(artistDir)
+ if p.ArtistDir == "" {
+ p.ArtistDir = normalize.Name(artistDir)
}
- if p.AlbumDir != "" {
- p.Issues = removeIssues(p.Issues, "no ASCII folder name; open the editor to supply one")
+ p.Year = tags.NormalizeYear(s.Year)
+ if strings.TrimSpace(s.Year) != "" && p.Year == "" {
+ p.Warnings = append(p.Warnings, fmt.Sprintf("could not read a year from %q", s.Year))
}
- if p.ArtistDir != "" {
- p.Issues = removeIssues(p.Issues, `album artist "`)
+
+ // A dropped or renumbered track changes the totals, and a stale TRACKTOTAL
+ // is worse than none: players use it to tell a complete album from a
+ // half-finished download.
+ maxTrack := 0
+ for i := range kept {
+ if kept[i].Number > maxTrack {
+ maxTrack = kept[i].Number
+ }
}
- p.Issues = removeIssues(p.Issues, "normalise to the same name")
- p.Issues = removeIssues(p.Issues, "would be written twice")
-}
+ width := normalize.PadWidth(maxTrack)
+ if p.Width > width {
+ width = p.Width
+ }
+ discs := map[int]bool{}
+ for _, t := range kept {
+ discs[t.Disc] = true
+ }
+ withDisc := useDiscPrefix(p.DiscPrefix, len(discs))
+
+ for i := range kept {
+ name := kept[i].Title
+ override := strings.TrimSpace(s.Tracks[i].File)
+ if override != "" {
+ name = strings.ReplaceAll(filepath.Base(override), string(filepath.Separator), "-")
+ if name == "." || name == ".." {
+ return fmt.Errorf("track %d would be written to %q", s.Tracks[i].ID, name)
+ }
+ if filepath.Ext(name) == "" {
+ name += kept[i].Ext
+ }
+ kept[i].Target = name
+ kept[i].Title = titleFromFile(name)
+ } else {
+ kept[i].Target = normalize.TrackFile(kept[i].Disc, kept[i].Number, width, kept[i].Title, kept[i].Ext, withDisc)
+ }
+ if n, cut := normalize.Truncate(kept[i].Target, maxNameBytes); cut {
+ kept[i].Target = n
+ kept[i].Truncated = true
+ } else {
+ kept[i].Truncated = false
+ }
+ }
+ p.Tracks = kept
+ p.retotal()
-func BuildWithArtist(a scan.Album, archiveRoot, libraryRoot string, key DirKey, discPrefix string, edited *Plan, session *names.Session) Plan {
- p := Build(a, archiveRoot, libraryRoot, key, discPrefix)
- if edited == nil {
- return p
+ for i := range p.Tracks {
+ t := &p.Tracks[i]
+ t.Tags = setTag(t.Tags, tags.FieldTrackNumber, fmt.Sprintf("%d", t.Number))
+ t.Tags = setTag(t.Tags, tags.FieldDiscNumber, fmt.Sprintf("%d", t.Disc))
+ t.Tags = setTag(t.Tags, tags.FieldTitle, t.Title)
+ t.Tags = setTag(t.Tags, tags.FieldAlbum, p.AlbumName)
+ t.Tags = setTag(t.Tags, tags.FieldArtist, t.Artist)
+ t.Tags = setTag(t.Tags, tags.FieldAlbumArtist, p.AlbumArt)
+ t.Tags = setTag(t.Tags, tags.FieldDate, p.Year)
}
- var editedItems []names.Item
- if session != nil {
- editedItems = session.Tracks
- } else {
- editedItems = edited.EditItems()
+
+ p.rebuildPaths()
+ p.reassessAfterEdit()
+ noteCollisions(p, titlePairs(p.Tracks), sourcePairs(p.Tracks))
+ return nil
+}
+
+// retotal recounts the per-disc track totals and the disc total.
+func (p *Plan) retotal() {
+ perDisc := map[int]int{}
+ for _, t := range p.Tracks {
+ perDisc[t.Disc]++
}
- bySource := map[string]names.Item{}
- for _, it := range editedItems {
- bySource[it.Source] = it
+ for i := range p.Tracks {
+ t := &p.Tracks[i]
+ n := fmt.Sprintf("%d", perDisc[t.Disc])
+ t.Tags = setTag(t.Tags, tags.FieldTrackTotal, n)
+ t.Tags = setTag(t.Tags, tags.FieldLegacyTrackTotal, n)
+ d := fmt.Sprintf("%d", len(perDisc))
+ t.Tags = setTag(t.Tags, tags.FieldDiscTotal, d)
+ t.Tags = setTag(t.Tags, tags.FieldLegacyDiscTotal, d)
}
- if edited.AlbumName != "" {
- p.AlbumName = edited.AlbumName
- p.AlbumTag = edited.AlbumName
- for i := range p.Tracks {
- for j := range p.Tracks[i].Tags {
- if p.Tracks[i].Tags[j].Field == tags.FieldAlbum {
- p.Tracks[i].Tags[j].Value = edited.AlbumName
- }
+}
+
+// setTag replaces a tag, adds it, or removes it when the value is empty.
+func setTag(notes []tags.Note, field, value string) []tags.Note {
+ for i := range notes {
+ if strings.EqualFold(notes[i].Field, field) {
+ if value == "" {
+ return append(notes[:i], notes[i+1:]...)
}
+ notes[i].Value = value
+ return notes
}
}
- if edited.AlbumArt != "" && strings.TrimSpace(edited.CreditFromTags) == "" {
- oldArt := edited.AlbumArt
- p.AlbumArt = edited.AlbumArt
- for i := range p.Tracks {
- for j := range p.Tracks[i].Tags {
- switch p.Tracks[i].Tags[j].Field {
- case tags.FieldAlbumArtist:
- p.Tracks[i].Tags[j].Value = edited.AlbumArt
- case tags.FieldArtist:
- if v := strings.TrimSpace(p.Tracks[i].Tags[j].Value); v == "" || v == oldArt {
- p.Tracks[i].Tags[j].Value = edited.AlbumArt
- }
- }
- }
- }
+ if value == "" {
+ return notes
}
- for i := range p.Tracks {
- item, ok := bySource[p.Tracks[i].Source]
- if !ok {
- continue
- }
- target := item.Full()
- if target == "" {
- continue
- }
- p.Tracks[i].Target = target
- if item.Suffix != item.Suggested {
- _, suffix := splitTarget(target)
- p.Tracks[i].Title = titleFromSuffix(suffix)
- for k := range p.Tracks[i].Tags {
- if p.Tracks[i].Tags[k].Field == tags.FieldTitle {
- p.Tracks[i].Tags[k].Value = p.Tracks[i].Title
- }
- }
+ return append(notes, tags.Note{Field: field, Value: value})
+}
+
+// titleFromFile is the reverse of the naming rule, for a hand-typed file name.
+func titleFromFile(name string) string {
+ s := strings.TrimSuffix(name, filepath.Ext(name))
+ if i := strings.IndexByte(s, '_'); i >= 0 {
+ s = s[i+1:]
+ }
+ return strings.TrimSpace(strings.ReplaceAll(s, "-", " "))
+}
+
+// reassessAfterEdit works out the name issues again from what the plan now says,
+// rather than from what the tags originally said. Clearing and re-adding is the
+// point: an issue that the edit fixed has to disappear, or the album stays
+// blocked by a complaint about metadata that is no longer there.
+func (p *Plan) reassessAfterEdit() {
+ for _, text := range []string{
+ "album name is unknown",
+ "album name is empty",
+ "album artist is unknown",
+ "album artist is empty",
+ "no ASCII folder name",
+ "no usable title",
+ "normalise to the same name",
+ "would be written twice",
+ } {
+ p.Issues = removeIssues(p.Issues, text)
+ }
+ if p.AlbumName == "" {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: "the album has no name; put one in the album sheet"})
+ }
+ if p.AlbumArt == "" {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: "the album has no artist; put one in the album sheet"})
+ }
+ if p.AlbumDir == "" {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("album name %q has no ASCII folder name; put one in the album sheet", p.AlbumName)})
+ }
+ if p.ArtistDir == "" {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("album artist %q has no ASCII folder name; put one in the album sheet", p.AlbumArt)})
+ }
+ for _, t := range p.Tracks {
+ if strings.TrimSpace(t.Title) == "" {
+ p.Issues = append(p.Issues, Issue{Hard: true, Text: fmt.Sprintf("%s: no usable title; put one in the album sheet", filepath.Base(t.Source))})
}
}
+}
+
+// BuildWithArtist builds the plan again with the artist folder that was
+// resolved (or chosen) rather than the one the tags implied.
+func BuildWithArtist(a scan.Album, archiveRoot, libraryRoot string, key DirKey, discPrefix string) Plan {
+ p := Build(a, archiveRoot, libraryRoot, key, discPrefix)
if key.Dir != "" {
p.ArtistDir = key.Dir
}
+ if strings.TrimSpace(key.Credit) != "" {
+ p.AlbumArt = key.Credit
+ p.CreditFromTags = key.Credit
+ for i := range p.Tracks {
+ p.Tracks[i].Tags = setTag(p.Tracks[i].Tags, tags.FieldAlbumArtist, key.Credit)
+ if v, ok := tags.Get(p.Tracks[i].Tags, tags.FieldArtist); !ok || strings.TrimSpace(v) == "" {
+ p.Tracks[i].Tags = setTag(p.Tracks[i].Tags, tags.FieldArtist, key.Credit)
+ }
+ }
+ }
p.rebuildPaths()
- p.reassessIssues(true, true)
noteCollisions(&p, titlePairs(p.Tracks), sourcePairs(p.Tracks))
return p
}
@@ -663,19 +740,6 @@ func removeIssues(in []Issue, contains string) []Issue {
return out
}
-func splitTarget(target string) (prefix, suffix string) {
- if i := strings.IndexByte(target, '_'); i >= 0 {
- return target[:i+1], target[i+1:]
- }
- return "", target
-}
-
-func titleFromSuffix(suffix string) string {
- s := strings.TrimSuffix(suffix, filepath.Ext(suffix))
- s = strings.ReplaceAll(s, "-", " ")
- return strings.TrimSpace(s)
-}
-
func commonValue(a scan.Album, field string) (string, bool) {
var value string
found := false
diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go
@@ -8,6 +8,7 @@ import (
"intake/internal/audio"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/tags"
)
@@ -482,3 +483,262 @@ func firstHard(p Plan) string {
}
return ""
}
+
+// The sheet is the last word on what gets written. Everything it carries has to
+// land in the plan and in the tags, because the tags are the only record of it
+// once the album is filed.
+func TestApplySheetSetsEveryField(t *testing.T) {
+ p := planFor([]audio.Track{
+ track("01 one.flac", 1, 1, "One"),
+ track("02 two.flac", 2, 1, "Two"),
+ })
+ s := p.Sheet()
+ s.Album = "Corrected Album"
+ s.AlbumArtist = "Corrected Artist"
+ s.ArtistDir = "corrected-artist"
+ s.AlbumDir = "corrected-album"
+ s.Year = "1999"
+ s.Tracks[0].Artist = "Guest Singer"
+ s.Tracks[1].Title = "Two, Reprised"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+
+ if p.AlbumName != "Corrected Album" || p.AlbumArt != "Corrected Artist" {
+ t.Errorf("album = %q by %q", p.AlbumName, p.AlbumArt)
+ }
+ if p.ArtistDir != "corrected-artist" || p.AlbumDir != "corrected-album" {
+ t.Errorf("folders = %q/%q", p.ArtistDir, p.AlbumDir)
+ }
+ if got := value(t, p, 0, tags.FieldAlbum); got != "Corrected Album" {
+ t.Errorf("ALBUM = %q", got)
+ }
+ if got := value(t, p, 0, tags.FieldAlbumArtist); got != "Corrected Artist" {
+ t.Errorf("ALBUMARTIST = %q", got)
+ }
+ if got := value(t, p, 0, tags.FieldArtist); got != "Guest Singer" {
+ t.Errorf("track 1 ARTIST = %q", got)
+ }
+ if got := value(t, p, 1, tags.FieldArtist); got != "Corrected Artist" {
+ t.Errorf("an empty track artist should mean the album artist, got %q", got)
+ }
+ if got := value(t, p, 0, tags.FieldDate); got != "1999" {
+ t.Errorf("DATE = %q", got)
+ }
+ if got := value(t, p, 1, tags.FieldTitle); got != "Two, Reprised" {
+ t.Errorf("TITLE = %q", got)
+ }
+ if p.Tracks[1].Target != "1-02_two-reprised.flac" {
+ t.Errorf("target = %q", p.Tracks[1].Target)
+ }
+ if !strings.Contains(p.AlbumDir, "corrected") {
+ t.Errorf("the destination should follow the folder: %q", p.ArchiveDir)
+ }
+}
+
+// A folder that still holds the name intake derived it from follows a rename of
+// the album; one that was typed by hand does not.
+func TestApplySheetFollowsARenamedAlbumButNotATypedFolder(t *testing.T) {
+ p := planFor([]audio.Track{track("01 one.flac", 1, 1, "One")})
+
+ s := p.Sheet()
+ s.Album = "A Longer Album Name"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if p.AlbumDir != "a-longer-album-name" {
+ t.Errorf("the derived folder did not follow the rename: %q", p.AlbumDir)
+ }
+
+ s = p.Sheet()
+ s.AlbumDir = "deliberate"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if p.AlbumDir != "deliberate" {
+ t.Errorf("a typed folder was overwritten: %q", p.AlbumDir)
+ }
+ s = p.Sheet()
+ s.Album = "Yet Another Name"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if p.AlbumDir != "deliberate" {
+ t.Errorf("a typed folder followed a rename anyway: %q", p.AlbumDir)
+ }
+}
+
+// Deleting a block leaves the file out of the album, and the totals are counted
+// again: a stale TRACKTOTAL is what makes a player show an album as unfinished.
+func TestApplySheetDropsATrackAndRecounts(t *testing.T) {
+ p := planFor([]audio.Track{
+ track("01 one.flac", 1, 1, "One"),
+ track("02 two.flac", 2, 1, "Two"),
+ track("03 three.flac", 3, 1, "Three"),
+ })
+ if got := value(t, p, 0, tags.FieldTrackTotal); got != "3" {
+ t.Fatalf("TRACKTOTAL before the edit = %q", got)
+ }
+
+ s := p.Sheet()
+ s.Tracks = append(s.Tracks[:1], s.Tracks[2])
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if len(p.Tracks) != 2 {
+ t.Fatalf("got %d tracks, want 2", len(p.Tracks))
+ }
+ if filepath.Base(p.Tracks[1].Source) != "03 three.flac" {
+ t.Errorf("the wrong track was dropped: %s", p.Tracks[1].Source)
+ }
+ if got := value(t, p, 0, tags.FieldTrackTotal); got != "2" {
+ t.Errorf("TRACKTOTAL after the drop = %q, want 2", got)
+ }
+ if got := value(t, p, 1, tags.FieldDiscTotal); got != "1" {
+ t.Errorf("DISCTOTAL = %q, want 1", got)
+ }
+}
+
+// A disc that loses all its tracks stops being a disc.
+func TestApplySheetRecountsDiscs(t *testing.T) {
+ p := planFor([]audio.Track{
+ track("01 one.flac", 1, 1, "One"),
+ track("01 two.flac", 1, 2, "Two"),
+ })
+ s := p.Sheet()
+ s.Tracks = s.Tracks[:1]
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if got := value(t, p, 0, tags.FieldDiscTotal); got != "1" {
+ t.Errorf("DISCTOTAL = %q, want 1", got)
+ }
+ if !strings.HasPrefix(p.Tracks[0].Target, "1-01") {
+ t.Errorf("the disc prefix should stay: %q", p.Tracks[0].Target)
+ }
+}
+
+// Renumbering a track renames its file, and the padding of the other tracks does
+// not change underneath them: deleting track 100 should not rename track 1.
+func TestApplySheetRenumbersWithoutRenamingEverything(t *testing.T) {
+ p := planFor([]audio.Track{
+ track("001 one.flac", 1, 1, "One"),
+ track("100 two.flac", 100, 1, "Two"),
+ })
+ if got := p.Tracks[0].Target; got != "1-001_one.flac" {
+ t.Fatalf("target = %q", got)
+ }
+ s := p.Sheet()
+ s.Tracks = s.Tracks[:1]
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if got := p.Tracks[0].Target; got != "1-001_one.flac" {
+ t.Errorf("the remaining track was renamed to %q", got)
+ }
+
+ s = p.Sheet()
+ s.Tracks[0].Number = 7
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if got := value(t, p, 0, tags.FieldTrackNumber); got != "7" {
+ t.Errorf("TRACKNUMBER = %q", got)
+ }
+ if got := p.Tracks[0].Target; got != "1-007_one.flac" {
+ t.Errorf("target = %q", got)
+ }
+}
+
+// A typed file name is used as it is, with the extension filled in when it is
+// missing, and the TITLE tag follows the name the person chose.
+func TestApplySheetHonoursATypedFileName(t *testing.T) {
+ p := planFor([]audio.Track{track("01 one.flac", 1, 1, "One")})
+ s := p.Sheet()
+ s.Tracks[0].File = "1-01_a-different-name"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if p.Tracks[0].Target != "1-01_a-different-name.flac" {
+ t.Errorf("target = %q", p.Tracks[0].Target)
+ }
+ if got := value(t, p, 0, tags.FieldTitle); got != "a different name" {
+ t.Errorf("TITLE = %q", got)
+ }
+
+ s = p.Sheet()
+ s.Tracks[0].File = "../../escape.flac"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if strings.Contains(p.Tracks[0].Target, "/") || strings.Contains(p.Tracks[0].Target, "..") {
+ t.Errorf("a typed name escaped the album: %q", p.Tracks[0].Target)
+ }
+}
+
+// An issue the edit fixed has to go away, or the album stays blocked by a
+// complaint about metadata that is no longer there.
+func TestApplySheetClearsIssuesItFixed(t *testing.T) {
+ p := planFor([]audio.Track{{
+ Path: "/staging/01 one.flac",
+ Ext: ".flac",
+ Tags: []tags.Note{
+ {Field: tags.FieldAlbum, Value: "An Album"},
+ {Field: tags.FieldTrackNumber, Value: "1"},
+ {Field: tags.FieldDiscNumber, Value: "1"},
+ },
+ }})
+ if !p.Blocked() {
+ t.Fatal("an album with no artist and no title should be blocked")
+ }
+
+ s := p.Sheet()
+ s.AlbumArtist = "Somebody"
+ s.Tracks[0].Title = "One"
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if p.Blocked() {
+ t.Fatalf("the sheet fixed everything, but the plan is still blocked: %+v", p.Issues)
+ }
+}
+
+// Two tracks given the same title and number in the sheet are caught, because
+// the second would overwrite the first.
+func TestApplySheetCatchesANewClash(t *testing.T) {
+ p := planFor([]audio.Track{
+ track("01 one.flac", 1, 1, "One"),
+ track("02 two.flac", 2, 1, "Two"),
+ })
+ s := p.Sheet()
+ s.Tracks[1].Title = "One"
+ s.Tracks[1].Number = 1
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if !p.Blocked() {
+ t.Fatal("two tracks now target the same file and the plan is not blocked")
+ }
+ if !strings.Contains(firstHard(p), "written twice") {
+ t.Errorf("the issue should be about the clash: %s", firstHard(p))
+ }
+}
+
+// An empty year removes the tag rather than leaving the old one behind.
+func TestApplySheetClearsTheYear(t *testing.T) {
+ p := planFor([]audio.Track{track("01 one.flac", 1, 1, "One")})
+ if err := p.ApplySheet(func() sheet.Sheet { s := p.Sheet(); s.Year = "1987"; return s }()); err != nil {
+ t.Fatal(err)
+ }
+ if got := value(t, p, 0, tags.FieldDate); got != "1987" {
+ t.Fatalf("DATE = %q", got)
+ }
+ s := p.Sheet()
+ s.Year = ""
+ if err := p.ApplySheet(s); err != nil {
+ t.Fatal(err)
+ }
+ if _, ok := tags.Get(p.Tracks[0].Tags, tags.FieldDate); ok {
+ t.Error("clearing the year left a DATE tag behind")
+ }
+}
diff --git a/internal/sheet/sheet.go b/internal/sheet/sheet.go
@@ -0,0 +1,270 @@
+// Package sheet renders and parses the one document a person edits to describe
+// an album before it is filed.
+//
+// It replaces the narrower track-name editor that came first. That one could
+// change the two folder names and the text after the track number, and nothing
+// else: the album title, the album artist, the year, the disc and track numbers
+// and the artist on a single track were all decided from the tags and could only
+// be fixed by retagging the files. Everything intake is about to write is
+// editable here instead, in one file, at any point before the album is filed.
+//
+// The format is deliberately dull: 'field: value' lines, whole-line '#' comments,
+// and '## n' to begin a track. Nothing is positional, so a block can be moved or
+// deleted without the rest shifting underneath it; a value may contain '#' and
+// ':' freely because a comment is only ever a whole line.
+package sheet
+
+import (
+ "errors"
+ "fmt"
+ "strconv"
+ "strings"
+)
+
+// Track is one file in the album. ID is the identity: it is the position of the
+// track in the plan as it was when the sheet was written, and it is what ties an
+// edited block back to the audio it describes. It is deliberately not the file
+// name, which the user may change, nor the track number, which they may also
+// change, nor the row order, which they may sort.
+type Track struct {
+ ID int
+ Title string
+ Artist string
+ Disc int
+ Number int
+ File string // the exact name to write; empty means derive it from the title
+ Source string // shown as a comment; not editable
+ Derived string // the name intake would use with an empty File; a comment
+}
+
+type Sheet struct {
+ Album string
+ AlbumArtist string
+ ArtistDir string
+ AlbumDir string
+ Year string
+ // Cover is a path to an image, the word "placeholder", or "none". Only the
+ // caller knows what to do with it.
+ Cover string
+
+ Tracks []Track
+}
+
+// Changes says which fields differ from the sheet the user started with, so a
+// caller can tell "left alone" from "deliberately set to this value".
+type Changes struct {
+ Album bool
+ AlbumArtist bool
+ ArtistDir bool
+ AlbumDir bool
+ Year bool
+ Cover bool
+}
+
+func (s Sheet) ChangedFrom(before Sheet) Changes {
+ return Changes{
+ Album: s.Album != before.Album,
+ AlbumArtist: s.AlbumArtist != before.AlbumArtist,
+ ArtistDir: s.ArtistDir != before.ArtistDir,
+ AlbumDir: s.AlbumDir != before.AlbumDir,
+ Year: s.Year != before.Year,
+ Cover: s.Cover != before.Cover,
+ }
+}
+
+// Any reports whether anything at all was changed.
+func (c Changes) Any() bool {
+ return c.Album || c.AlbumArtist || c.ArtistDir || c.AlbumDir || c.Year || c.Cover
+}
+
+const header = `# intake album sheet
+#
+# Edit anything here and save. intake throws away everything it read from the
+# tags and uses exactly what this file says, so a wrong album artist or a
+# missing year is fixed here rather than by retagging the files.
+#
+# Whole lines beginning with '#' are comments. A '#' or a ':' inside a value is
+# fine. Save an empty file to leave the album alone.
+#
+# album ALBUM tag, and the folder name unless albumdir says otherwise
+# albumartist ALBUMARTIST tag, and the artist folder unless artistdir says
+# otherwise
+# artistdir the folder under archive/ and library/
+# albumdir the folder inside artistdir
+# year DATE tag, four digits, or empty
+# cover a path to an image, or 'placeholder', or 'none'
+#
+# Each '## n' block is one file. 'n' says which file, and must not change.
+# Delete a whole block to leave that file out of the album.
+#
+# title TITLE tag; the file name is made from it
+# artist ARTIST tag; empty means the album artist
+# disc disc number
+# track track number, which is the number at the front of the file name
+# file the exact name to write; empty means intake names it from the
+# title. Uncomment the commented line to set it by hand.
+`
+
+// Render writes the sheet a person edits.
+func Render(s Sheet) string {
+ var b strings.Builder
+ b.WriteString(header)
+ b.WriteString("\n")
+ fmt.Fprintf(&b, "album: %s\n", s.Album)
+ fmt.Fprintf(&b, "albumartist: %s\n", s.AlbumArtist)
+ fmt.Fprintf(&b, "artistdir: %s\n", s.ArtistDir)
+ fmt.Fprintf(&b, "albumdir: %s\n", s.AlbumDir)
+ fmt.Fprintf(&b, "year: %s\n", s.Year)
+ fmt.Fprintf(&b, "cover: %s\n", s.Cover)
+
+ for _, t := range s.Tracks {
+ fmt.Fprintf(&b, "\n## %d\n", t.ID)
+ if t.Source != "" {
+ fmt.Fprintf(&b, "# from: %s\n", t.Source)
+ }
+ fmt.Fprintf(&b, "title: %s\n", t.Title)
+ fmt.Fprintf(&b, "artist: %s\n", t.Artist)
+ fmt.Fprintf(&b, "disc: %d\n", t.Disc)
+ fmt.Fprintf(&b, "track: %d\n", t.Number)
+ if t.File != "" {
+ fmt.Fprintf(&b, "file: %s\n", t.File)
+ continue
+ }
+ // Commented so that the common case stays empty and the line is there to
+ // uncomment when a file name really should differ from the title.
+ name := t.Derived
+ if name == "" {
+ name = "(from the title)"
+ }
+ fmt.Fprintf(&b, "# file: %s\n", name)
+ }
+ return b.String()
+}
+
+var albumFields = []string{"album", "albumartist", "artistdir", "albumdir", "year", "cover"}
+var trackFields = []string{"title", "artist", "disc", "track", "file"}
+
+// Parse reads an edited sheet back. initial supplies the track identities and
+// every value that was not touched. Only whole lines are comments, so a title
+// containing '#' survives.
+func Parse(text string, initial Sheet) (Sheet, error) {
+ out := initial
+ out.Tracks = make([]Track, len(initial.Tracks))
+ copy(out.Tracks, initial.Tracks)
+ present := make([]bool, len(out.Tracks))
+
+ byID := make(map[int]int, len(out.Tracks))
+ for i, t := range out.Tracks {
+ byID[t.ID] = i
+ }
+
+ current := -1
+ for n, raw := range strings.Split(text, "\n") {
+ line := n + 1
+ trimmed := strings.TrimSpace(strings.TrimRight(raw, "\r"))
+ if trimmed == "" {
+ continue
+ }
+ // '##' before '#' or a track heading looks like a comment.
+ if !strings.HasPrefix(trimmed, "##") && strings.HasPrefix(trimmed, "#") {
+ continue
+ }
+ if strings.HasPrefix(trimmed, "##") {
+ id, err := strconv.Atoi(strings.TrimSpace(strings.TrimPrefix(trimmed, "##")))
+ if err != nil {
+ return Sheet{}, fmt.Errorf("line %d: %q is not a track number", line, trimmed)
+ }
+ i, ok := byID[id]
+ if !ok {
+ return Sheet{}, fmt.Errorf("line %d: there is no track %d; the number after '##' says which file the block is about and must not change", line, id)
+ }
+ if present[i] {
+ return Sheet{}, fmt.Errorf("line %d: track %d appears twice", line, id)
+ }
+ present[i] = true
+ current = i
+ continue
+ }
+
+ key, value, ok := strings.Cut(trimmed, ":")
+ if !ok {
+ return Sheet{}, fmt.Errorf("line %d: %q is not a 'field: value' line and does not begin a track", line, trimmed)
+ }
+ key = strings.ToLower(strings.TrimSpace(key))
+ value = strings.TrimSpace(value)
+
+ if current < 0 {
+ switch key {
+ case "album":
+ out.Album = value
+ case "albumartist":
+ out.AlbumArtist = value
+ case "artistdir":
+ out.ArtistDir = value
+ case "albumdir":
+ out.AlbumDir = value
+ case "year":
+ out.Year = value
+ case "cover":
+ out.Cover = value
+ default:
+ if contains(trackFields, key) {
+ return Sheet{}, fmt.Errorf("line %d: %q belongs inside a track block, after a '## n' line", line, key)
+ }
+ return Sheet{}, fmt.Errorf("line %d: unknown field %q; the album fields are %s", line, key, strings.Join(albumFields, ", "))
+ }
+ continue
+ }
+
+ switch key {
+ case "title":
+ out.Tracks[current].Title = value
+ case "artist":
+ out.Tracks[current].Artist = value
+ case "file":
+ out.Tracks[current].File = value
+ case "disc":
+ v, err := strconv.Atoi(value)
+ if err != nil || v < 1 {
+ return Sheet{}, fmt.Errorf("line %d: disc %q is not a number of 1 or more", line, value)
+ }
+ out.Tracks[current].Disc = v
+ case "track":
+ v, err := strconv.Atoi(value)
+ if err != nil || v < 1 {
+ return Sheet{}, fmt.Errorf("line %d: track %q is not a number of 1 or more", line, value)
+ }
+ out.Tracks[current].Number = v
+ default:
+ if contains(albumFields, key) {
+ return Sheet{}, fmt.Errorf("line %d: %q is an album field and belongs at the top of the file, before the first '## n' line", line, key)
+ }
+ return Sheet{}, fmt.Errorf("line %d: unknown track field %q; the track fields are %s", line, key, strings.Join(trackFields, ", "))
+ }
+ }
+
+ kept := make([]Track, 0, len(out.Tracks))
+ for i, t := range out.Tracks {
+ if !present[i] {
+ continue
+ }
+ t.Title = strings.TrimSpace(t.Title)
+ t.Artist = strings.TrimSpace(t.Artist)
+ t.File = strings.TrimSpace(t.File)
+ kept = append(kept, t)
+ }
+ if len(kept) == 0 {
+ return Sheet{}, errors.New("no tracks are left in the sheet; an empty sheet leaves the album alone")
+ }
+ out.Tracks = kept
+ return out, nil
+}
+
+func contains(list []string, s string) bool {
+ for _, v := range list {
+ if v == s {
+ return true
+ }
+ }
+ return false
+}
diff --git a/internal/sheet/sheet_test.go b/internal/sheet/sheet_test.go
@@ -0,0 +1,160 @@
+package sheet
+
+import (
+ "strings"
+ "testing"
+)
+
+func sample() Sheet {
+ return Sheet{
+ Album: "The Perfect Crime",
+ AlbumArtist: "Straylight Run",
+ ArtistDir: "straylight-run",
+ AlbumDir: "the-perfect-crime",
+ Year: "2004",
+ Cover: "",
+ Tracks: []Track{
+ {ID: 1, Title: "The Perfect Crime #2", Artist: "", Disc: 1, Number: 1, Source: "01 one.flac", Derived: "1-01_the-perfect-crime-2.flac"},
+ {ID: 2, Title: "Handcuffs", Artist: "Someone Else", Disc: 1, Number: 2, Source: "02 two.flac", Derived: "1-02_handcuffs.flac"},
+ {ID: 3, Title: "A/B", Artist: "", Disc: 2, Number: 1, Source: "03 three.flac", Derived: "2-01_a_b.flac"},
+ },
+ }
+}
+
+func TestRenderParseRoundTrip(t *testing.T) {
+ before := sample()
+ got, err := Parse(Render(before), before)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Album != before.Album || got.AlbumArtist != before.AlbumArtist {
+ t.Errorf("album fields changed: %+v", got)
+ }
+ if got.ArtistDir != before.ArtistDir || got.AlbumDir != before.AlbumDir || got.Year != before.Year {
+ t.Errorf("folder fields changed: %+v", got)
+ }
+ if len(got.Tracks) != len(before.Tracks) {
+ t.Fatalf("got %d tracks, want %d", len(got.Tracks), len(before.Tracks))
+ }
+ for i := range before.Tracks {
+ if got.Tracks[i] != before.Tracks[i] {
+ t.Errorf("track %d: got %+v, want %+v", i, got.Tracks[i], before.Tracks[i])
+ }
+ }
+ if c := got.ChangedFrom(before); c.Any() {
+ t.Errorf("a round trip reported changes: %+v", c)
+ }
+}
+
+// A '#' is ordinary text in a title, and a ':' is ordinary text in a title. The
+// old editor stripped an inline ' #' comment, which quietly truncated any title
+// that contained one.
+func TestHashesAndColonsSurviveInValues(t *testing.T) {
+ before := sample()
+ text := Render(before)
+ text = strings.Replace(text, "title: The Perfect Crime #2", "title: Track #4: Reprise #2", 1)
+
+ got, err := Parse(text, before)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.Tracks[0].Title != "Track #4: Reprise #2" {
+ t.Errorf("title = %q", got.Tracks[0].Title)
+ }
+}
+
+// Deleting a block is how a file is left out of the album. The old editor told
+// people a deleted line skipped the file and then filed it anyway, under a name
+// it had already chosen for it.
+func TestDeletingABlockDropsThatTrack(t *testing.T) {
+ before := sample()
+ text := Render(before)
+ lines := strings.Split(text, "\n")
+ var kept []string
+ skip := false
+ for _, l := range lines {
+ if strings.HasPrefix(l, "## 2") {
+ skip = true
+ continue
+ }
+ if skip && strings.HasPrefix(l, "## 3") {
+ skip = false
+ }
+ if !skip {
+ kept = append(kept, l)
+ }
+ }
+
+ got, err := Parse(strings.Join(kept, "\n"), before)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(got.Tracks) != 2 {
+ t.Fatalf("got %d tracks, want 2", len(got.Tracks))
+ }
+ if got.Tracks[0].ID != 1 || got.Tracks[1].ID != 3 {
+ t.Errorf("the wrong tracks survived: %+v", got.Tracks)
+ }
+}
+
+func TestParsingRefusesDamage(t *testing.T) {
+ before := sample()
+ cases := []struct {
+ name string
+ edit func(string) string
+ want string
+ }{
+ {"empty", func(string) string { return "" }, "no tracks"},
+ {"unknown id", func(s string) string { return strings.Replace(s, "## 3", "## 9", 1) }, "no track 9"},
+ {"duplicate id", func(s string) string { return strings.Replace(s, "## 3", "## 1", 1) }, "appears twice"},
+ {"unknown album field", func(s string) string { return "genre: metal\n" + s }, "unknown field"},
+ {"unknown track field", func(s string) string { return strings.Replace(s, "## 1\n", "## 1\nlabel: x\n", 1) }, "unknown track field"},
+ {"track field at the top", func(s string) string { return "title: x\n" + s }, "belongs inside a track block"},
+ {"album field in a block", func(s string) string { return strings.Replace(s, "## 1\n", "## 1\nalbum: x\n", 1) }, "belongs at the top"},
+ {"junk line", func(s string) string { return "nonsense\n" + s }, "not a 'field: value' line"},
+ {"bad disc", func(s string) string { return strings.Replace(s, "disc: 1", "disc: 0", 1) }, "disc"},
+ {"bad track", func(s string) string { return strings.Replace(s, "track: 1", "track: x", 1) }, "track"},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := Parse(tc.edit(Render(before)), before)
+ if err == nil {
+ t.Fatal("parsing should have failed")
+ }
+ if !strings.Contains(err.Error(), tc.want) {
+ t.Errorf("error %q does not mention %q", err, tc.want)
+ }
+ })
+ }
+}
+
+func TestChangesAreReported(t *testing.T) {
+ before := sample()
+ after := before
+ after.Year = "2005"
+ after.Cover = "none"
+
+ got := after.ChangedFrom(before)
+ if !got.Year || !got.Cover || got.Album || got.ArtistDir || got.AlbumArtist || got.AlbumDir {
+ t.Errorf("changes = %+v", got)
+ }
+ if !got.Any() {
+ t.Error("Any should be true")
+ }
+ same := before
+ if same.ChangedFrom(before).Any() {
+ t.Error("nothing changed, but Any is true")
+ }
+}
+
+// The rendered file has to say what intake will name each file, because the name
+// is not obvious from the title once normalisation has had it.
+func TestRenderShowsTheDerivedName(t *testing.T) {
+ text := Render(sample())
+ if !strings.Contains(text, "1-01_the-perfect-crime-2.flac") {
+ t.Errorf("the derived name is missing:\n%s", text)
+ }
+ if !strings.Contains(text, "# from: 01 one.flac") {
+ t.Errorf("the source file is missing:\n%s", text)
+ }
+}
diff --git a/internal/tui/editor.go b/internal/tui/editor.go
@@ -9,10 +9,13 @@ import (
tea "charm.land/bubbletea/v2"
- "intake/internal/names"
+ "intake/internal/sheet"
)
-type editorFinishedMsg struct{ err error }
+type editorFinishedMsg struct {
+ err error
+ before sheet.Sheet
+}
func editorCommand() string {
for _, k := range []string{"VISUAL", "EDITOR"} {
@@ -23,75 +26,119 @@ func editorCommand() string {
return "vi"
}
-func (m Module) openNameEditor() (tea.Model, tea.Cmd) {
- if m.pl.ArchiveDir == "" && len(m.pl.Tracks) == 0 {
+// openSheetEditor hands the whole album to $EDITOR.
+//
+// It is available from every screen that has a plan loaded except the one that
+// is filing it: deciding on a cover is exactly when somebody notices that the
+// album artist is wrong, and having to back out of the picker first, or quit and
+// start over, would make the editor useless at the moment it is most wanted.
+func (m Module) openSheetEditor() (tea.Model, tea.Cmd) {
+ if len(m.pl.Tracks) == 0 {
m.status = "nothing to edit yet"
return m, nil
}
- session := m.pl.EditSession()
- if len(session.Tracks) == 0 {
- m.status = "nothing to edit"
- return m, nil
- }
+ current := m.pl.Sheet()
+ current.Cover = m.coverDescription()
- dir, err := os.MkdirTemp("", "intake-names-")
- if err != nil {
- m.status = "could not create a temporary file: " + err.Error()
+ path := filepath.Join(m.opts.Config.StateDir, "sheet.txt")
+ if m.opts.Config.StateDir == "" {
+ dir, err := os.MkdirTemp("", "intake-sheet-")
+ if err != nil {
+ m.status = "could not create a temporary file: " + err.Error()
+ return m, nil
+ }
+ path = filepath.Join(dir, "sheet.txt")
+ } else if err := os.MkdirAll(m.opts.Config.StateDir, 0o755); err != nil {
+ m.status = "could not create " + m.opts.Config.StateDir + ": " + err.Error()
return m, nil
}
- path := filepath.Join(dir, "names.txt")
- if err := os.WriteFile(path, []byte(names.Render(session)), 0o644); err != nil {
+ if err := os.WriteFile(path, []byte(sheet.Render(current)), 0o644); err != nil {
m.status = "could not write " + path + ": " + err.Error()
return m, nil
}
- m.namesPath = path
- m.namesSession = &session
+ m.sheetPath = path
+ m.sheetBefore = ¤t
cmd := exec.Command("sh", "-c", editorCommand()+" "+shellQuote(path))
return m, tea.ExecProcess(cmd, func(err error) tea.Msg {
- return editorFinishedMsg{err: err}
+ return editorFinishedMsg{err: err, before: current}
})
}
-func (m *Module) applyEditedNames() {
- path, session := m.namesPath, m.namesSession
- if path == "" || session == nil {
+// coverDescription is what the sheet's cover field shows: the cover already
+// chosen, or nothing.
+func (m Module) coverDescription() string {
+ switch {
+ case m.hasCover && m.cover.Placeholder:
+ return "placeholder"
+ case m.hasCover && m.cover.Path != "":
+ return m.cover.Path
+ default:
+ return ""
+ }
+}
+
+func (m *Module) applyEditedSheet(msg editorFinishedMsg) {
+ path, before := m.sheetPath, msg.before
+ if path == "" {
return
}
defer func() {
os.Remove(path)
- m.namesPath, m.namesSession = "", nil
+ m.sheetPath, m.sheetBefore = "", nil
}()
- edited, err := os.ReadFile(path)
+ text, err := os.ReadFile(path)
if err != nil {
- m.status = "could not read the edited names: " + err.Error()
+ m.status = "could not read the edited sheet: " + err.Error()
return
}
- res, err := names.Parse(string(edited), *session)
+ next, err := sheet.Parse(string(text), before)
if err != nil {
m.status = err.Error()
return
}
- if dupes := names.Duplicates(res.Tracks); len(dupes) > 0 {
- for target, sources := range dupes {
- m.status = fmt.Sprintf("two tracks would be named %q (from %s)", target, joinComma(sources))
+ if err := m.pl.ApplySheet(next); err != nil {
+ m.status = err.Error()
+ return
+ }
+
+ // A cover typed into the sheet replaces whatever was chosen and one set to
+ // "none" clears it; a cover left alone is left alone.
+ if next.ChangedFrom(before).Cover {
+ if err := m.applySheetCover(next.Cover); err != nil {
+ m.status = err.Error()
return
}
}
- m.pl.ApplyNames(res)
- m.status = "names updated"
+
+ m.status = fmt.Sprintf("album updated: %d track%s", len(m.pl.Tracks), plural(len(m.pl.Tracks)))
}
-func joinComma(in []string) string {
- out := ""
- for i, s := range in {
- if i > 0 {
- out += ", "
+func (m *Module) applySheetCover(value string) error {
+ switch strings.ToLower(strings.TrimSpace(value)) {
+ case "":
+ return nil
+ case "none":
+ m.cover, m.hasCover = CoverOption{}, false
+ return nil
+ case "placeholder":
+ m.cover = CoverOption{Source: "placeholder", Label: "placeholder", Placeholder: true}
+ m.hasCover = true
+ return nil
+ }
+ path := strings.TrimSpace(value)
+ if strings.HasPrefix(path, "~/") {
+ if home, err := os.UserHomeDir(); err == nil {
+ path = filepath.Join(home, path[2:])
}
- out += s
}
- return out
+ if _, err := os.Stat(path); err != nil {
+ return fmt.Errorf("the cover in the sheet cannot be read: %v", err)
+ }
+ m.cover = CoverOption{Path: path, Source: "file", Label: filepath.Base(path)}
+ m.hasCover = true
+ return nil
}
func shellQuote(s string) string {
diff --git a/internal/tui/tui.go b/internal/tui/tui.go
@@ -12,9 +12,9 @@ import (
"charm.land/lipgloss/v2"
"intake/internal/config"
- "intake/internal/names"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
)
type Screen int
@@ -104,8 +104,8 @@ type Module struct {
coversFetched bool
placed *kittyPlacement
- namesPath string
- namesSession *names.Session
+ sheetPath string
+ sheetBefore *sheet.Sheet
events chan tea.Msg
@@ -298,7 +298,7 @@ func (m Module) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.status = "editor exited with an error: " + msg.err.Error()
return m, nil
}
- m.applyEditedNames()
+ m.applyEditedSheet(msg)
return m, nil
case progressMsg:
@@ -494,10 +494,7 @@ func (m Module) keyReview(key string) (tea.Model, tea.Cmd) {
m.screen = ScreenBrowse
return m, nil
case "e":
- if m.step == StepTracks || m.step == StepMetadata {
- return m.openNameEditor()
- }
- return m, nil
+ return m.openSheetEditor()
case "enter":
return m.nextStep()
}
@@ -547,6 +544,10 @@ func (m Module) keyCover(key string) (tea.Model, tea.Cmd) {
switch key {
case "q":
return m.quit()
+ case "e":
+ // The whole album stays editable while a cover is being chosen: this is
+ // where a wrong album artist is usually noticed.
+ return m.openSheetEditor()
case "esc":
m.screen = ScreenReview
m.scroll = 0
@@ -618,7 +619,7 @@ func (m *Module) resetAlbum() {
m.cover, m.hasCover = CoverOption{}, false
m.covers, m.coverCursor, m.coversFetched = nil, 0, false
m.placed = nil
- m.namesPath, m.namesSession = "", nil
+ m.sheetPath, m.sheetBefore = "", nil
m.log, m.outcome = nil, Outcome{}
m.err, m.status = nil, ""
m.scroll, m.offset = 0, 0
@@ -751,13 +752,9 @@ func (m Module) renderFooter() string {
hints = []string{"↑↓", "move", "/", "filter", "enter", "review", "r", "refresh", "q", "quit"}
}
case ScreenReview:
- hints = []string{"↑↓", "scroll", "enter", "continue"}
- if m.step == StepTracks || m.step == StepMetadata {
- hints = append(hints, "e", "edit names")
- }
- hints = append(hints, "esc", "back", "q", "quit")
+ hints = []string{"↑↓", "scroll", "enter", "continue", "e", "edit album", "esc", "back", "q", "quit"}
case ScreenCoverPicker:
- hints = []string{"↑↓", "choose", "enter", "use this cover", "esc", "cancel"}
+ hints = []string{"↑↓", "choose", "enter", "use this cover", "e", "edit album", "esc", "cancel"}
case ScreenWorking:
hints = []string{"q", "stop watching"}
case ScreenDone:
diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go
@@ -3,6 +3,8 @@ package tui
import (
"context"
"fmt"
+ "os"
+ "path/filepath"
"strings"
"testing"
@@ -455,3 +457,83 @@ func TestDoneScreenExplainsPlaceholderRecovery(t *testing.T) {
type errBoom struct{}
func (errBoom) Error() string { return "boom" }
+
+// Everything about an album stays editable until it is actually being filed.
+// Deciding on a cover is exactly when a wrong album artist gets noticed, so the
+// editor has to be reachable from the picker too, without backing out first.
+func TestTheSheetEditorIsReachableFromEveryReviewScreen(t *testing.T) {
+ if mode := testModule(120, 40); mode.mode() == modeTiny {
+ t.Skip("the module needs a normal-sized terminal")
+ }
+
+ for _, screen := range []Screen{ScreenReview, ScreenCoverPicker} {
+ for _, step := range []Step{StepMetadata, StepTracks, StepCover, StepConfirm} {
+ m := testModule(120, 40)
+ m.opts.Config.StateDir = t.TempDir()
+ m.pl = readyPlan()
+ m.sheetPath, m.sheetBefore = "", nil
+ m.current = scan.Album{Name: "boards", Dir: m.pl.Album.Dir, Status: scan.StatusReady}
+ m.screen, m.step = screen, step
+ m.covers = []CoverOption{{Path: "/tmp/cover.jpg", Source: "folder"}}
+
+ got, cmd := m.handleKey(key("e"))
+ m = got.(Module)
+ if m.sheetPath == "" {
+ t.Errorf("screen %v step %v: 'e' did not open the album sheet", screen, step)
+ }
+ if cmd == nil {
+ t.Errorf("screen %v step %v: 'e' produced no command", screen, step)
+ }
+ if m.status != "" && strings.Contains(m.status, "nothing") {
+ t.Errorf("screen %v step %v: %s", screen, step, m.status)
+ }
+ }
+ }
+}
+
+// The sheet is written with the cover that is already chosen, so saving without
+// touching the cover field does not silently drop it.
+func TestTheSheetShowsTheChosenCover(t *testing.T) {
+ m := testModule(120, 40)
+ m.pl = readyPlan()
+ m.cover = CoverOption{Path: "/tmp/front.jpg", Source: "folder"}
+ m.hasCover = true
+ if got := m.coverDescription(); got != "/tmp/front.jpg" {
+ t.Errorf("cover field = %q", got)
+ }
+ m.cover = CoverOption{Placeholder: true}
+ if got := m.coverDescription(); got != "placeholder" {
+ t.Errorf("cover field = %q", got)
+ }
+ m.cover, m.hasCover = CoverOption{}, false
+ if got := m.coverDescription(); got != "" {
+ t.Errorf("cover field = %q", got)
+ }
+}
+
+// A cover typed into the sheet wins over the picker, and 'none' clears whatever
+// was chosen.
+func TestACoverTypedIntoTheSheetIsApplied(t *testing.T) {
+ dir := t.TempDir()
+ img := filepath.Join(dir, "front.jpg")
+ if err := os.WriteFile(img, []byte("not really a jpeg"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+
+ m := testModule(120, 40)
+ if err := m.applySheetCover(img); err != nil {
+ t.Fatal(err)
+ }
+ if !m.hasCover || m.cover.Path != img {
+ t.Errorf("the cover was not applied: %+v", m.cover)
+ }
+ if err := m.applySheetCover("none"); err != nil {
+ t.Fatal(err)
+ }
+ if m.hasCover {
+ t.Error("'none' did not clear the cover")
+ }
+ if err := m.applySheetCover("/does/not/exist.jpg"); err == nil {
+ t.Error("a cover that cannot be read should be reported")
+ }
+}