Clean up the temporary directory the cover previewer stages into
stageTempImage made a directory per image and only removed it on the error path. Nothing else ever cleaned up, and kitty does not delete the file for the file medium, so every cover previewed in a picker session left a directory behind. Thirty-two of them had accumulated in /tmp on this machine before the fix. The previewer now owns a single directory for the session, created on first use, holding one file per source image rather than reusing a fixed name. Close removes it, and the runner defers that call so ctrl+c and error exits clean up as well as an ordinary quit. Closes 35adbdc68a4df586175a298c7617a4686977d5a3
diff --git a/internal/tui/kitty.go b/internal/tui/kitty.go
@@ -2,6 +2,7 @@ package tui
import (
"bytes"
+ "crypto/sha256"
"encoding/base64"
"fmt"
"image"
@@ -147,7 +148,7 @@ func base64Path(path string) string {
return base64.StdEncoding.EncodeToString([]byte(path))
}
-func stageTempImage(src string) string {
+func stageTempImage(dir, src string) string {
// f=100 means png and kitty has no jpeg format at all, so anything that isn't
// png gets re-encoded. embedded flac art is usually jpeg, so that's the normal
// path, not the edge case. a jpeg handed over as-is comes back EBADPNG.
@@ -176,16 +177,41 @@ func stageTempImage(src string) string {
payload = buf.Bytes()
}
+ // One name per source image, since the directory is shared. The terminal
+ // reads the file by path and never deletes it for the file medium.
+ sum := sha256.Sum256([]byte(src))
+ dst := filepath.Join(dir, fmt.Sprintf("cover-%x.png", sum[:8]))
+ if err := os.WriteFile(dst, payload, 0o600); err != nil {
+ return ""
+ }
+ return dst
+}
+
+// stageDir returns the directory staged images live in, creating it on first
+// use. One directory for the whole session, rather than one per image: nothing
+// deletes the file afterwards, so a directory per image leaked every cover that
+// was ever previewed until /tmp was cleared.
+func (p *ImagePreviewer) stageDir() string {
+ if p.dir != "" {
+ return p.dir
+ }
dir, err := os.MkdirTemp("", "intake-tty-graphics-protocol-")
if err != nil {
return ""
}
- dst := filepath.Join(dir, "cover.png")
- if err := os.WriteFile(dst, payload, 0o600); err != nil {
- os.RemoveAll(dir)
- return ""
+ p.dir = dir
+ return dir
+}
+
+// Close removes the staged images. Safe to call more than once.
+func (p *ImagePreviewer) Close() {
+ p.mu.Lock()
+ defer p.mu.Unlock()
+ if p.dir != "" {
+ os.RemoveAll(p.dir)
+ p.dir = ""
}
- return dst
+ p.staged = map[string]string{}
}
func imagePixels(path string) (int, int) {
diff --git a/internal/tui/preview.go b/internal/tui/preview.go
@@ -22,6 +22,8 @@ type ImagePreviewer struct {
cache map[string][]string
pending *kittyPlacement
staged map[string]string
+ // dir is the single directory staged images are written to.
+ dir string
}
func NewImagePreviewer() *ImagePreviewer {
@@ -96,7 +98,7 @@ func (p *ImagePreviewer) Render(path string, maxW, maxH int) []string {
if staged, ok := p.staged[path]; ok {
place.tempPath = staged
} else {
- place.tempPath = stageTempImage(path)
+ place.tempPath = stageTempImage(p.stageDir(), path)
p.staged[path] = place.tempPath
}
p.mu.Unlock()
diff --git a/internal/tui/preview_test.go b/internal/tui/preview_test.go
@@ -147,12 +147,28 @@ func makeBigImage(t *testing.T, path string) {
}
}
+// stagingDir returns the directory the previewer actually stages into, so the
+// tests exercise the real path rather than one they made up.
+func stagingDir(t *testing.T) string {
+ t.Helper()
+ p := &ImagePreviewer{}
+ dir := p.stageDir()
+ if dir == "" {
+ t.Fatal("could not create a staging directory")
+ }
+ if !strings.Contains(dir, "tty-graphics-protocol") {
+ t.Fatalf("staging directory %q lacks the marker the protocol requires", dir)
+ }
+ t.Cleanup(p.Close)
+ return dir
+}
+
func TestStagingConvertsJPEGToPNG(t *testing.T) {
dir := t.TempDir()
src := filepath.Join(dir, "cover.jpg")
writeJPEG(t, src, 64)
- staged := stageTempImage(src)
+ staged := stageTempImage(stagingDir(t), src)
if staged == "" {
t.Fatal("staging a JPEG failed; the cover would not be shown")
}
@@ -182,7 +198,7 @@ func TestStagingLeavesPNGAlone(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- staged := stageTempImage(src)
+ staged := stageTempImage(stagingDir(t), src)
if staged == "" {
t.Fatal("staging a PNG failed")
}
@@ -203,10 +219,10 @@ func TestStagingRefusesWhatItCannotConvert(t *testing.T) {
if err := os.WriteFile(junk, []byte("this is not an image"), 0o600); err != nil {
t.Fatal(err)
}
- if got := stageTempImage(junk); got != "" {
+ if got := stageTempImage(stagingDir(t), junk); got != "" {
t.Errorf("staging junk returned %q, want empty", got)
}
- if got := stageTempImage(filepath.Join(dir, "missing.png")); got != "" {
+ if got := stageTempImage(stagingDir(t), filepath.Join(dir, "missing.png")); got != "" {
t.Errorf("staging a missing file returned %q, want empty", got)
}
}
@@ -380,3 +396,33 @@ func writeTempCover(t *testing.T) string {
writeSolidPNG(t, path, 120)
return path
}
+
+// Every previewed cover used to get its own temporary directory and nothing ever
+// removed it: kitty does not delete the file for the file medium, so one
+// directory per image accumulated until /tmp was cleared by hand.
+func TestStagingReusesOneDirectoryAndCleansUp(t *testing.T) {
+ srcs := t.TempDir()
+ p := &ImagePreviewer{}
+
+ dirs := map[string]bool{}
+ for i := 0; i < 5; i++ {
+ src := filepath.Join(srcs, fmt.Sprintf("cover%d.jpg", i))
+ writeJPEG(t, src, 32)
+ staged := stageTempImage(p.stageDir(), src)
+ if staged == "" {
+ t.Fatalf("staging image %d failed", i)
+ }
+ dirs[filepath.Dir(staged)] = true
+ }
+ if len(dirs) != 1 {
+ t.Errorf("five images staged into %d directories, want 1", len(dirs))
+ }
+
+ dir := p.stageDir()
+ p.Close()
+ if _, err := os.Stat(dir); !os.IsNotExist(err) {
+ t.Errorf("Close left %s in place", dir)
+ }
+ // Closing twice must not panic; the program can exit by more than one path.
+ p.Close()
+}
diff --git a/internal/tui/runner.go b/internal/tui/runner.go
@@ -128,6 +128,10 @@ func Run(cfg config.Config, out io.Writer, in io.Reader) error {
opts = append(opts, tea.WithOutput(out))
}
opts = append(opts, programColourOptions()...)
+ // Staged cover images outlive the frame that placed them, so they are
+ // cleaned up here rather than by whatever drew them. Deferred because the
+ // program exits on ctrl+c and on error just as much as on quit.
+ defer m.prev.Close()
if _, err := tea.NewProgram(m, opts...).Run(); err != nil {
return err
}