feat: keep an album's edits between runs
An edited album used to live only in memory, so quitting the interface, or opening a different album and coming back, threw away every correction that had been typed. The sheet is already a complete description of what intake is about to write, so keeping it on disk is all that is needed to make a review resumable. Edits are saved under <state>/review/, named after the staging folder plus a short digest of the whole path, so a directory listing stays readable while two albums with the same folder name in different places cannot collide. Opening an album applies the saved sheet and says so; the interface offers 'D' to discard it; and filing the album drops it, because leaving it would apply last time's corrections to a fresh folder of the same name. The same store makes the command line repeatable: 'intake album <name>' twice in a row does the same thing the second time rather than starting from the tags again. A saved sheet that no longer fits the album -- the files changed underneath it -- is discarded with a note rather than left to fail on every run. Closes fb2c905
diff --git a/README.md b/README.md
@@ -29,6 +29,8 @@ produces a clean, consistently tagged, consistently named library: lossless mast
- Transcodes a streaming copy from the masters and optimises its cover
- Remembers artist folder decisions in `artists.toml` and the journal, so the same
artist always lands in the same folder
+- Keeps the edits you make to an album, so quitting mid-review or coming back to it later
+ does not lose them
- Notices when an album is already in the library — by artist and name, not just by path —
before the review starts, and offers keep both, replace or skip
- `b` on the album list files everything ready in one unattended pass, parking whatever
diff --git a/internal/cli/cli.go b/internal/cli/cli.go
@@ -448,6 +448,14 @@ func (a *App) Transcode(ctx context.Context, target string) error {
return nil
}
+// fixLibrary rebuilds an album's library copy from its masters, when the audit
+// was asked to repair what it finds. It reports whether it succeeded, and never
+// touches the archive: the masters are the one thing a rebuild cannot replace.
+
+// fixLibraryCover puts a cover.jpg back into a library copy that has lost one,
+// without re-encoding the audio: the archive still has the cover it was built
+// from, so this is a copy rather than a rebuild.
+
type auditFinding struct {
Album string `json:"album"`
Dir string `json:"dir"`
diff --git a/internal/cli/file.go b/internal/cli/file.go
@@ -13,6 +13,7 @@ import (
"intake/internal/library"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/state"
)
@@ -222,7 +223,37 @@ 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
+ p := plan.BuildWithArtist(al, a.Cfg.Archive(), a.Cfg.Library(), key, a.Cfg.DiscPrefix)
+ a.applySavedSheet(&p)
+ return p, nil
+}
+
+// applySavedSheet puts back the edits made to this album on an earlier run.
+//
+// The sheet is a complete description of what intake is about to write, so
+// keeping it is all that is needed to make a review resumable: `intake album
+// <name>` twice in a row does the same thing the second time, and quitting the
+// interface in the middle of a correction no longer throws it away.
+func (a *App) applySavedSheet(p *plan.Plan) {
+ text, ok, err := a.sheetStore().Load(p.Album.Dir)
+ if err != nil || !ok {
+ return
+ }
+ before := p.Sheet()
+ next, err := sheet.Parse(text, before)
+ if err != nil {
+ // A saved sheet that no longer fits -- the files changed underneath it --
+ // is dropped rather than left to fail on every run.
+ a.sheetStore().Delete(p.Album.Dir)
+ a.logf("note: the saved album sheet no longer fits this album and was discarded: %v", err)
+ return
+ }
+ if err := p.ApplySheet(next); err != nil {
+ a.sheetStore().Delete(p.Album.Dir)
+ a.logf("note: the saved album sheet could not be applied and was discarded: %v", err)
+ return
+ }
+ a.logf("applied the album sheet saved from an earlier run")
}
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
@@ -408,9 +408,21 @@ func (a *App) editSheet(p *plan.Plan) (*sheet.Sheet, error) {
if err := p.ApplySheet(next); err != nil {
return nil, fmt.Errorf("%s: %w", path, err)
}
+ // Kept, so that a second run on the same album starts from the same place
+ // rather than from the tags again.
+ if saved, err := a.sheetStore().Save(p.Album.Dir, string(edited)); err != nil {
+ fmt.Fprintf(a.Err, "warning: could not save the album sheet: %v\n", err)
+ } else if saved != "" {
+ a.logf("saved to %s", saved)
+ }
return &next, nil
}
+// sheetStore is where an album's edits are kept between runs.
+func (a *App) sheetStore() sheet.Store {
+ return sheet.Store{Dir: filepath.Join(a.Cfg.StateDir, "review")}
+}
+
// 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
diff --git a/internal/cli/journal_test.go b/internal/cli/journal_test.go
@@ -15,7 +15,9 @@ import (
"intake/internal/config"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/state"
+ "intake/internal/tags"
)
func cliApp(t *testing.T, root string) (*App, *bytes.Buffer) {
@@ -879,3 +881,74 @@ func TestMergingAnArtistFolder(t *testing.T) {
t.Errorf("the refused merge moved something anyway: %v", err)
}
}
+
+// A review that is interrupted should not throw away the corrections. The sheet
+// is already a complete description of what intake is about to write, so keeping
+// it is all that is needed.
+func TestSavedSheetSurvivesBetweenRuns(t *testing.T) {
+ needTools(t, "ffmpeg", "metaflac")
+ root := t.TempDir()
+ a, _ := cliApp(t, root)
+
+ p := plan.Plan{
+ Album: scan.Album{Dir: filepath.Join(root, "staging", "an-album")},
+ AlbumName: "An Album",
+ AlbumArt: "An Artist",
+ ArtistDir: "an-artist",
+ AlbumDir: "an-album",
+ Tracks: []plan.TrackPlan{{
+ Source: filepath.Join(root, "staging", "an-album", "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.FieldAlbumArtist, Value: "An Artist"},
+ {Field: tags.FieldTrackNumber, Value: "1"},
+ },
+ }},
+ }
+
+ // Stand in for an editor that fixed the album artist and the title.
+ edited := p.Sheet()
+ edited.AlbumArtist = "The Right Artist"
+ edited.ArtistDir = "the-right-artist"
+ edited.Tracks[0].Title = "One, Reprised"
+ if _, err := a.sheetStore().Save(p.Album.Dir, sheet.Render(edited)); err != nil {
+ t.Fatal(err)
+ }
+
+ // A later run on the same album starts from the saved sheet.
+ again := p
+ a.applySavedSheet(&again)
+ if again.AlbumArt != "The Right Artist" || again.ArtistDir != "the-right-artist" {
+ t.Errorf("the saved sheet was not applied: %+v", again)
+ }
+ if again.Tracks[0].Title != "One, Reprised" {
+ t.Errorf("title = %q", again.Tracks[0].Title)
+ }
+ if again.Tracks[0].Target != "1-01_one-reprised.flac" {
+ t.Errorf("target = %q", again.Tracks[0].Target)
+ }
+
+ // A saved sheet that no longer fits the album is dropped rather than left to
+ // fail on every run.
+ if _, err := a.sheetStore().Save(p.Album.Dir, "## 9\ntitle: nonsense\n"); err != nil {
+ t.Fatal(err)
+ }
+ fresh := p
+ a.applySavedSheet(&fresh)
+ if fresh.Tracks[0].Title != "One" {
+ t.Errorf("a sheet that does not fit changed the plan: %q", fresh.Tracks[0].Title)
+ }
+ if _, ok, _ := a.sheetStore().Load(p.Album.Dir); ok {
+ t.Error("the sheet that did not fit was kept")
+ }
+}
+
+// An audit that only reports leaves the same chore every time. --fix rebuilds
+// what it can, and never touches the archive: the masters are the one thing a
+// rebuild cannot replace.
diff --git a/internal/sheet/store.go b/internal/sheet/store.go
@@ -0,0 +1,83 @@
+package sheet
+
+import (
+ "crypto/sha256"
+ "encoding/hex"
+ "fmt"
+ "os"
+ "path/filepath"
+
+ "intake/internal/normalize"
+)
+
+// Store keeps an album's edited sheet between runs.
+//
+// An edited album used to live only in memory, so quitting the interface, or
+// opening a different album and coming back, threw away every correction that
+// had been typed. The sheet is already a complete description of what intake is
+// about to write, so keeping it on disk is all that is needed to make a review
+// resumable, and it is also what makes `intake album <name>` twice in a row do
+// the same thing the second time.
+//
+// The file is named after the staging folder plus a short digest of the whole
+// path, so a directory listing stays readable while two albums with the same
+// folder name in different places cannot collide.
+type Store struct {
+ Dir string
+}
+
+func (s Store) Path(albumDir string) string {
+ base := normalize.Name(filepath.Base(albumDir))
+ if base == "" {
+ base = "album"
+ }
+ sum := sha256.Sum256([]byte(filepath.Clean(albumDir)))
+ return filepath.Join(s.Dir, fmt.Sprintf("%s-%s.txt", base, hex.EncodeToString(sum[:4])))
+}
+
+// Save writes the sheet as it stands. It returns the path it wrote, for saying
+// where the edits went.
+func (s Store) Save(albumDir string, text string) (string, error) {
+ if s.Dir == "" {
+ return "", nil
+ }
+ if err := os.MkdirAll(s.Dir, 0o755); err != nil {
+ return "", err
+ }
+ path := s.Path(albumDir)
+ tmp := path + ".tmp"
+ if err := os.WriteFile(tmp, []byte(text), 0o644); err != nil {
+ return "", err
+ }
+ if err := os.Rename(tmp, path); err != nil {
+ os.Remove(tmp)
+ return "", err
+ }
+ return path, nil
+}
+
+// Load returns the saved sheet text. The second result is false when there is
+// none, which is the ordinary case rather than an error: most albums are looked
+// at once and filed.
+func (s Store) Load(albumDir string) (string, bool, error) {
+ if s.Dir == "" {
+ return "", false, nil
+ }
+ data, err := os.ReadFile(s.Path(albumDir))
+ if err != nil {
+ if os.IsNotExist(err) {
+ return "", false, nil
+ }
+ return "", false, err
+ }
+ return string(data), true, nil
+}
+
+// Delete drops a saved sheet, once the album it describes has been filed or the
+// person looking at it has said to forget it.
+func (s Store) Delete(albumDir string) {
+ if s.Dir == "" {
+ return
+ }
+ os.Remove(s.Path(albumDir))
+}
diff --git a/internal/sheet/store_test.go b/internal/sheet/store_test.go
@@ -0,0 +1,87 @@
+package sheet
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// The saved sheet is what makes a review resumable, so it has to survive being
+// written and read back exactly.
+func TestStoreRoundTrips(t *testing.T) {
+ dir := t.TempDir()
+ s := Store{Dir: filepath.Join(dir, "review")}
+ album := filepath.Join(dir, "staging", "Boards of Canada - Geogaddi")
+ before := sample()
+
+ if _, ok, err := s.Load(album); err != nil || ok {
+ t.Fatalf("an album with no saved sheet should report none: %v %v", ok, err)
+ }
+ path, err := s.Save(album, Render(before))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !strings.HasPrefix(filepath.Base(path), "boards-of-canada-geogaddi-") {
+ t.Errorf("the stored name %q is not readable", filepath.Base(path))
+ }
+ text, ok, err := s.Load(album)
+ if err != nil || !ok {
+ t.Fatalf("Load: %v %v", ok, err)
+ }
+ got, err := Parse(text, before)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got.ChangedFrom(before).Any() || len(got.Tracks) != len(before.Tracks) {
+ t.Errorf("the round trip changed the sheet: %+v", got)
+ }
+
+ s.Delete(album)
+ if _, ok, _ := s.Load(album); ok {
+ t.Error("Delete left the sheet behind")
+ }
+}
+
+// Two albums with the same folder name in different places must not share a
+// saved sheet.
+func TestStoreKeepsSameNamedAlbumsApart(t *testing.T) {
+ dir := t.TempDir()
+ s := Store{Dir: dir}
+ a := filepath.Join(dir, "one", "Boards of Canada - Geogaddi")
+ b := filepath.Join(dir, "two", "Boards of Canada - Geogaddi")
+ if s.Path(a) == s.Path(b) {
+ t.Fatalf("both albums map to %s", s.Path(a))
+ }
+ if _, err := s.Save(a, "album: One\n"); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := s.Save(b, "album: Two\n"); err != nil {
+ t.Fatal(err)
+ }
+ ta, _, _ := s.Load(a)
+ tb, _, _ := s.Load(b)
+ if !strings.Contains(ta, "One") || !strings.Contains(tb, "Two") {
+ t.Errorf("the two sheets got mixed up: %q and %q", ta, tb)
+ }
+}
+
+// A store with nowhere to write is not an error: a configuration without a state
+// directory simply has no resumable reviews.
+func TestStoreWithNoDirectory(t *testing.T) {
+ s := Store{}
+ if _, err := s.Save("/x", "y"); err != nil {
+ t.Errorf("Save: %v", err)
+ }
+ if _, ok, err := s.Load("/x"); err != nil || ok {
+ t.Errorf("Load: %v %v", ok, err)
+ }
+ s.Delete("/x")
+ if p := s.Path("/x"); !strings.HasPrefix(p, "x-") || !strings.HasSuffix(p, ".txt") {
+ t.Errorf("Path = %q", p)
+ }
+ // Nothing was created anywhere.
+ if _, err := os.Stat(filepath.Join(os.TempDir(), "review")); err == nil {
+ t.Error("a store with no directory wrote something")
+ }
+}
diff --git a/internal/tui/editor.go b/internal/tui/editor.go
@@ -65,6 +65,41 @@ func (m Module) openSheetEditor() (tea.Model, tea.Cmd) {
})
}
+// sheetStore is where an album's edits are kept between runs.
+func (m Module) sheetStore() sheet.Store {
+ return sheet.Store{Dir: filepath.Join(m.opts.Config.StateDir, "review")}
+}
+
+// restoreSavedEdits applies the sheet saved for this album, if there is one, so
+// that quitting in the middle of a review does not throw the corrections away.
+func (m *Module) restoreSavedEdits() bool {
+ text, ok, err := m.sheetStore().Load(m.current.Dir)
+ if err != nil || !ok {
+ return false
+ }
+ before := m.pl.Sheet()
+ next, err := sheet.Parse(text, before)
+ if err != nil {
+ // A saved sheet that no longer fits the album -- the files changed under
+ // it -- is dropped rather than left to fail on every open.
+ m.sheetStore().Delete(m.current.Dir)
+ m.status = "could not restore your saved edits: " + err.Error()
+ return false
+ }
+ if err := m.pl.ApplySheet(next); err != nil {
+ m.sheetStore().Delete(m.current.Dir)
+ m.status = "could not restore your saved edits: " + err.Error()
+ return false
+ }
+ m.restored = true
+ if next.ChangedFrom(before).Cover {
+ if err := m.applySheetCover(next.Cover); err != nil {
+ m.status = err.Error()
+ }
+ }
+ return true
+}
+
// coverDescription is what the sheet's cover field shows: the cover already
// chosen, or nothing.
func (m Module) coverDescription() string {
@@ -102,6 +137,11 @@ func (m *Module) applyEditedSheet(msg editorFinishedMsg) {
m.status = err.Error()
return
}
+ // Kept so that quitting, or opening another album and coming back, does not
+ // lose what was just typed.
+ if _, err := m.sheetStore().Save(m.current.Dir, string(text)); err != nil {
+ m.status = "the album was updated but your edits could not be saved: " + err.Error()
+ }
// A cover typed into the sheet replaces whatever was chosen and one set to
// "none" clears it; a cover left alone is left alone.
diff --git a/internal/tui/tui.go b/internal/tui/tui.go
@@ -155,9 +155,11 @@ type Module struct {
coversFetched bool
placed *kittyPlacement
- attention map[string]string
- existing *Existing
- collision string
+ attention map[string]string
+ existing *Existing
+ collision string
+ // restored is set when the album's saved edits were applied on opening it.
+ restored bool
batch *BatchReport
journal []JournalEntry
journalCursor int
@@ -399,6 +401,9 @@ func (m Module) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.current, m.pl = msg.album, msg.plan
m.screen, m.step, m.scroll = ScreenReview, StepMetadata, 0
m.status = fmt.Sprintf("%d track%s read", len(msg.plan.Tracks), plural(len(msg.plan.Tracks)))
+ if m.restoreSavedEdits() {
+ m.status = "restored the edits you saved for this album; press D to discard them"
+ }
if m.opts.Existing != nil {
return m, tea.Batch(clear, m.existingCmd(msg.plan))
}
@@ -458,6 +463,13 @@ func (m Module) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
return m, nil
}
}
+ if msg.batch == nil {
+ // The album is filed, so the edits it was built from have done their
+ // job; leaving them would apply last time's corrections to a fresh
+ // folder of the same name.
+ m.sheetStore().Delete(m.current.Dir)
+ m.restored = false
+ }
m.outcome, m.batch = msg.outcome, msg.batch
m.screen = ScreenDone
m.scroll = 0
@@ -775,6 +787,13 @@ func (m Module) keyReview(key string) (tea.Model, tea.Cmd) {
return m, nil
case "e":
return m.openSheetEditor()
+ case "D":
+ if m.restored {
+ m.sheetStore().Delete(m.current.Dir)
+ m.restored = false
+ m.status = "discarded the saved edits; reopen the album to start from the tags again"
+ return m, nil
+ }
case "enter":
return m.nextStep()
}
@@ -904,7 +923,7 @@ func (m *Module) resetAlbum() {
m.placed = nil
m.sheetPath, m.sheetBefore = "", nil
m.journalMenu, m.journalAction, m.journalConfirm = -1, 0, false
- m.batch, m.existing, m.collision = nil, nil, ""
+ m.batch, m.existing, m.collision, m.restored = nil, nil, "", false
m.log, m.outcome = nil, Outcome{}
m.err, m.status = nil, ""
m.scroll, m.offset = 0, 0
@@ -1619,7 +1638,7 @@ func (m Module) keyJournalMenu(keyp string) (tea.Model, tea.Cmd) {
switch keyp {
case "esc", "q":
m.journalMenu, m.journalAction, m.journalConfirm = -1, 0, false
- m.batch, m.existing, m.collision = nil, nil, ""
+ m.batch, m.existing, m.collision, m.restored = nil, nil, "", false
return m, nil
case "up", "k":
if m.journalAction > 0 {
@@ -1649,6 +1668,6 @@ func (m Module) keyJournalMenu(keyp string) (tea.Model, tea.Cmd) {
func (m Module) runJournalAction(a journalAction) (tea.Model, tea.Cmd) {
entry := m.journal[m.journalCursor]
m.journalMenu, m.journalAction, m.journalConfirm = -1, 0, false
- m.batch, m.existing, m.collision = nil, nil, ""
+ m.batch, m.existing, m.collision, m.restored = nil, nil, "", false
return m, a.run(&m, entry)
}
diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go
@@ -14,6 +14,7 @@ import (
"intake/internal/config"
"intake/internal/plan"
"intake/internal/scan"
+ "intake/internal/sheet"
"intake/internal/tags"
)
@@ -1145,3 +1146,58 @@ func TestTinyHelpIsOneLine(t *testing.T) {
t.Errorf("the tiny help should say what it is: %q", out)
}
}
+
+// Restoring the saved edits is what makes a review resumable, and the album says
+// so rather than silently applying corrections from an earlier session.
+func TestSavedEditsAreRestoredOnOpeningAnAlbum(t *testing.T) {
+ m := testModule(120, 40)
+ m.opts.Config.StateDir = t.TempDir()
+ m.pl = readyPlan()
+ m.current = scan.Album{Name: "boards", Dir: m.pl.Album.Dir, Status: scan.StatusReady}
+
+ edited := m.pl.Sheet()
+ edited.AlbumArtist = "The Right Artist"
+ if _, err := m.sheetStore().Save(m.current.Dir, sheet.Render(edited)); err != nil {
+ t.Fatal(err)
+ }
+
+ gotModel, _ := m.Update(planMsg{album: m.current, plan: readyPlan()})
+ m = gotModel.(Module)
+ if m.pl.AlbumArt != "The Right Artist" {
+ t.Fatalf("the saved edits were not applied: %q", m.pl.AlbumArt)
+ }
+ if !m.restored {
+ t.Error("the album does not know its edits were restored")
+ }
+ if !strings.Contains(m.status, "restored") {
+ t.Errorf("status = %q", m.status)
+ }
+
+ // And they can be thrown away.
+ got, _ := m.handleKey(key("D"))
+ m = got.(Module)
+ if m.restored {
+ t.Error("'D' did not discard the saved edits")
+ }
+ if _, ok, _ := m.sheetStore().Load(m.current.Dir); ok {
+ t.Error("the saved sheet is still on disk")
+ }
+}
+
+// Once the album is filed the edits have done their job: leaving them would apply
+// last time's corrections to a fresh folder of the same name.
+func TestSavedEditsAreDroppedAfterFiling(t *testing.T) {
+ m := testModule(120, 40)
+ m.opts.Config.StateDir = t.TempDir()
+ m.pl = readyPlan()
+ m.current = scan.Album{Name: "boards", Dir: m.pl.Album.Dir, Status: scan.StatusReady}
+ if _, err := m.sheetStore().Save(m.current.Dir, sheet.Render(m.pl.Sheet())); err != nil {
+ t.Fatal(err)
+ }
+
+ gotModel, _ := m.Update(doneMsg{outcome: Outcome{Files: 1}})
+ m = gotModel.(Module)
+ if _, ok, _ := m.sheetStore().Load(m.current.Dir); ok {
+ t.Error("the saved sheet outlived the filing")
+ }
+}