Improve error handling and the output file lifecycle

CLAUDE.md

@@ -39,13 +39,13 @@ Everything happens in `main()` in `main.go`, in a straight-line pipeline:
    - `go.abhg.dev/goldmark/mermaid` with `NoScript: true` (diagram rendering is handled ourselves)
    - `html.WithUnsafe()` (raw HTML passthrough is required for the image-tag handling above)
 4. **Title extraction** (`getTitleFromAST`/`extractText`) — walks the parsed AST for the first H1 to use as the `<title>`.
-5. **Mermaid script embedding** (`embedMermaidScript`) — only injects the embedded `mermaid.min.js` + an init script (with light/dark theme detection via `prefers-color-scheme`) if the rendered HTML actually contains a mermaid block, keeping non-diagram output lean.
-6. **Templating & output** — `template.html` and `github-markdown.css` are embedded via `//go:embed` and combined with the rendered content via `fmt.Fprintf`. `-bare` skips the CSS. Output goes to `-o <path>` if given, otherwise a random temp filename (see below).
+5. **Mermaid script embedding** (`embedMermaidScript`) — only injects the embedded `mermaid.min.js` + an init script (with light/dark theme detection via `prefers-color-scheme`) if the parsed AST contains a mermaid block (`hasMermaidBlock`; raw HTML that mentions `class="mermaid"` does not count), keeping non-diagram output lean.
+6. **Templating & output** — `template.html` and `github-markdown.css` are embedded via `//go:embed` and combined with the rendered content via `fmt.Fprintf`. `-bare` skips the CSS. `writeOutput` writes the file and reports write and close errors. Output goes to `-o <path>` if given, otherwise a random temp filename (see below).
 7. **Launch** — opens the resulting HTML file via `github.com/pkg/browser`.
 
 ### Temp file / Snap sandboxing (`getTempDir`, `isSnap`)
 
-Output location resolution order: `MDVIEW_DIR` env var → (if running as a Snap, per `SNAP_USER_COMMON` env var) `~/mdview-temp` → OS default temp dir. This exists because Snap-sandboxed browsers (e.g. Snap Firefox) can't read from `/tmp`, only from the user's home directory — `check()` also prints a Snap-specific hint on file errors for this reason.
+Output location resolution order: `MDVIEW_DIR` env var → (if running as a Snap, per `SNAP_USER_COMMON` env var) `~/mdview-temp` → OS default temp dir. `MDVIEW_DIR` and `~/mdview-temp` are created with `MkdirAll`. Before writing a temp file, `removeStaleTempFiles` deletes generated `mdview<32 hex>.html` files older than 24 hours from that directory (the browser reads the new file after mdview exits, so mdview cannot delete it right away). This exists because Snap-sandboxed browsers (e.g. Snap Firefox) can't read from `/tmp`, only from the user's home directory — `check()` writes errors to stderr, names the failing path, and prints a Snap-specific hint for permission and not-found errors for this reason.
 
 ### Packaging
 

main.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/pkg/browser"
 	"github.com/yuin/goldmark"
@@ -123,32 +124,27 @@ func main() {
 
 	// Render to HTML
 	var buf bytes.Buffer
-	if err := md.Renderer().Render(&buf, dat, doc); err != nil {
-		log.Fatal(err)
-	}
+	check(md.Renderer().Render(&buf, dat, doc))
 	htmlContent := buf.String()
-	
+
 	// Add embedded Mermaid.js and initialization script when diagrams are present
-	htmlContent = embedMermaidScript(htmlContent)
+	if hasMermaidBlock(doc) {
+		htmlContent = embedMermaidScript(htmlContent)
+	}
 
 	outfilePath := *outfilePtr
 	if outfilePath == "" {
-		outfilePath = tempFileName("mdview", ".html")
+		tempDir := getTempDir()
+		removeStaleTempFiles(tempDir, staleTempFileAge)
+		outfilePath = tempFileName(tempDir, "mdview", ".html")
 	}
 
-	f, err := os.Create(outfilePath)
-	check(err)
-	defer f.Close()
-	var actualStyle string
+	actualStyle := style
 	if *barePtr {
 		actualStyle = ""
-	} else {
-		actualStyle = style
 	}
 
-	_, err = fmt.Fprintf(f, template, actualStyle, stdhtml.EscapeString(title), htmlContent)
-	check(err)
-	f.Sync()
+	check(writeOutput(outfilePath, actualStyle, stdhtml.EscapeString(title), htmlContent))
 	browser.Stderr = nil
 	browser.Stdout = nil
 	err = browser.OpenFile(outfilePath)
@@ -189,54 +185,102 @@ func stdinIsTerminal() bool {
 	return err == nil && info.Mode()&os.ModeCharDevice != 0
 }
 
-func tempFileName(prefix, suffix string) string {
+// writeOutput writes the HTML document to path and reports any write or close error
+func writeOutput(path string, style string, title string, content string) error {
+	f, err := os.Create(path)
+	if err != nil {
+		return err
+	}
+	if _, err := fmt.Fprintf(f, template, style, title, content); err != nil {
+		f.Close()
+		return err
+	}
+	if err := f.Sync(); err != nil {
+		f.Close()
+		return err
+	}
+	return f.Close()
+}
+
+func tempFileName(dir, prefix, suffix string) string {
 	randBytes := make([]byte, 16)
 	rand.Read(randBytes)
-	return filepath.Join(getTempDir(), prefix+hex.EncodeToString(randBytes)+suffix)
+	return filepath.Join(dir, prefix+hex.EncodeToString(randBytes)+suffix)
+}
+
+// staleTempFileAge is how old a generated temp file must be before mdview removes it
+const staleTempFileAge = 24 * time.Hour
+
+// tempFileRegex matches the names produced by tempFileName, so cleanup never touches other files
+var tempFileRegex = regexp.MustCompile(`^mdview[0-9a-f]{32}\.html$`)
+
+// removeStaleTempFiles deletes generated temp files in dir that are older than maxAge.
+// The browser reads the current file after mdview exits, so only older files are removed.
+func removeStaleTempFiles(dir string, maxAge time.Duration) {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return
+	}
+	for _, entry := range entries {
+		if !entry.Type().IsRegular() || !tempFileRegex.MatchString(entry.Name()) {
+			continue
+		}
+		if info, err := entry.Info(); err == nil && time.Since(info.ModTime()) > maxAge {
+			os.Remove(filepath.Join(dir, entry.Name()))
+		}
+	}
 }
 
 func getTempDir() string {
 	if os.Getenv("MDVIEW_DIR") != "" {
 		var tempDir = os.Getenv(("MDVIEW_DIR"))
-		if _, err := os.Stat(tempDir); os.IsNotExist(err) {
-			err = os.Mkdir(tempDir, 0700)
-			check(err)
-		}
-
+		check(os.MkdirAll(tempDir, 0700))
 		return tempDir
 	}
 
 	if isSnap() {
 		var tmpdir = os.Getenv("HOME") + "/mdview-temp"
-		if _, err := os.Stat(tmpdir); os.IsNotExist(err) {
-			err = os.Mkdir(tmpdir, 0700)
-			check(err)
-		}
+		check(os.MkdirAll(tmpdir, 0700))
 		return tmpdir
 	}
 
 	return os.TempDir()
 }
 
+// check prints a failure to standard error and exits with status 1
 func check(e error) {
-	if e != nil {
-		if errors.Is(e, fs.ErrPermission) {
-			fmt.Println("There was a permission error accessing the file.")
-		}
+	if e == nil {
+		return
+	}
 
-		if errors.Is(e, fs.ErrNotExist) {
-			fmt.Println("mdview was unable to find the file.")
-		}
+	permission := errors.Is(e, fs.ErrPermission)
+	notExist := errors.Is(e, fs.ErrNotExist)
 
-		if isSnap() {
-			fmt.Println("Since mdview was installed as a Snap, it can only access files in your HOME directory.")
-			fmt.Println("If you need to use it with files outside of your HOME directory, choose a different installation method.")
-			fmt.Println("https://github.com/mapitman/mdview?tab=readme-ov-file#installation")
-			fmt.Println()
-		}
+	// Name the path, because the failing file can be the input, the output or the output directory
+	target := "the file"
+	var pathErr *fs.PathError
+	if errors.As(e, &pathErr) {
+		target = pathErr.Path
+	}
 
-		log.Fatal(e)
+	if permission {
+		fmt.Fprintf(os.Stderr, "There was a permission error accessing %s.\n", target)
 	}
+
+	if notExist {
+		fmt.Fprintf(os.Stderr, "mdview was unable to find %s.\n", target)
+	}
+
+	// The Snap sandbox only blocks file access, so show its hint for those errors only
+	if isSnap() && (permission || notExist) {
+		fmt.Fprintln(os.Stderr, "Since mdview was installed as a Snap, it can only access files in your HOME directory.")
+		fmt.Fprintln(os.Stderr, "If you need to use it with files outside of your HOME directory, choose a different installation method.")
+		fmt.Fprintln(os.Stderr, "https://github.com/mapitman/mdview?tab=readme-ov-file#installation")
+		fmt.Fprintln(os.Stderr)
+	}
+
+	fmt.Fprintln(os.Stderr, "mdview:", e)
+	os.Exit(1)
 }
 
 func getTitleFromAST(doc ast.Node, source []byte) string {
@@ -533,15 +577,23 @@ func getMimeType(path string) string {
 	return ""
 }
 
+// hasMermaidBlock reports whether the document contains a mermaid code block.
+// Checking the AST ignores raw HTML that merely mentions class="mermaid".
+func hasMermaidBlock(doc ast.Node) bool {
+	found := false
+	_ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
+		if entering && n.Kind() == mermaid.Kind {
+			found = true
+			return ast.WalkStop, nil
+		}
+		return ast.WalkContinue, nil
+	})
+	return found
+}
+
 // embedMermaidScript adds the embedded Mermaid.js library to the HTML content
 // Since we set NoScript: true on the Mermaid extender, we need to manually add the script
 func embedMermaidScript(htmlContent string) string {
-	// Check if there are any mermaid diagrams in the content
-	// The Goldmark mermaid extension uses class="mermaid" for diagram blocks
-	if !strings.Contains(htmlContent, `class="mermaid"`) {
-		return htmlContent // No mermaid diagrams, don't add the script
-	}
-
 	// Escape any </script> tags (with closing >) inside the mermaid.js code 
 	// to prevent premature script closure. The standard way is to escape the forward slash.
 	escapedMermaidJS := strings.ReplaceAll(mermaidJS, "</script>", "<\\/script>")

main_test.go

@@ -10,6 +10,7 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/yuin/goldmark"
 	"github.com/yuin/goldmark/text"
@@ -274,3 +275,116 @@ func TestParseFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestHasMermaidBlock(t *testing.T) {
+	tests := []struct {
+		name     string
+		markdown string
+		want     bool
+	}{
+		{"mermaid fence", "```mermaid\ngraph TD; A-->B\n```\n", true},
+		{"other fence", "```go\nfmt.Println()\n```\n", false},
+		{"no code", "# Title\n\ntext\n", false},
+		{"raw html mentioning mermaid", "<pre class=\"mermaid\">graph TD</pre>\n", false},
+		{"code span mentioning mermaid", "Use `class=\"mermaid\"` in HTML.\n", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			source := []byte(tt.markdown)
+			doc := newMarkdown(".").Parser().Parse(text.NewReader(source))
+			if got := hasMermaidBlock(doc); got != tt.want {
+				t.Fatalf("hasMermaidBlock = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestEmbedMermaidScript(t *testing.T) {
+	out := embedMermaidScript("<p>x</p>")
+	if !strings.HasPrefix(out, "<p>x</p><script>") || !strings.Contains(out, "mermaid.initialize") {
+		t.Fatalf("mermaid script was not appended: %.80s", out)
+	}
+	if strings.Count(out, "</script>") != 2 {
+		t.Fatalf("expected exactly two closing script tags, got %d", strings.Count(out, "</script>"))
+	}
+}
+
+func TestGetTempDir(t *testing.T) {
+	t.Run("MDVIEW_DIR is created with parents", func(t *testing.T) {
+		dir := filepath.Join(t.TempDir(), "a", "b", "c")
+		t.Setenv("MDVIEW_DIR", dir)
+		if got := getTempDir(); got != dir {
+			t.Fatalf("getTempDir = %q, want %q", got, dir)
+		}
+		if info, err := os.Stat(dir); err != nil || !info.IsDir() {
+			t.Fatalf("directory was not created: %v", err)
+		}
+	})
+
+	t.Run("snap uses mdview-temp in HOME", func(t *testing.T) {
+		home := t.TempDir()
+		t.Setenv("MDVIEW_DIR", "")
+		t.Setenv("SNAP_USER_COMMON", "/snap/common")
+		t.Setenv("HOME", home)
+		want := filepath.Join(home, "mdview-temp")
+		if got := getTempDir(); got != want {
+			t.Fatalf("getTempDir = %q, want %q", got, want)
+		}
+	})
+
+	t.Run("default is the OS temp dir", func(t *testing.T) {
+		t.Setenv("MDVIEW_DIR", "")
+		t.Setenv("SNAP_USER_COMMON", "")
+		if got := getTempDir(); got != os.TempDir() {
+			t.Fatalf("getTempDir = %q, want %q", got, os.TempDir())
+		}
+	})
+}
+
+func TestWriteOutput(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "out.html")
+	if err := writeOutput(path, "body{}", "T", "<p>hi</p>"); err != nil {
+		t.Fatal(err)
+	}
+	got, err := os.ReadFile(path)
+	if err != nil || !strings.Contains(string(got), "<title>T</title>") || !strings.Contains(string(got), "<p>hi</p>") {
+		t.Fatalf("unexpected output (err %v): %s", err, got)
+	}
+	if err := writeOutput(filepath.Join(dir, "missing", "out.html"), "", "T", ""); err == nil {
+		t.Fatal("expected an error for a missing directory")
+	}
+}
+
+func TestRemoveStaleTempFiles(t *testing.T) {
+	dir := t.TempDir()
+	name := func(c string) string { return "mdview" + strings.Repeat(c, 32) + ".html" }
+	old := time.Now().Add(-48 * time.Hour)
+
+	files := map[string]time.Time{
+		name("a"):         old,        // stale generated file: removed
+		name("b"):         time.Now(), // fresh generated file: kept
+		"notes.html":      old,        // other file: kept
+		"mdview-old.html": old,        // wrong name pattern: kept
+		name("c") + "x":   old,        // wrong suffix: kept
+	}
+	for file, mtime := range files {
+		path := filepath.Join(dir, file)
+		if err := os.WriteFile(path, []byte("x"), 0644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Chtimes(path, mtime, mtime); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	removeStaleTempFiles(dir, staleTempFileAge)
+
+	for file := range files {
+		_, err := os.Stat(filepath.Join(dir, file))
+		wantRemoved := file == name("a")
+		if wantRemoved != os.IsNotExist(err) {
+			t.Errorf("%s: removed = %v, want %v", file, os.IsNotExist(err), wantRemoved)
+		}
+	}
+}