From ea09b7c7112d91c45b147af50067db14f1397931 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 14:31:37 +0000 Subject: [PATCH 1/7] refactor: use Go stdlib functions for string and slice operations - Replace manual rune loop with strings.Map in sanitiseForFilename - Use os.ReadFile instead of os.Open + bytes.Buffer.ReadFrom in artwork.go - Use slices.Insert for inserting multiple elements in metadata.go - Modernise benchmark loops from b.N to b.Loop() Signed-off-by: Martin Wimpress --- cmd/jivedrop/main.go | 9 ++++----- cmd/jivedrop/main_test.go | 4 ++-- internal/encoder/metadata.go | 12 +++++------- internal/id3/artwork.go | 12 ++---------- 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index 1fe6c03..0d103dc 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -113,13 +113,12 @@ func sanitiseForFilename(s string) string { s = strings.ToLower(s) // Remove any characters that aren't alphanumeric, hyphens, or underscores // Keep dots for file extensions - result := strings.Builder{} - for _, r := range s { + return strings.Map(func(r rune) rune { if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' || r == '.' { - result.WriteRune(r) + return r } - } - return result.String() + return -1 + }, s) } // generateFilename creates the output filename based on mode and metadata diff --git a/cmd/jivedrop/main_test.go b/cmd/jivedrop/main_test.go index 9408130..4b91488 100644 --- a/cmd/jivedrop/main_test.go +++ b/cmd/jivedrop/main_test.go @@ -744,7 +744,7 @@ func BenchmarkSanitiseForFilename(b *testing.B) { } b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { for _, s := range testStrings { sanitiseForFilename(s) } @@ -756,7 +756,7 @@ func BenchmarkGenerateFilename(b *testing.B) { CLI.Artist = "Linux Matters" b.ResetTimer() - for i := 0; i < b.N; i++ { + for b.Loop() { generateFilename(HugoMode, "67", "") generateFilename(StandaloneMode, "42", "My Podcast") generateFilename(StandaloneMode, "1", "") diff --git a/internal/encoder/metadata.go b/internal/encoder/metadata.go index e504290..4120deb 100644 --- a/internal/encoder/metadata.go +++ b/internal/encoder/metadata.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "time" @@ -198,18 +199,15 @@ func UpdateFrontmatter(markdownPath, duration string, bytes int64) error { // If fields don't exist, add them before the closing delimiter if !updated || !bytesUpdated { - newLines := make([]string, 0, len(lines)+2) - newLines = append(newLines, lines[:end]...) - + var insertLines []string if !updated { - newLines = append(newLines, fmt.Sprintf("podcast_duration: %s", duration)) + insertLines = append(insertLines, fmt.Sprintf("podcast_duration: %s", duration)) } if !bytesUpdated { - newLines = append(newLines, fmt.Sprintf("podcast_bytes: %d", bytes)) + insertLines = append(insertLines, fmt.Sprintf("podcast_bytes: %d", bytes)) } - newLines = append(newLines, lines[end:]...) - lines = newLines + lines = slices.Insert(lines, end, insertLines...) } // Write back to file diff --git a/internal/id3/artwork.go b/internal/id3/artwork.go index 533eb5e..d4ff226 100644 --- a/internal/id3/artwork.go +++ b/internal/id3/artwork.go @@ -63,20 +63,12 @@ func ScaleCoverArt(inputPath string) ([]byte, error) { if !needsScaling && format == "png" { cli.PrintCover(fmt.Sprintf("%dx%d %s (no scaling needed)", width, height, format)) - // Re-open and read entire original PNG file - file.Close() - file, err = os.Open(inputPath) + data, err := os.ReadFile(inputPath) if err != nil { - return nil, fmt.Errorf("failed to re-open cover art: %w", err) - } - defer file.Close() - - var buf bytes.Buffer - if _, err := buf.ReadFrom(file); err != nil { return nil, fmt.Errorf("failed to read cover art: %w", err) } - return buf.Bytes(), nil + return data, nil } // Scale if needed From 795f791fbbac3717aec799f4a5fa8173b83a1781 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 16:48:58 +0000 Subject: [PATCH 2/7] refactor: eliminate dead code and redundant style declarations - Remove unused `ctx` variable assignment from main.go - Replace duplicate lipgloss style declarations in styles.go with references to shared cli package styles - Consolidate colour variables, removing unused ones (successColor, errorColor, highlightColor, textColor, borderColor) - Move successStyle, errorStyle, highlightStyle, keyStyle, valueStyle, and boxStyle to shared cli package equivalents Signed-off-by: Martin Wimpress --- cmd/jivedrop/main.go | 2 -- internal/ui/styles.go | 52 +++++++++++-------------------------------- 2 files changed, 13 insertions(+), 41 deletions(-) diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index 0d103dc..8001a03 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -237,8 +237,6 @@ func run() int { } } - _ = ctx // Kong context available for future use - // Validate audio file exists if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { cli.PrintError(fmt.Sprintf("Audio file not found: %s", CLI.AudioFile)) diff --git a/internal/ui/styles.go b/internal/ui/styles.go index 526de3c..54e3c8d 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -7,14 +7,9 @@ import ( // Import shared colour palette from cli package var ( - primaryColor = cli.PrimaryColor - accentColor = cli.AccentColor - successColor = cli.SuccessColor - mutedColor = cli.MutedColor - highlightColor = cli.HighlightColor - textColor = cli.TextColor - errorColor = cli.ErrorColor - borderColor = cli.BorderColor + primaryColor = cli.PrimaryColor + accentColor = cli.AccentColor + mutedColor = cli.MutedColor // Disco ball gradient colours gradientIndigo = cli.GradientIndigo @@ -32,38 +27,17 @@ var accentStyle = lipgloss.NewStyle(). Bold(true). Foreground(accentColor) -// Success message style -var successStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(successColor) - -// Error message style -var errorStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(errorColor) - -// Highlight style for important values -var highlightStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(highlightColor) - -// Key-value pair styles -var keyStyle = lipgloss.NewStyle(). - Foreground(mutedColor) - -var valueStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(textColor) +// Shared styles from cli package +var ( + successStyle = cli.SuccessStyle + errorStyle = cli.ErrorStyle + highlightStyle = cli.HighlightStyle + keyStyle = cli.KeyStyle + valueStyle = cli.ValueStyle + boxStyle = cli.BoxStyle +) -// Muted text style +// Muted text style (no cli equivalent) var mutedStyle = lipgloss.NewStyle(). Foreground(mutedColor). Italic(true) - -// Box style for framed content -var boxStyle = lipgloss.NewStyle(). - Border(lipgloss.RoundedBorder()). - BorderForeground(borderColor). - Padding(1, 2). - MarginTop(1). - MarginBottom(1) From f83ea14eca72524044692a6e7c14de1e6047fbed Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 17:01:22 +0000 Subject: [PATCH 3/7] refactor(id3): invert dependencies for cover art logging - Change ScaleCoverArt to accept logFn callback instead of importing cli - Update callers to pass cli.PrintCover or test-appropriate callbacks - Remove dead addCoverArt function and CoverArtPath field from TagInfo Signed-off-by: Martin Wimpress --- cmd/jivedrop/main.go | 2 +- internal/id3/artwork.go | 17 ++++++++++------- internal/id3/artwork_test.go | 22 +++++++++++----------- internal/id3/writer.go | 20 +------------------- internal/id3/writer_test.go | 36 +++++++++++++++++++++++------------- 5 files changed, 46 insertions(+), 51 deletions(-) diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index 8001a03..f329cff 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -391,7 +391,7 @@ func run() int { return } - artwork, err := id3.ScaleCoverArt(coverArtPath) + artwork, err := id3.ScaleCoverArt(coverArtPath, cli.PrintCover) coverArtChan <- coverArtResult{data: artwork, err: err} }() diff --git a/internal/id3/artwork.go b/internal/id3/artwork.go index d4ff226..61d89ef 100644 --- a/internal/id3/artwork.go +++ b/internal/id3/artwork.go @@ -7,7 +7,6 @@ import ( "image/png" "os" - "github.com/linuxmatters/jivedrop/internal/cli" "golang.org/x/image/draw" ) @@ -18,7 +17,7 @@ import ( // // Performance optimization: Returns original PNG bytes directly when no scaling // is required. Only re-encodes to PNG for scaled images or non-PNG inputs. -func ScaleCoverArt(inputPath string) ([]byte, error) { +func ScaleCoverArt(inputPath string, logFn func(string)) ([]byte, error) { // Open and decode the image file, err := os.Open(inputPath) if err != nil { @@ -61,7 +60,9 @@ func ScaleCoverArt(inputPath string) ([]byte, error) { // If no scaling needed and format is PNG, return original file bytes if !needsScaling && format == "png" { - cli.PrintCover(fmt.Sprintf("%dx%d %s (no scaling needed)", width, height, format)) + if logFn != nil { + logFn(fmt.Sprintf("%dx%d %s (no scaling needed)", width, height, format)) + } data, err := os.ReadFile(inputPath) if err != nil { @@ -98,10 +99,12 @@ func ScaleCoverArt(inputPath string) ([]byte, error) { } // Log scaling decision - if needsScaling { - cli.PrintCover(fmt.Sprintf("%dx%d %s scaled to %dx%d PNG", width, height, format, targetSize, targetSize)) - } else { - cli.PrintCover(fmt.Sprintf("%dx%d %s re-encoded to PNG", width, height, format)) + if logFn != nil { + if needsScaling { + logFn(fmt.Sprintf("%dx%d %s scaled to %dx%d PNG", width, height, format, targetSize, targetSize)) + } else { + logFn(fmt.Sprintf("%dx%d %s re-encoded to PNG", width, height, format)) + } } return buf.Bytes(), nil diff --git a/internal/id3/artwork_test.go b/internal/id3/artwork_test.go index d8e104b..2bc6aa7 100644 --- a/internal/id3/artwork_test.go +++ b/internal/id3/artwork_test.go @@ -74,7 +74,7 @@ func TestScaleCoverArt_ValidSquareImage(t *testing.T) { } // Scale the image - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -156,7 +156,7 @@ func TestScaleCoverArt_NonSquareImage(t *testing.T) { } // Attempt to scale - _, err := ScaleCoverArt(testImagePath) + _, err := ScaleCoverArt(testImagePath, func(string) {}) if !tt.wantErr && err != nil { t.Errorf("Unexpected error: %v", err) @@ -174,7 +174,7 @@ func TestScaleCoverArt_NonSquareImage(t *testing.T) { // TestScaleCoverArt_NonExistentFile tests error handling for missing files func TestScaleCoverArt_NonExistentFile(t *testing.T) { - _, err := ScaleCoverArt("/nonexistent/path/to/image.png") + _, err := ScaleCoverArt("/nonexistent/path/to/image.png", func(string) {}) if err == nil { t.Error("Expected error for non-existent file, got nil") @@ -195,7 +195,7 @@ func TestScaleCoverArt_CorruptFile(t *testing.T) { t.Fatalf("Failed to create corrupt file: %v", err) } - _, err := ScaleCoverArt(corruptPath) + _, err := ScaleCoverArt(corruptPath, func(string) {}) if err == nil { t.Error("Expected error for corrupt file, got nil") @@ -215,7 +215,7 @@ func TestScaleCoverArt_TextFile(t *testing.T) { t.Fatalf("Failed to create text file: %v", err) } - _, err := ScaleCoverArt(textPath) + _, err := ScaleCoverArt(textPath, func(string) {}) if err == nil { t.Error("Expected error for text file, got nil") @@ -236,7 +236,7 @@ func TestScaleCoverArt_RealImageFile(t *testing.T) { } // Scale the real image - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -280,7 +280,7 @@ func TestScaleCoverArt_OutputIsPNG(t *testing.T) { t.Fatalf("Failed to create test PNG: %v", err) } - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -317,7 +317,7 @@ func TestScaleCoverArt_SkipPNGReencoding(t *testing.T) { originalSize := originalInfo.Size() // Process with ScaleCoverArt - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -392,7 +392,7 @@ func TestScaleCoverArt_EdgeCases(t *testing.T) { t.Fatalf("Failed to create test PNG: %v", err) } - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -422,7 +422,7 @@ func TestScaleCoverArt_OutputDataSize(t *testing.T) { t.Fatalf("Failed to create test PNG: %v", err) } - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -461,7 +461,7 @@ func TestScaleCoverArt_MultipleScalings(t *testing.T) { t.Fatalf("Failed to create test PNG: %v", err) } - scaledData, err := ScaleCoverArt(testImagePath) + scaledData, err := ScaleCoverArt(testImagePath, func(string) {}) if err != nil { t.Fatalf("ScaleCoverArt failed for size %d: %v", tt.inputSize, err) } diff --git a/internal/id3/writer.go b/internal/id3/writer.go index 78ea62f..7ae140d 100644 --- a/internal/id3/writer.go +++ b/internal/id3/writer.go @@ -14,8 +14,7 @@ type TagInfo struct { Album string // Optional: defaults to empty if not provided Date string // Optional: Format: "YYYY-MM" Comment string // Optional: defaults to empty if not provided - CoverArtPath string // Optional - CoverArtData []byte // Optional: pre-processed cover art bytes (CoverArtPath ignored if provided) + CoverArtData []byte // Optional: pre-processed cover art bytes Description string // Optional: cover art description (defaults to "{Artist} Logo" if not provided) } @@ -65,15 +64,10 @@ func WriteTags(mp3Path string, info TagInfo) error { } // APIC: Cover art - // Use pre-processed CoverArtData if available, otherwise process from CoverArtPath if len(info.CoverArtData) > 0 { if err := addCoverArtData(tag, info.CoverArtData, info.Artist, info.Description); err != nil { return fmt.Errorf("failed to add cover art: %w", err) } - } else if info.CoverArtPath != "" { - if err := addCoverArt(tag, info.CoverArtPath, info.Artist, info.Description); err != nil { - return fmt.Errorf("failed to add cover art: %w", err) - } } // Save the tag @@ -84,18 +78,6 @@ func WriteTags(mp3Path string, info TagInfo) error { return nil } -// addCoverArt adds cover artwork as an APIC frame -// If description is empty, defaults to "{artist} Logo" -func addCoverArt(tag *id3v2.Tag, coverPath, artist, description string) error { - // Scale the cover art according to Apple Podcasts specifications - artwork, err := ScaleCoverArt(coverPath) - if err != nil { - return fmt.Errorf("failed to scale cover art: %w", err) - } - - return addCoverArtData(tag, artwork, artist, description) -} - // addCoverArtData adds pre-processed cover artwork as an APIC frame // If description is empty, defaults to "{artist} Logo" func addCoverArtData(tag *id3v2.Tag, artwork []byte, artist, description string) error { diff --git a/internal/id3/writer_test.go b/internal/id3/writer_test.go index 2be1ef4..34cac2c 100644 --- a/internal/id3/writer_test.go +++ b/internal/id3/writer_test.go @@ -32,7 +32,6 @@ func TestWriteTags(t *testing.T) { Album: "Linux Matters", Date: "2025-11", Comment: "https://linuxmatters.sh/", - CoverArtPath: "", // Skip cover art for basic test } // Write tags @@ -149,6 +148,12 @@ func TestWriteTags_WithCoverArt(t *testing.T) { t.Skipf("Test cover art not found at %s", coverArtPath) } + // Pre-process cover art + coverArtData, err := ScaleCoverArt(coverArtPath, nil) + if err != nil { + t.Fatalf("ScaleCoverArt failed: %v", err) + } + // Create tag info with cover art info := TagInfo{ EpisodeNumber: "67", @@ -157,7 +162,7 @@ func TestWriteTags_WithCoverArt(t *testing.T) { Album: "Linux Matters", Date: "2025-11", Comment: "https://linuxmatters.sh/", - CoverArtPath: coverArtPath, + CoverArtData: coverArtData, } // Write tags with cover art @@ -226,15 +231,8 @@ func TestWriteTags_WithCoverArt_InvalidPath(t *testing.T) { t.Fatalf("Failed to create test MP3: %v", err) } - // Create tag info with non-existent cover art path - info := TagInfo{ - EpisodeNumber: "67", - Title: "Test Episode", - CoverArtPath: "/nonexistent/path/to/cover.png", - } - - // WriteTags should fail with missing cover art - err = WriteTags(mp3Path, info) + // Attempt to pre-process non-existent cover art + _, err = ScaleCoverArt("/nonexistent/path/to/cover.png", nil) if err == nil { t.Error("Expected error for non-existent cover art, got nil") } @@ -261,6 +259,12 @@ func TestWriteTags_WithCoverArt_AllMetadata(t *testing.T) { t.Skipf("Test cover art not found") } + // Pre-process cover art + coverArtData, err := ScaleCoverArt(coverArtPath, nil) + if err != nil { + t.Fatalf("ScaleCoverArt failed: %v", err) + } + // Create comprehensive tag info info := TagInfo{ EpisodeNumber: "42", @@ -269,7 +273,7 @@ func TestWriteTags_WithCoverArt_AllMetadata(t *testing.T) { Album: "Season 2", Date: "2025-12", Comment: "https://adventurepodcast.example.com/episode-42", - CoverArtPath: coverArtPath, + CoverArtData: coverArtData, } // Write all tags @@ -371,11 +375,17 @@ func TestWriteTags_CoverArt_NoOtherMetadata(t *testing.T) { t.Skipf("Test cover art not found") } + // Pre-process cover art + coverArtData, err := ScaleCoverArt(coverArtPath, nil) + if err != nil { + t.Fatalf("ScaleCoverArt failed: %v", err) + } + // Minimal tag info with only required fields and cover art info := TagInfo{ EpisodeNumber: "1", Title: "Pilot", - CoverArtPath: coverArtPath, + CoverArtData: coverArtData, } // Write tags From ce6ce679593d0ad0e0465b7390d24966b93f0e89 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 17:10:49 +0000 Subject: [PATCH 4/7] refactor(main): extract workflow abstraction for mode-specific operations - Introduce Workflow interface with Validate(), CollectMetadata(), PostEncode() - Implement HugoWorkflow: parses metadata from frontmatter, handles post-encode comparison and update prompting for podcast_duration and podcast_bytes - Implement StandaloneWorkflow: builds metadata from CLI flags, displays stats - Add newWorkflow() factory to instantiate correct workflow based on mode This abstraction enables refactoring to simplify run(). Signed-off-by: Martin Wimpress --- cmd/jivedrop/hugo.go | 140 +++++++++++++++++++++++++++++++++++++ cmd/jivedrop/standalone.go | 64 +++++++++++++++++ cmd/jivedrop/workflow.go | 34 +++++++++ 3 files changed, 238 insertions(+) create mode 100644 cmd/jivedrop/hugo.go create mode 100644 cmd/jivedrop/standalone.go create mode 100644 cmd/jivedrop/workflow.go diff --git a/cmd/jivedrop/hugo.go b/cmd/jivedrop/hugo.go new file mode 100644 index 0000000..70db17b --- /dev/null +++ b/cmd/jivedrop/hugo.go @@ -0,0 +1,140 @@ +package main + +import ( + "fmt" + "os" + + "github.com/linuxmatters/jivedrop/internal/cli" + "github.com/linuxmatters/jivedrop/internal/encoder" + "github.com/linuxmatters/jivedrop/internal/id3" +) + +// HugoWorkflow implements the Workflow interface for Hugo mode. +// It reads metadata from Hugo frontmatter and supports frontmatter updates after encoding. +type HugoWorkflow struct { + // hugoMetadata is set during CollectMetadata and read during PostEncode + hugoMetadata *encoder.EpisodeMetadata +} + +// Validate checks Hugo-specific arguments and file existence. +func (h *HugoWorkflow) Validate() error { + // Validate markdown argument + if err := validateHugoMode(); err != nil { + return err + } + + // Validate audio file exists + if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { + return fmt.Errorf("audio file not found: %s", CLI.AudioFile) + } + + // Validate episode markdown file exists + if _, err := os.Stat(CLI.EpisodeMD); os.IsNotExist(err) { + return fmt.Errorf("episode file not found: %s", CLI.EpisodeMD) + } + + // Validate custom cover art exists (if specified) + if CLI.Cover != "" { + if _, err := os.Stat(CLI.Cover); os.IsNotExist(err) { + return fmt.Errorf("cover art not found: %s", CLI.Cover) + } + } + + return nil +} + +// CollectMetadata parses Hugo frontmatter, applies defaults and flag overrides, +// and resolves the cover art path. +func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) { + // Parse episode metadata from markdown + metadata, err := encoder.ParseEpisodeMetadata(CLI.EpisodeMD) + if err != nil { + return id3.TagInfo{}, "", fmt.Errorf("failed to parse episode metadata: %w", err) + } + h.hugoMetadata = metadata + + // Apply Hugo defaults + episodeNum := metadata.Episode + episodeTitle := metadata.Title + artist := HugoDefaultArtist + comment := HugoDefaultComment + date := encoder.FormatDateForID3(metadata.Date) + var album string + + // Allow flag overrides + if CLI.Artist != "" { + artist = CLI.Artist + } + if CLI.Album != "" { + album = CLI.Album + } else { + album = artist // Inherit from artist + } + if CLI.Comment != "" { + comment = CLI.Comment + } + if CLI.Title != "" { + episodeTitle = CLI.Title + } + if CLI.Num != "" { + episodeNum = CLI.Num + } + if CLI.Date != "" { + date = CLI.Date + } + + // Resolve cover art path + var coverArtPath string + if CLI.Cover != "" { + coverArtPath = CLI.Cover + } else { + coverArtPath, err = encoder.ResolveCoverArtPath(CLI.EpisodeMD, metadata.EpisodeImage) + if err != nil { + return id3.TagInfo{}, "", fmt.Errorf("failed to resolve cover art: %w", err) + } + } + + tagInfo := id3.TagInfo{ + EpisodeNumber: episodeNum, + Title: episodeTitle, + Artist: artist, + Album: album, + Date: date, + Comment: comment, + } + + return tagInfo, coverArtPath, nil +} + +// PostEncode displays podcast statistics and handles frontmatter comparison and update prompting. +func (h *HugoWorkflow) PostEncode(stats *encoder.FileStats, outputPath string) error { + // Display podcast statistics + fmt.Println("\nPodcast statistics:") + cli.PrintLabelValue("• podcast_duration:", stats.DurationString) + cli.PrintLabelValue("• podcast_bytes:", fmt.Sprintf("%d", stats.FileSizeBytes)) + + // Check if values differ from existing frontmatter + needsUpdate := false + if h.hugoMetadata.PodcastDuration != "" && h.hugoMetadata.PodcastDuration != stats.DurationString { + cli.PrintWarning(fmt.Sprintf("Duration mismatch: frontmatter has %s, calculated %s", + h.hugoMetadata.PodcastDuration, stats.DurationString)) + needsUpdate = true + } + if h.hugoMetadata.PodcastBytes > 0 && h.hugoMetadata.PodcastBytes != stats.FileSizeBytes { + cli.PrintWarning(fmt.Sprintf("File size mismatch: frontmatter has %d, calculated %d", + h.hugoMetadata.PodcastBytes, stats.FileSizeBytes)) + needsUpdate = true + } + + // Prompt user to update frontmatter if values differ or are missing + if needsUpdate { + promptAndUpdateFrontmatter(CLI.EpisodeMD, "\nUpdate frontmatter with new values? [y/N]: ", stats.DurationString, stats.FileSizeBytes) + } else if h.hugoMetadata.PodcastDuration == "" || h.hugoMetadata.PodcastBytes == 0 { + promptAndUpdateFrontmatter(CLI.EpisodeMD, "\nAdd podcast_duration and podcast_bytes to frontmatter? [y/N]: ", stats.DurationString, stats.FileSizeBytes) + } + + return nil +} + +// Ensure HugoWorkflow implements Workflow at compile time. +var _ Workflow = (*HugoWorkflow)(nil) diff --git a/cmd/jivedrop/standalone.go b/cmd/jivedrop/standalone.go new file mode 100644 index 0000000..f953921 --- /dev/null +++ b/cmd/jivedrop/standalone.go @@ -0,0 +1,64 @@ +package main + +import ( + "fmt" + "os" + + "github.com/linuxmatters/jivedrop/internal/cli" + "github.com/linuxmatters/jivedrop/internal/encoder" + "github.com/linuxmatters/jivedrop/internal/id3" +) + +// StandaloneWorkflow implements the Workflow interface for standalone mode. +// Metadata comes entirely from CLI flags. +type StandaloneWorkflow struct{} + +// Validate checks standalone-specific arguments and file existence. +func (s *StandaloneWorkflow) Validate() error { + // Validate required standalone flags + if err := validateStandaloneMode(); err != nil { + return err + } + + // Validate audio file exists + if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { + return fmt.Errorf("audio file not found: %s", CLI.AudioFile) + } + + // Validate cover art exists + if _, err := os.Stat(CLI.Cover); os.IsNotExist(err) { + return fmt.Errorf("cover art not found: %s", CLI.Cover) + } + + return nil +} + +// CollectMetadata builds TagInfo from CLI flags. +func (s *StandaloneWorkflow) CollectMetadata() (id3.TagInfo, string, error) { + album := CLI.Album + if album == "" && CLI.Artist != "" { + album = CLI.Artist // Inherit from artist + } + + tagInfo := id3.TagInfo{ + EpisodeNumber: CLI.Num, + Title: CLI.Title, + Artist: CLI.Artist, + Album: album, + Date: CLI.Date, + Comment: CLI.Comment, + } + + return tagInfo, CLI.Cover, nil +} + +// PostEncode displays podcast statistics. Standalone mode has no frontmatter to update. +func (s *StandaloneWorkflow) PostEncode(stats *encoder.FileStats, outputPath string) error { + fmt.Println("\nPodcast statistics:") + cli.PrintLabelValue("• podcast_duration:", stats.DurationString) + cli.PrintLabelValue("• podcast_bytes:", fmt.Sprintf("%d", stats.FileSizeBytes)) + return nil +} + +// Ensure StandaloneWorkflow implements Workflow at compile time. +var _ Workflow = (*StandaloneWorkflow)(nil) diff --git a/cmd/jivedrop/workflow.go b/cmd/jivedrop/workflow.go new file mode 100644 index 0000000..659f4eb --- /dev/null +++ b/cmd/jivedrop/workflow.go @@ -0,0 +1,34 @@ +package main + +import ( + "github.com/linuxmatters/jivedrop/internal/encoder" + "github.com/linuxmatters/jivedrop/internal/id3" +) + +// Workflow defines the mode-specific operations for Hugo and Standalone workflows. +// resolveOutputPath remains a shared package-level helper called from run(). +type Workflow interface { + // Validate checks mode-specific arguments and file existence. + Validate() error + + // CollectMetadata gathers ID3 tag info and cover art path for the current mode. + // The cover art path is returned separately because it feeds the concurrent + // cover art goroutine, not TagInfo directly. + CollectMetadata() (id3.TagInfo, string, error) + + // PostEncode handles post-encoding operations: stats display and, + // in Hugo mode, frontmatter comparison and update prompting. + PostEncode(stats *encoder.FileStats, outputPath string) error +} + +// newWorkflow returns the Workflow implementation for the given mode. +func newWorkflow(mode WorkflowMode) Workflow { + switch mode { + case HugoMode: + return &HugoWorkflow{} + case StandaloneMode: + return &StandaloneWorkflow{} + default: + panic("unknown workflow mode") + } +} From ad48379f848345885a027769b7bed4ef0ee2dc7b Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 17:21:38 +0000 Subject: [PATCH 5/7] refactor(main): wire workflow interface into run() - Remove ~105 lines of inline mode-specific validation, metadata collection, and post-encode logic - Replace with three workflow method calls: Validate(), CollectMetadata(), PostEncode() - Eliminate seven loose variables (episodeNum, episodeTitle, artist, album, date, comment, coverArtPath) and hugoMetadata struct - Thread tagInfo directly from CollectMetadata() to ID3 tag writing Signed-off-by: Martin Wimpress --- cmd/jivedrop/main.go | 160 ++++--------------------------------------- 1 file changed, 15 insertions(+), 145 deletions(-) diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index f329cff..d6751e0 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -221,118 +221,23 @@ func run() int { return 0 } - // Detect workflow mode and validate arguments + // Detect workflow mode, validate, and collect metadata mode := detectMode() + wf := newWorkflow(mode) - // Mode-specific validation - if mode == HugoMode { - if err := validateHugoMode(); err != nil { - cli.PrintError(err.Error()) - return 1 - } - } else { - if err := validateStandaloneMode(); err != nil { - cli.PrintError(err.Error()) - return 1 - } - } - - // Validate audio file exists - if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { - cli.PrintError(fmt.Sprintf("Audio file not found: %s", CLI.AudioFile)) - cli.PrintInfo("Make sure the audio file exists.") + if err := wf.Validate(); err != nil { + cli.PrintError(err.Error()) return 1 } - // Validate episode markdown file exists (Hugo mode) - if mode == HugoMode { - if _, err := os.Stat(CLI.EpisodeMD); os.IsNotExist(err) { - cli.PrintError(fmt.Sprintf("Episode file not found: %s", CLI.EpisodeMD)) - cli.PrintInfo("Make sure the episode markdown file exists.") - return 1 - } - } - - // Validate custom cover art exists (if specified) - if CLI.Cover != "" { - if _, err := os.Stat(CLI.Cover); os.IsNotExist(err) { - cli.PrintError(fmt.Sprintf("Cover art not found: %s", CLI.Cover)) - cli.PrintInfo("Make sure the cover art file exists.") - return 1 - } - } - - // Collect metadata based on workflow mode - var episodeNum, episodeTitle, artist, album, date, comment, coverArtPath string - var hugoMetadata *encoder.EpisodeMetadata - - if mode == HugoMode { - // Parse episode metadata from markdown - var err error - hugoMetadata, err = encoder.ParseEpisodeMetadata(CLI.EpisodeMD) - if err != nil { - cli.PrintError(fmt.Sprintf("Failed to parse episode metadata: %v", err)) - return 1 - } - - // Apply Hugo defaults - episodeNum = hugoMetadata.Episode - episodeTitle = hugoMetadata.Title - artist = HugoDefaultArtist - comment = HugoDefaultComment - date = encoder.FormatDateForID3(hugoMetadata.Date) - - // Allow flag overrides - if CLI.Artist != "" { - artist = CLI.Artist - } - if CLI.Album != "" { - album = CLI.Album - } else { - album = artist // Inherit from artist - } - if CLI.Comment != "" { - comment = CLI.Comment - } - if CLI.Title != "" { - episodeTitle = CLI.Title - } - if CLI.Num != "" { - episodeNum = CLI.Num - } - if CLI.Date != "" { - date = CLI.Date - } - - // Resolve cover art path - if CLI.Cover != "" { - coverArtPath = CLI.Cover - } else { - coverArtPath, err = encoder.ResolveCoverArtPath(CLI.EpisodeMD, hugoMetadata.EpisodeImage) - if err != nil { - cli.PrintError(fmt.Sprintf("Failed to resolve cover art: %v", err)) - cli.PrintInfo("Use --cover flag to specify a custom cover art path.") - return 1 - } - } - } else { - // Standalone mode: use CLI flags - episodeNum = CLI.Num - episodeTitle = CLI.Title - artist = CLI.Artist - album = CLI.Album - date = CLI.Date - comment = CLI.Comment - coverArtPath = CLI.Cover - - // Apply defaults for standalone mode - if album == "" && artist != "" { - album = artist // Inherit from artist - } + tagInfo, coverArtPath, err := wf.CollectMetadata() + if err != nil { + cli.PrintError(err.Error()) + return 1 } // Resolve output path - outputPath, err := resolveOutputPath(mode, episodeNum, artist) + outputPath, err := resolveOutputPath(mode, tagInfo.EpisodeNumber, tagInfo.Artist) if err != nil { cli.PrintError(fmt.Sprintf("Failed to resolve output path: %v", err)) return 1 @@ -340,7 +245,7 @@ func run() int { // Display encoding info cli.PrintSuccessLabel("Ready to encode:", fmt.Sprintf("%s -> MP3", CLI.AudioFile)) - cli.PrintLabelValue("• Episode:", fmt.Sprintf("%s - %s", episodeNum, episodeTitle)) + cli.PrintLabelValue("• Episode:", fmt.Sprintf("%s - %s", tagInfo.EpisodeNumber, tagInfo.Title)) if mode == HugoMode { cli.PrintLabelValue("• Episode markdown:", CLI.EpisodeMD) } @@ -426,16 +331,7 @@ func run() int { // Write ID3v2 tags fmt.Println("\nEmbedding ID3v2 tags...") - tagInfo := id3.TagInfo{ - EpisodeNumber: episodeNum, - Title: episodeTitle, - Artist: artist, - Album: album, - Date: date, - Comment: comment, - CoverArtData: coverResult.data, // Use pre-processed cover art from concurrent processing - Description: "", // Will default to "{artist} Logo" in addCoverArt - } + tagInfo.CoverArtData = coverResult.data if err := id3.WriteTags(outputPath, tagInfo); err != nil { cli.PrintError(fmt.Sprintf("Failed to write ID3 tags: %v", err)) @@ -453,36 +349,10 @@ func run() int { return 0 } - // Display podcast statistics (both modes) - fmt.Println("\nPodcast statistics:") - cli.PrintLabelValue("• podcast_duration:", stats.DurationString) - cli.PrintLabelValue("• podcast_bytes:", fmt.Sprintf("%d", stats.FileSizeBytes)) - - // Only handle frontmatter updates in Hugo mode - if mode == StandaloneMode { - return 0 - } - - // Hugo mode: check and update frontmatter if needed - // Check if values differ from existing frontmatter - needsUpdate := false - if hugoMetadata.PodcastDuration != "" && hugoMetadata.PodcastDuration != stats.DurationString { - cli.PrintWarning(fmt.Sprintf("Duration mismatch: frontmatter has %s, calculated %s", - hugoMetadata.PodcastDuration, stats.DurationString)) - needsUpdate = true - } - if hugoMetadata.PodcastBytes > 0 && hugoMetadata.PodcastBytes != stats.FileSizeBytes { - cli.PrintWarning(fmt.Sprintf("File size mismatch: frontmatter has %d, calculated %d", - hugoMetadata.PodcastBytes, stats.FileSizeBytes)) - needsUpdate = true - } - - // Prompt user to update frontmatter if values differ or are missing - if needsUpdate { - promptAndUpdateFrontmatter(CLI.EpisodeMD, "\nUpdate frontmatter with new values? [y/N]: ", stats.DurationString, stats.FileSizeBytes) - } else if hugoMetadata.PodcastDuration == "" || hugoMetadata.PodcastBytes == 0 { - // If frontmatter is missing these fields, offer to add them - promptAndUpdateFrontmatter(CLI.EpisodeMD, "\nAdd podcast_duration and podcast_bytes to frontmatter? [y/N]: ", stats.DurationString, stats.FileSizeBytes) + // Post-encode: display stats and handle mode-specific operations + if err := wf.PostEncode(stats, outputPath); err != nil { + cli.PrintError(err.Error()) + return 1 } return 0 From 85012828b8384aa5a44a27c4557815a80720373f Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 17:54:06 +0000 Subject: [PATCH 6/7] refactor(main): complete workflow abstraction and simplify run() Wired workflow interface fully into run(), reducing it to under 100 lines with no mode-specific branching. Extracted encoding block into encode() helper and moved validateHugoMode/validateStandaloneMode tests into workflow implementations. Fixes: - Lowercase 5 error strings (ST1005) - Remove trailing blank line (gofumpt) - Remove unused error return from addCoverArtData (unparam) Eliminates 7 loose metadata variables and 22+ linter errors. Signed-off-by: Martin Wimpress --- cmd/jivedrop/hugo.go | 9 +- cmd/jivedrop/hugo_test.go | 279 +++++++++++++++ cmd/jivedrop/main.go | 169 ++++----- cmd/jivedrop/main_test.go | 614 -------------------------------- cmd/jivedrop/standalone.go | 12 +- cmd/jivedrop/standalone_test.go | 373 +++++++++++++++++++ internal/id3/writer.go | 8 +- 7 files changed, 746 insertions(+), 718 deletions(-) create mode 100644 cmd/jivedrop/hugo_test.go create mode 100644 cmd/jivedrop/standalone_test.go diff --git a/cmd/jivedrop/hugo.go b/cmd/jivedrop/hugo.go index 70db17b..778909f 100644 --- a/cmd/jivedrop/hugo.go +++ b/cmd/jivedrop/hugo.go @@ -3,6 +3,7 @@ package main import ( "fmt" "os" + "strings" "github.com/linuxmatters/jivedrop/internal/cli" "github.com/linuxmatters/jivedrop/internal/encoder" @@ -19,8 +20,12 @@ type HugoWorkflow struct { // Validate checks Hugo-specific arguments and file existence. func (h *HugoWorkflow) Validate() error { // Validate markdown argument - if err := validateHugoMode(); err != nil { - return err + if CLI.EpisodeMD == "" { + return fmt.Errorf("hugo mode requires episode markdown file as second argument") + } + + if !strings.HasSuffix(strings.ToLower(CLI.EpisodeMD), ".md") { + return fmt.Errorf("episode markdown file must have .md extension: %s", CLI.EpisodeMD) } // Validate audio file exists diff --git a/cmd/jivedrop/hugo_test.go b/cmd/jivedrop/hugo_test.go new file mode 100644 index 0000000..23c28e0 --- /dev/null +++ b/cmd/jivedrop/hugo_test.go @@ -0,0 +1,279 @@ +package main + +import ( + "strings" + "testing" +) + +// TestHugoWorkflowValidate tests Hugo mode validation of episode markdown arguments +func TestHugoWorkflowValidate(t *testing.T) { + tests := []struct { + name string + episodeMD string + wantErr bool + errMatch string // Substring to match in error message + }{ + // Valid cases + { + name: "valid markdown file lowercase .md", + episodeMD: "episode.md", + wantErr: false, + }, + { + name: "valid markdown file uppercase .MD", + episodeMD: "episode.MD", + wantErr: false, + }, + { + name: "valid markdown file mixed case .Md", + episodeMD: "episode.Md", + wantErr: false, + }, + { + name: "valid markdown with nested path", + episodeMD: "content/episodes/67.md", + wantErr: false, + }, + { + name: "valid markdown deeply nested", + episodeMD: "posts/blog/2025/11/article.md", + wantErr: false, + }, + { + name: "valid markdown with spaces in filename", + episodeMD: "my episode file.md", + wantErr: false, + }, + { + name: "valid markdown with special characters", + episodeMD: "episode-67_final.md", + wantErr: false, + }, + { + name: "valid markdown with multiple dots", + episodeMD: "my.episode.v2.md", + wantErr: false, + }, + + // Invalid cases: missing EpisodeMD + { + name: "empty episode markdown string", + episodeMD: "", + wantErr: true, + errMatch: "requires episode markdown file", + }, + + // Invalid cases: wrong file extensions + { + name: "wrong extension .txt", + episodeMD: "episode.txt", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: "wrong extension .yaml", + episodeMD: "episode.yaml", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: "wrong extension .json", + episodeMD: "episode.json", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: "wrong extension .mp3", + episodeMD: "episode.mp3", + wantErr: true, + errMatch: "must have .md extension", + }, + + // Invalid cases: .md not at end + { + name: ".md in middle of filename", + episodeMD: "markdown_file.mp3", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: ".md with backup suffix", + episodeMD: "episode.md.backup", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: ".md with bak extension", + episodeMD: "episode.md.bak", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: ".md with old extension", + episodeMD: "episode.md.old", + wantErr: true, + errMatch: "must have .md extension", + }, + + // Edge cases + { + name: "just .md filename", + episodeMD: ".md", + wantErr: false, + }, + { + name: "no extension", + episodeMD: "episode", + wantErr: true, + errMatch: "must have .md extension", + }, + { + name: "uppercase .MD only", + episodeMD: ".MD", + wantErr: false, + }, + { + name: "path with uppercase .MD", + episodeMD: "content/episodes/POST.MD", + wantErr: false, + }, + { + name: "relative path with ./", + episodeMD: "./episode.md", + wantErr: false, + }, + { + name: "relative path with ../", + episodeMD: "../episodes/episode.md", + wantErr: false, + }, + { + name: "absolute path (unix)", + episodeMD: "/home/user/episodes/67.md", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save and restore global state + originalEpisodeMD := CLI.EpisodeMD + originalAudioFile := CLI.AudioFile + defer func() { + CLI.EpisodeMD = originalEpisodeMD + CLI.AudioFile = originalAudioFile + }() + + // Set test value + CLI.EpisodeMD = tt.episodeMD + // Set a dummy audio file so file-existence checks do not mask + // the argument validation errors we are testing for. + CLI.AudioFile = "testdata" + + // Call Validate on HugoWorkflow + wf := &HugoWorkflow{} + err := wf.Validate() + + // Check error expectations + if tt.wantErr { + if err == nil { + t.Errorf("HugoWorkflow.Validate() expected error, got nil (EpisodeMD=%q)", tt.episodeMD) + return + } + if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { + t.Errorf("HugoWorkflow.Validate() error %q does not contain %q", err.Error(), tt.errMatch) + } + return + } + + // For valid cases we only check argument validation, not file existence. + // File-not-found errors are acceptable here since the files do not exist on disk. + if err != nil && !strings.Contains(err.Error(), "not found") { + t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, tt.episodeMD) + } + }) + } +} + +// TestHugoWorkflowValidate_Integration tests HugoWorkflow.Validate with realistic scenarios +func TestHugoWorkflowValidate_Integration(t *testing.T) { + tests := []struct { + name string + episodeMD string + wantErr bool + description string + }{ + { + name: "real hugo workflow file", + episodeMD: "content/episodes/67.md", + wantErr: false, + description: "Typical Linux Matters episode markdown path", + }, + { + name: "common user mistake: .txt instead of .md", + episodeMD: "episode.txt", + wantErr: true, + description: "User accidentally passes wrong file type", + }, + { + name: "common user mistake: no extension", + episodeMD: "episode", + wantErr: true, + description: "User passes file without extension", + }, + { + name: "common user mistake: .md.bak backup file", + episodeMD: "episode.md.bak", + wantErr: true, + description: "User passes backup file instead of original", + }, + { + name: "windows path with backslashes", + episodeMD: "content\\episodes\\67.md", + wantErr: false, + description: "Cross-platform support for Windows paths", + }, + { + name: "uppercase .MD extension", + episodeMD: "EPISODE.MD", + wantErr: false, + description: "Case-insensitive extension matching", + }, + { + name: "missing argument", + episodeMD: "", + wantErr: true, + description: "User forgot to provide episode markdown file", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save and restore global state + originalEpisodeMD := CLI.EpisodeMD + originalAudioFile := CLI.AudioFile + defer func() { + CLI.EpisodeMD = originalEpisodeMD + CLI.AudioFile = originalAudioFile + }() + + // Set test value + CLI.EpisodeMD = tt.episodeMD + CLI.AudioFile = "testdata" + + // Call Validate on HugoWorkflow + wf := &HugoWorkflow{} + err := wf.Validate() + + // Check error expectations + if tt.wantErr && err == nil { + t.Errorf("HugoWorkflow.Validate() expected error but got nil\n Description: %s\n EpisodeMD=%q", + tt.description, tt.episodeMD) + } + if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") { + t.Errorf("HugoWorkflow.Validate() unexpected error: %v\n Description: %s\n EpisodeMD=%q", + err, tt.description, tt.episodeMD) + } + }) + } +} diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index d6751e0..6a482f9 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -73,38 +73,6 @@ func detectMode() WorkflowMode { return StandaloneMode } -// validateHugoMode validates Hugo workflow arguments -func validateHugoMode() error { - // In Hugo mode, episode markdown is required - if CLI.EpisodeMD == "" { - return fmt.Errorf("hugo mode requires episode markdown file as second argument") - } - - if !strings.HasSuffix(strings.ToLower(CLI.EpisodeMD), ".md") { - return fmt.Errorf("episode markdown file must have .md extension: %s", CLI.EpisodeMD) - } - - return nil -} - -// validateStandaloneMode validates standalone workflow arguments -func validateStandaloneMode() error { - // In standalone mode, --title, --num, and --cover are required - if CLI.Title == "" { - return fmt.Errorf("standalone mode requires --title flag") - } - - if CLI.Num == "" { - return fmt.Errorf("standalone mode requires --num flag (episode number)") - } - - if CLI.Cover == "" { - return fmt.Errorf("standalone mode requires --cover flag (cover art path)") - } - - return nil -} - // sanitiseForFilename replaces spaces and invalid characters for safe filenames func sanitiseForFilename(s string) string { // Replace spaces with hyphens @@ -196,53 +164,11 @@ func promptAndUpdateFrontmatter(markdownPath, promptMsg, duration string, bytes } } -func main() { - os.Exit(run()) -} - -func run() int { - ctx := kong.Parse(&CLI, - kong.Name("jivedrop"), - kong.Description("Drop the mix, ship the show—metadata, cover art, and all."), - kong.Vars{"version": version}, - kong.UsageOnError(), - kong.Help(cli.StyledHelpPrinter(kong.HelpOptions{Compact: true})), - ) - - // Handle version flag - if CLI.Version { - cli.PrintVersion(version) - return 0 - } - - // If no audio file provided, show help - if CLI.AudioFile == "" { - _ = ctx.PrintUsage(false) - return 0 - } - - // Detect workflow mode, validate, and collect metadata - mode := detectMode() - wf := newWorkflow(mode) - - if err := wf.Validate(); err != nil { - cli.PrintError(err.Error()) - return 1 - } - - tagInfo, coverArtPath, err := wf.CollectMetadata() - if err != nil { - cli.PrintError(err.Error()) - return 1 - } - - // Resolve output path - outputPath, err := resolveOutputPath(mode, tagInfo.EpisodeNumber, tagInfo.Artist) - if err != nil { - cli.PrintError(fmt.Sprintf("Failed to resolve output path: %v", err)) - return 1 - } - +// encode runs the full encoding pipeline: display info, create encoder, run +// Bubbletea UI, process cover art concurrently, write ID3 tags, and extract +// file statistics. Returns nil stats (with nil error) when stats extraction +// fails but the MP3 was written successfully. +func encode(mode WorkflowMode, tagInfo id3.TagInfo, coverArtPath, outputPath string) (*encoder.FileStats, error) { // Display encoding info cli.PrintSuccessLabel("Ready to encode:", fmt.Sprintf("%s -> MP3", CLI.AudioFile)) cli.PrintLabelValue("• Episode:", fmt.Sprintf("%s - %s", tagInfo.EpisodeNumber, tagInfo.Title)) @@ -263,15 +189,13 @@ func run() int { Stereo: CLI.Stereo, }) if err != nil { - cli.PrintError(fmt.Sprintf("Failed to create encoder: %v", err)) - return 1 + return nil, fmt.Errorf("failed to create encoder: %w", err) } defer enc.Close() // Initialize encoder if err := enc.Initialize(); err != nil { - cli.PrintError(fmt.Sprintf("Failed to initialize encoder: %v", err)) - return 1 + return nil, fmt.Errorf("failed to initialize encoder: %w", err) } // Get input info @@ -288,7 +212,6 @@ func run() int { } // Start cover art processing concurrently with encoding - // This hides cover art processing time behind the audio encoding time coverArtChan := make(chan coverArtResult, 1) go func() { if coverArtPath == "" { @@ -296,8 +219,8 @@ func run() int { return } - artwork, err := id3.ScaleCoverArt(coverArtPath, cli.PrintCover) - coverArtChan <- coverArtResult{data: artwork, err: err} + artwork, artErr := id3.ScaleCoverArt(coverArtPath, cli.PrintCover) + coverArtChan <- coverArtResult{data: artwork, err: artErr} }() // Start encoding with Bubbletea UI @@ -307,26 +230,23 @@ func run() int { p := tea.NewProgram(encodeModel) finalModel, err := p.Run() if err != nil { - cli.PrintError(fmt.Sprintf("UI error: %v", err)) - return 1 + return nil, fmt.Errorf("UI error: %w", err) } // Check for encoding errors if encModel, ok := finalModel.(*ui.EncodeModel); ok { if encModel.Error() != nil { - cli.PrintError(fmt.Sprintf("Encoding failed: %v", encModel.Error())) // Clean up partial output file os.Remove(outputPath) - return 1 + return nil, fmt.Errorf("encoding failed: %w", encModel.Error()) } } // Collect cover art result from concurrent processing coverResult := <-coverArtChan if coverResult.err != nil { - cli.PrintError(fmt.Sprintf("Failed to process cover art: %v", coverResult.err)) cli.PrintInfo(fmt.Sprintf("MP3 file created but missing cover art: %s", outputPath)) - return 1 + return nil, fmt.Errorf("failed to process cover art: %w", coverResult.err) } // Write ID3v2 tags @@ -334,9 +254,8 @@ func run() int { tagInfo.CoverArtData = coverResult.data if err := id3.WriteTags(outputPath, tagInfo); err != nil { - cli.PrintError(fmt.Sprintf("Failed to write ID3 tags: %v", err)) cli.PrintInfo(fmt.Sprintf("MP3 file created but missing metadata: %s", outputPath)) - return 1 + return nil, fmt.Errorf("failed to write ID3 tags: %w", err) } cli.PrintSuccessLabel("Complete:", outputPath) @@ -346,6 +265,68 @@ func run() int { stats, err := encoder.GetFileStats(outputPath, durationSecs) if err != nil { cli.PrintWarning(fmt.Sprintf("Could not extract file statistics: %v", err)) + return nil, nil + } + + return stats, nil +} + +func main() { + os.Exit(run()) +} + +func run() int { + ctx := kong.Parse(&CLI, + kong.Name("jivedrop"), + kong.Description("Drop the mix, ship the show—metadata, cover art, and all."), + kong.Vars{"version": version}, + kong.UsageOnError(), + kong.Help(cli.StyledHelpPrinter(kong.HelpOptions{Compact: true})), + ) + + // Handle version flag + if CLI.Version { + cli.PrintVersion(version) + return 0 + } + + // If no audio file provided, show help + if CLI.AudioFile == "" { + _ = ctx.PrintUsage(false) + return 0 + } + + // Detect workflow mode, validate, and collect metadata + mode := detectMode() + wf := newWorkflow(mode) + + if err := wf.Validate(); err != nil { + cli.PrintError(err.Error()) + return 1 + } + + tagInfo, coverArtPath, err := wf.CollectMetadata() + if err != nil { + cli.PrintError(err.Error()) + return 1 + } + + // Resolve output path + outputPath, err := resolveOutputPath(mode, tagInfo.EpisodeNumber, tagInfo.Artist) + if err != nil { + cli.PrintError(fmt.Sprintf("Failed to resolve output path: %v", err)) + return 1 + } + + // Encode audio, process cover art, write ID3 tags, and collect file statistics + stats, err := encode(mode, tagInfo, coverArtPath, outputPath) + if err != nil { + cli.PrintError(err.Error()) + return 1 + } + + // Stats may be nil when extraction failed but encoding succeeded + if stats == nil { return 0 } diff --git a/cmd/jivedrop/main_test.go b/cmd/jivedrop/main_test.go index 4b91488..cee331c 100644 --- a/cmd/jivedrop/main_test.go +++ b/cmd/jivedrop/main_test.go @@ -762,617 +762,3 @@ func BenchmarkGenerateFilename(b *testing.B) { generateFilename(StandaloneMode, "1", "") } } - -// TestValidateHugoMode tests Hugo mode validation of episode markdown arguments -func TestValidateHugoMode(t *testing.T) { - tests := []struct { - name string - episodeMD string - wantErr bool - errMatch string // Substring to match in error message - }{ - // Valid cases - { - name: "valid markdown file lowercase .md", - episodeMD: "episode.md", - wantErr: false, - }, - { - name: "valid markdown file uppercase .MD", - episodeMD: "episode.MD", - wantErr: false, - }, - { - name: "valid markdown file mixed case .Md", - episodeMD: "episode.Md", - wantErr: false, - }, - { - name: "valid markdown with nested path", - episodeMD: "content/episodes/67.md", - wantErr: false, - }, - { - name: "valid markdown deeply nested", - episodeMD: "posts/blog/2025/11/article.md", - wantErr: false, - }, - { - name: "valid markdown with spaces in filename", - episodeMD: "my episode file.md", - wantErr: false, - }, - { - name: "valid markdown with special characters", - episodeMD: "episode-67_final.md", - wantErr: false, - }, - { - name: "valid markdown with multiple dots", - episodeMD: "my.episode.v2.md", - wantErr: false, - }, - - // Invalid cases: missing EpisodeMD - { - name: "empty episode markdown string", - episodeMD: "", - wantErr: true, - errMatch: "requires episode markdown file", - }, - - // Invalid cases: wrong file extensions - { - name: "wrong extension .txt", - episodeMD: "episode.txt", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: "wrong extension .yaml", - episodeMD: "episode.yaml", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: "wrong extension .json", - episodeMD: "episode.json", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: "wrong extension .mp3", - episodeMD: "episode.mp3", - wantErr: true, - errMatch: "must have .md extension", - }, - - // Invalid cases: .md not at end - { - name: ".md in middle of filename", - episodeMD: "markdown_file.mp3", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: ".md with backup suffix", - episodeMD: "episode.md.backup", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: ".md with bak extension", - episodeMD: "episode.md.bak", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: ".md with old extension", - episodeMD: "episode.md.old", - wantErr: true, - errMatch: "must have .md extension", - }, - - // Edge cases - { - name: "just .md filename", - episodeMD: ".md", - wantErr: false, - }, - { - name: "no extension", - episodeMD: "episode", - wantErr: true, - errMatch: "must have .md extension", - }, - { - name: "uppercase .MD only", - episodeMD: ".MD", - wantErr: false, - }, - { - name: "path with uppercase .MD", - episodeMD: "content/episodes/POST.MD", - wantErr: false, - }, - { - name: "relative path with ./", - episodeMD: "./episode.md", - wantErr: false, - }, - { - name: "relative path with ../", - episodeMD: "../episodes/episode.md", - wantErr: false, - }, - { - name: "absolute path (unix)", - episodeMD: "/home/user/episodes/67.md", - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save and restore global state - originalEpisodeMD := CLI.EpisodeMD - defer func() { CLI.EpisodeMD = originalEpisodeMD }() - - // Set test value - CLI.EpisodeMD = tt.episodeMD - - // Call validateHugoMode - err := validateHugoMode() - - // Check error expectations - if tt.wantErr { - if err == nil { - t.Errorf("validateHugoMode() expected error, got nil (EpisodeMD=%q)", tt.episodeMD) - return - } - if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { - t.Errorf("validateHugoMode() error %q does not contain %q", err.Error(), tt.errMatch) - } - return - } - - // Success case - if err != nil { - t.Errorf("validateHugoMode() unexpected error: %v (EpisodeMD=%q)", err, tt.episodeMD) - } - }) - } -} - -// TestValidateHugoMode_Integration tests validateHugoMode with realistic scenarios -func TestValidateHugoMode_Integration(t *testing.T) { - tests := []struct { - name string - episodeMD string - wantErr bool - description string - }{ - { - name: "real hugo workflow file", - episodeMD: "content/episodes/67.md", - wantErr: false, - description: "Typical Linux Matters episode markdown path", - }, - { - name: "common user mistake: .txt instead of .md", - episodeMD: "episode.txt", - wantErr: true, - description: "User accidentally passes wrong file type", - }, - { - name: "common user mistake: no extension", - episodeMD: "episode", - wantErr: true, - description: "User passes file without extension", - }, - { - name: "common user mistake: .md.bak backup file", - episodeMD: "episode.md.bak", - wantErr: true, - description: "User passes backup file instead of original", - }, - { - name: "windows path with backslashes", - episodeMD: "content\\episodes\\67.md", - wantErr: false, - description: "Cross-platform support for Windows paths", - }, - { - name: "uppercase .MD extension", - episodeMD: "EPISODE.MD", - wantErr: false, - description: "Case-insensitive extension matching", - }, - { - name: "missing argument", - episodeMD: "", - wantErr: true, - description: "User forgot to provide episode markdown file", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save and restore global state - originalEpisodeMD := CLI.EpisodeMD - defer func() { CLI.EpisodeMD = originalEpisodeMD }() - - // Set test value - CLI.EpisodeMD = tt.episodeMD - - // Call validateHugoMode - err := validateHugoMode() - - // Check error expectations - if tt.wantErr && err == nil { - t.Errorf("validateHugoMode() expected error but got nil\n Description: %s\n EpisodeMD=%q", - tt.description, tt.episodeMD) - } - if !tt.wantErr && err != nil { - t.Errorf("validateHugoMode() unexpected error: %v\n Description: %s\n EpisodeMD=%q", - err, tt.description, tt.episodeMD) - } - }) - } -} - -// TestValidateStandaloneMode tests standalone mode validation of required flags -func TestValidateStandaloneMode(t *testing.T) { - tests := []struct { - name string - title string - num string - cover string - wantErr bool - errMatch string // Substring to match in error message - }{ - // Valid cases: all flags present - { - name: "all flags present", - title: "Episode Title", - num: "1", - cover: "cover.png", - wantErr: false, - }, - { - name: "all flags with special characters", - title: "Episode: The Quest (Part 1)", - num: "42", - cover: "artwork/cover-2025.png", - wantErr: false, - }, - { - name: "all flags with long values", - title: "This is a very long episode title with lots of words and details", - num: "999", - cover: "/absolute/path/to/very/deep/directory/structure/cover.png", - wantErr: false, - }, - { - name: "all flags with minimal values", - title: "A", - num: "0", - cover: "c.png", - wantErr: false, - }, - { - name: "all flags with unicode in title", - title: "Podcast — Episode 1", - num: "1", - cover: "cover.png", - wantErr: false, - }, - - // Invalid cases: missing title - { - name: "missing title", - title: "", - num: "1", - cover: "cover.png", - wantErr: true, - errMatch: "requires --title flag", - }, - { - name: "missing title with num and cover", - title: "", - num: "42", - cover: "/path/to/cover.png", - wantErr: true, - errMatch: "requires --title flag", - }, - - // Invalid cases: missing num - { - name: "missing num", - title: "Episode Title", - num: "", - cover: "cover.png", - wantErr: true, - errMatch: "requires --num flag", - }, - { - name: "missing num with title and cover", - title: "My Show", - num: "", - cover: "artwork.png", - wantErr: true, - errMatch: "requires --num flag", - }, - - // Invalid cases: missing cover - { - name: "missing cover", - title: "Episode Title", - num: "1", - cover: "", - wantErr: true, - errMatch: "requires --cover flag", - }, - { - name: "missing cover with title and num", - title: "My Podcast", - num: "100", - cover: "", - wantErr: true, - errMatch: "requires --cover flag", - }, - - // Invalid cases: multiple flags missing - { - name: "missing title and num", - title: "", - num: "", - cover: "cover.png", - wantErr: true, - errMatch: "requires --title flag", // First validation fails - }, - { - name: "missing title and cover", - title: "", - num: "1", - cover: "", - wantErr: true, - errMatch: "requires --title flag", // First validation fails - }, - { - name: "missing num and cover", - title: "Episode", - num: "", - cover: "", - wantErr: true, - errMatch: "requires --num flag", // First validation fails - }, - { - name: "all flags empty", - title: "", - num: "", - cover: "", - wantErr: true, - errMatch: "requires --title flag", // First validation fails - }, - - // Edge cases: whitespace-only values (still invalid if they parse as empty) - { - name: "whitespace-only title (not trimmed by validator)", - title: " ", - num: "1", - cover: "cover.png", - wantErr: false, // Not empty string, so passes validation - }, - { - name: "whitespace-only num (not trimmed by validator)", - title: "Episode", - num: " ", - cover: "cover.png", - wantErr: false, // Not empty string, so passes validation - }, - - // Edge cases: numeric-like values - { - name: "num as string with leading zeros", - title: "Episode", - num: "007", - cover: "cover.png", - wantErr: false, - }, - { - name: "num as negative (still valid as string)", - title: "Episode", - num: "-1", - cover: "cover.png", - wantErr: false, - }, - { - name: "num as decimal (valid as string)", - title: "Episode", - num: "1.5", - cover: "cover.png", - wantErr: false, - }, - - // Cover art path variations - { - name: "cover as url", - title: "Episode", - num: "1", - cover: "https://example.com/cover.png", - wantErr: false, - }, - { - name: "cover as relative path", - title: "Episode", - num: "1", - cover: "./images/cover.png", - wantErr: false, - }, - { - name: "cover as absolute path", - title: "Episode", - num: "1", - cover: "/absolute/path/cover.png", - wantErr: false, - }, - { - name: "cover with spaces in path", - title: "Episode", - num: "1", - cover: "my cover art/final version.png", - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save and restore global state - originalTitle := CLI.Title - originalNum := CLI.Num - originalCover := CLI.Cover - defer func() { - CLI.Title = originalTitle - CLI.Num = originalNum - CLI.Cover = originalCover - }() - - // Set test values - CLI.Title = tt.title - CLI.Num = tt.num - CLI.Cover = tt.cover - - // Call validateStandaloneMode - err := validateStandaloneMode() - - // Check error expectations - if tt.wantErr { - if err == nil { - t.Errorf("validateStandaloneMode() expected error, got nil\n Title=%q, Num=%q, Cover=%q", - tt.title, tt.num, tt.cover) - return - } - if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { - t.Errorf("validateStandaloneMode() error %q does not contain %q", err.Error(), tt.errMatch) - } - return - } - - // Success case - if err != nil { - t.Errorf("validateStandaloneMode() unexpected error: %v\n Title=%q, Num=%q, Cover=%q", - err, tt.title, tt.num, tt.cover) - } - }) - } -} - -// TestValidateStandaloneMode_Integration tests validateStandaloneMode with realistic scenarios -func TestValidateStandaloneMode_Integration(t *testing.T) { - tests := []struct { - name string - title string - num string - cover string - wantErr bool - description string - }{ - { - name: "valid standalone workflow", - title: "Terminal Full of Sparkles", - num: "66", - cover: "artwork.png", - wantErr: false, - description: "User runs: jivedrop audio.flac --title 'Terminal Full of Sparkles' --num 66 --cover artwork.png", - }, - { - name: "common mistake: forgot --title flag", - title: "", - num: "42", - cover: "cover.png", - wantErr: true, - description: "User forgets required --title flag", - }, - { - name: "common mistake: forgot --num flag", - title: "My Episode", - num: "", - cover: "cover.png", - wantErr: true, - description: "User forgets required --num flag (episode number)", - }, - { - name: "common mistake: forgot --cover flag", - title: "My Episode", - num: "1", - cover: "", - wantErr: true, - description: "User forgets required --cover flag", - }, - { - name: "forgot all flags", - title: "", - num: "", - cover: "", - wantErr: true, - description: "User forgets all required flags", - }, - { - name: "minimal valid values", - title: "Ep", - num: "1", - cover: "art.png", - wantErr: false, - description: "Minimal valid values for standalone mode", - }, - { - name: "episode with complex path", - title: "The Daily Show", - num: "99", - cover: "/home/user/podcasts/2025/daily-show-99.png", - wantErr: false, - description: "Real-world example with full path to cover", - }, - { - name: "missing only cover field", - title: "Podcast Name", - num: "50", - cover: "", - wantErr: true, - description: "Most common mistake: user specifies --title and --num but forgets --cover", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save and restore global state - originalTitle := CLI.Title - originalNum := CLI.Num - originalCover := CLI.Cover - defer func() { - CLI.Title = originalTitle - CLI.Num = originalNum - CLI.Cover = originalCover - }() - - // Set test values - CLI.Title = tt.title - CLI.Num = tt.num - CLI.Cover = tt.cover - - // Call validateStandaloneMode - err := validateStandaloneMode() - - // Check error expectations - if tt.wantErr && err == nil { - t.Errorf("validateStandaloneMode() expected error but got nil\n Description: %s\n Title=%q, Num=%q, Cover=%q", - tt.description, tt.title, tt.num, tt.cover) - } - if !tt.wantErr && err != nil { - t.Errorf("validateStandaloneMode() unexpected error: %v\n Description: %s\n Title=%q, Num=%q, Cover=%q", - err, tt.description, tt.title, tt.num, tt.cover) - } - }) - } -} diff --git a/cmd/jivedrop/standalone.go b/cmd/jivedrop/standalone.go index f953921..ca9c46e 100644 --- a/cmd/jivedrop/standalone.go +++ b/cmd/jivedrop/standalone.go @@ -16,8 +16,16 @@ type StandaloneWorkflow struct{} // Validate checks standalone-specific arguments and file existence. func (s *StandaloneWorkflow) Validate() error { // Validate required standalone flags - if err := validateStandaloneMode(); err != nil { - return err + if CLI.Title == "" { + return fmt.Errorf("standalone mode requires --title flag") + } + + if CLI.Num == "" { + return fmt.Errorf("standalone mode requires --num flag (episode number)") + } + + if CLI.Cover == "" { + return fmt.Errorf("standalone mode requires --cover flag (cover art path)") } // Validate audio file exists diff --git a/cmd/jivedrop/standalone_test.go b/cmd/jivedrop/standalone_test.go new file mode 100644 index 0000000..7370a91 --- /dev/null +++ b/cmd/jivedrop/standalone_test.go @@ -0,0 +1,373 @@ +package main + +import ( + "strings" + "testing" +) + +// TestStandaloneWorkflowValidate tests standalone mode validation of required flags +func TestStandaloneWorkflowValidate(t *testing.T) { + tests := []struct { + name string + title string + num string + cover string + wantErr bool + errMatch string // Substring to match in error message + }{ + // Valid cases: all flags present + { + name: "all flags present", + title: "Episode Title", + num: "1", + cover: "cover.png", + wantErr: false, + }, + { + name: "all flags with special characters", + title: "Episode: The Quest (Part 1)", + num: "42", + cover: "artwork/cover-2025.png", + wantErr: false, + }, + { + name: "all flags with long values", + title: "This is a very long episode title with lots of words and details", + num: "999", + cover: "/absolute/path/to/very/deep/directory/structure/cover.png", + wantErr: false, + }, + { + name: "all flags with minimal values", + title: "A", + num: "0", + cover: "c.png", + wantErr: false, + }, + { + name: "all flags with unicode in title", + title: "Podcast — Episode 1", + num: "1", + cover: "cover.png", + wantErr: false, + }, + + // Invalid cases: missing title + { + name: "missing title", + title: "", + num: "1", + cover: "cover.png", + wantErr: true, + errMatch: "requires --title flag", + }, + { + name: "missing title with num and cover", + title: "", + num: "42", + cover: "/path/to/cover.png", + wantErr: true, + errMatch: "requires --title flag", + }, + + // Invalid cases: missing num + { + name: "missing num", + title: "Episode Title", + num: "", + cover: "cover.png", + wantErr: true, + errMatch: "requires --num flag", + }, + { + name: "missing num with title and cover", + title: "My Show", + num: "", + cover: "artwork.png", + wantErr: true, + errMatch: "requires --num flag", + }, + + // Invalid cases: missing cover + { + name: "missing cover", + title: "Episode Title", + num: "1", + cover: "", + wantErr: true, + errMatch: "requires --cover flag", + }, + { + name: "missing cover with title and num", + title: "My Podcast", + num: "100", + cover: "", + wantErr: true, + errMatch: "requires --cover flag", + }, + + // Invalid cases: multiple flags missing + { + name: "missing title and num", + title: "", + num: "", + cover: "cover.png", + wantErr: true, + errMatch: "requires --title flag", // First validation fails + }, + { + name: "missing title and cover", + title: "", + num: "1", + cover: "", + wantErr: true, + errMatch: "requires --title flag", // First validation fails + }, + { + name: "missing num and cover", + title: "Episode", + num: "", + cover: "", + wantErr: true, + errMatch: "requires --num flag", // First validation fails + }, + { + name: "all flags empty", + title: "", + num: "", + cover: "", + wantErr: true, + errMatch: "requires --title flag", // First validation fails + }, + + // Edge cases: whitespace-only values (still invalid if they parse as empty) + { + name: "whitespace-only title (not trimmed by validator)", + title: " ", + num: "1", + cover: "cover.png", + wantErr: false, // Not empty string, so passes validation + }, + { + name: "whitespace-only num (not trimmed by validator)", + title: "Episode", + num: " ", + cover: "cover.png", + wantErr: false, // Not empty string, so passes validation + }, + + // Edge cases: numeric-like values + { + name: "num as string with leading zeros", + title: "Episode", + num: "007", + cover: "cover.png", + wantErr: false, + }, + { + name: "num as negative (still valid as string)", + title: "Episode", + num: "-1", + cover: "cover.png", + wantErr: false, + }, + { + name: "num as decimal (valid as string)", + title: "Episode", + num: "1.5", + cover: "cover.png", + wantErr: false, + }, + + // Cover art path variations + { + name: "cover as url", + title: "Episode", + num: "1", + cover: "https://example.com/cover.png", + wantErr: false, + }, + { + name: "cover as relative path", + title: "Episode", + num: "1", + cover: "./images/cover.png", + wantErr: false, + }, + { + name: "cover as absolute path", + title: "Episode", + num: "1", + cover: "/absolute/path/cover.png", + wantErr: false, + }, + { + name: "cover with spaces in path", + title: "Episode", + num: "1", + cover: "my cover art/final version.png", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save and restore global state + originalTitle := CLI.Title + originalNum := CLI.Num + originalCover := CLI.Cover + originalAudioFile := CLI.AudioFile + defer func() { + CLI.Title = originalTitle + CLI.Num = originalNum + CLI.Cover = originalCover + CLI.AudioFile = originalAudioFile + }() + + // Set test values + CLI.Title = tt.title + CLI.Num = tt.num + CLI.Cover = tt.cover + // Set a dummy audio file so file-existence checks do not mask + // the argument validation errors we are testing for. + CLI.AudioFile = "testdata" + + // Call Validate on StandaloneWorkflow + wf := &StandaloneWorkflow{} + err := wf.Validate() + + // Check error expectations + if tt.wantErr { + if err == nil { + t.Errorf("StandaloneWorkflow.Validate() expected error, got nil\n Title=%q, Num=%q, Cover=%q", + tt.title, tt.num, tt.cover) + return + } + if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { + t.Errorf("StandaloneWorkflow.Validate() error %q does not contain %q", err.Error(), tt.errMatch) + } + return + } + + // For valid cases we only check argument validation, not file existence. + // File-not-found errors are acceptable here since the cover files do not exist on disk. + if err != nil && !strings.Contains(err.Error(), "not found") { + t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Title=%q, Num=%q, Cover=%q", + err, tt.title, tt.num, tt.cover) + } + }) + } +} + +// TestStandaloneWorkflowValidate_Integration tests StandaloneWorkflow.Validate with realistic scenarios +func TestStandaloneWorkflowValidate_Integration(t *testing.T) { + tests := []struct { + name string + title string + num string + cover string + wantErr bool + description string + }{ + { + name: "valid standalone workflow", + title: "Terminal Full of Sparkles", + num: "66", + cover: "artwork.png", + wantErr: false, + description: "User runs: jivedrop audio.flac --title 'Terminal Full of Sparkles' --num 66 --cover artwork.png", + }, + { + name: "common mistake: forgot --title flag", + title: "", + num: "42", + cover: "cover.png", + wantErr: true, + description: "User forgets required --title flag", + }, + { + name: "common mistake: forgot --num flag", + title: "My Episode", + num: "", + cover: "cover.png", + wantErr: true, + description: "User forgets required --num flag (episode number)", + }, + { + name: "common mistake: forgot --cover flag", + title: "My Episode", + num: "1", + cover: "", + wantErr: true, + description: "User forgets required --cover flag", + }, + { + name: "forgot all flags", + title: "", + num: "", + cover: "", + wantErr: true, + description: "User forgets all required flags", + }, + { + name: "minimal valid values", + title: "Ep", + num: "1", + cover: "art.png", + wantErr: false, + description: "Minimal valid values for standalone mode", + }, + { + name: "episode with complex path", + title: "The Daily Show", + num: "99", + cover: "/home/user/podcasts/2025/daily-show-99.png", + wantErr: false, + description: "Real-world example with full path to cover", + }, + { + name: "missing only cover field", + title: "Podcast Name", + num: "50", + cover: "", + wantErr: true, + description: "Most common mistake: user specifies --title and --num but forgets --cover", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Save and restore global state + originalTitle := CLI.Title + originalNum := CLI.Num + originalCover := CLI.Cover + originalAudioFile := CLI.AudioFile + defer func() { + CLI.Title = originalTitle + CLI.Num = originalNum + CLI.Cover = originalCover + CLI.AudioFile = originalAudioFile + }() + + // Set test values + CLI.Title = tt.title + CLI.Num = tt.num + CLI.Cover = tt.cover + CLI.AudioFile = "testdata" + + // Call Validate on StandaloneWorkflow + wf := &StandaloneWorkflow{} + err := wf.Validate() + + // Check error expectations + if tt.wantErr && err == nil { + t.Errorf("StandaloneWorkflow.Validate() expected error but got nil\n Description: %s\n Title=%q, Num=%q, Cover=%q", + tt.description, tt.title, tt.num, tt.cover) + } + if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") { + t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Description: %s\n Title=%q, Num=%q, Cover=%q", + err, tt.description, tt.title, tt.num, tt.cover) + } + }) + } +} diff --git a/internal/id3/writer.go b/internal/id3/writer.go index 7ae140d..96e83b9 100644 --- a/internal/id3/writer.go +++ b/internal/id3/writer.go @@ -65,9 +65,7 @@ func WriteTags(mp3Path string, info TagInfo) error { // APIC: Cover art if len(info.CoverArtData) > 0 { - if err := addCoverArtData(tag, info.CoverArtData, info.Artist, info.Description); err != nil { - return fmt.Errorf("failed to add cover art: %w", err) - } + addCoverArtData(tag, info.CoverArtData, info.Artist, info.Description) } // Save the tag @@ -80,7 +78,7 @@ func WriteTags(mp3Path string, info TagInfo) error { // addCoverArtData adds pre-processed cover artwork as an APIC frame // If description is empty, defaults to "{artist} Logo" -func addCoverArtData(tag *id3v2.Tag, artwork []byte, artist, description string) error { +func addCoverArtData(tag *id3v2.Tag, artwork []byte, artist, description string) { // Default description to "{artist} Logo" if not provided desc := description if desc == "" && artist != "" { @@ -97,6 +95,4 @@ func addCoverArtData(tag *id3v2.Tag, artwork []byte, artist, description string) } tag.AddAttachedPicture(pic) - - return nil } From 178b4c19e5e185579a8e026478a45cf811d2fa84 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Tue, 17 Mar 2026 18:14:52 +0000 Subject: [PATCH 7/7] fix(workflows): handle all os.Stat errors and suppress cover art logging - Replace os.IsNotExist checks with full error handling in Hugo and Standalone workflow validation; catches permission errors, broken symlinks, and I/O failures early - Pass nil to cli.PrintCover callback in main.go to suppress logging during Bubble Tea UI rendering and prevent stdout corruption of progress display - Update tests for both workflow implementations Signed-off-by: Martin Wimpress --- cmd/jivedrop/hugo.go | 16 ++++++++-------- cmd/jivedrop/hugo_test.go | 4 ++-- cmd/jivedrop/main.go | 2 +- cmd/jivedrop/standalone.go | 12 ++++++------ cmd/jivedrop/standalone_test.go | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/cmd/jivedrop/hugo.go b/cmd/jivedrop/hugo.go index 778909f..25ea191 100644 --- a/cmd/jivedrop/hugo.go +++ b/cmd/jivedrop/hugo.go @@ -28,20 +28,20 @@ func (h *HugoWorkflow) Validate() error { return fmt.Errorf("episode markdown file must have .md extension: %s", CLI.EpisodeMD) } - // Validate audio file exists - if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { - return fmt.Errorf("audio file not found: %s", CLI.AudioFile) + // Validate audio file exists and is accessible + if _, err := os.Stat(CLI.AudioFile); err != nil { + return fmt.Errorf("audio file not accessible: %w", err) } - // Validate episode markdown file exists - if _, err := os.Stat(CLI.EpisodeMD); os.IsNotExist(err) { - return fmt.Errorf("episode file not found: %s", CLI.EpisodeMD) + // Validate episode markdown file exists and is accessible + if _, err := os.Stat(CLI.EpisodeMD); err != nil { + return fmt.Errorf("episode file not accessible: %w", err) } // Validate custom cover art exists (if specified) if CLI.Cover != "" { - if _, err := os.Stat(CLI.Cover); os.IsNotExist(err) { - return fmt.Errorf("cover art not found: %s", CLI.Cover) + if _, err := os.Stat(CLI.Cover); err != nil { + return fmt.Errorf("cover art not accessible: %w", err) } } diff --git a/cmd/jivedrop/hugo_test.go b/cmd/jivedrop/hugo_test.go index 23c28e0..d903372 100644 --- a/cmd/jivedrop/hugo_test.go +++ b/cmd/jivedrop/hugo_test.go @@ -188,7 +188,7 @@ func TestHugoWorkflowValidate(t *testing.T) { // For valid cases we only check argument validation, not file existence. // File-not-found errors are acceptable here since the files do not exist on disk. - if err != nil && !strings.Contains(err.Error(), "not found") { + if err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, tt.episodeMD) } }) @@ -270,7 +270,7 @@ func TestHugoWorkflowValidate_Integration(t *testing.T) { t.Errorf("HugoWorkflow.Validate() expected error but got nil\n Description: %s\n EpisodeMD=%q", tt.description, tt.episodeMD) } - if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") { + if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { t.Errorf("HugoWorkflow.Validate() unexpected error: %v\n Description: %s\n EpisodeMD=%q", err, tt.description, tt.episodeMD) } diff --git a/cmd/jivedrop/main.go b/cmd/jivedrop/main.go index 6a482f9..9b80547 100644 --- a/cmd/jivedrop/main.go +++ b/cmd/jivedrop/main.go @@ -219,7 +219,7 @@ func encode(mode WorkflowMode, tagInfo id3.TagInfo, coverArtPath, outputPath str return } - artwork, artErr := id3.ScaleCoverArt(coverArtPath, cli.PrintCover) + artwork, artErr := id3.ScaleCoverArt(coverArtPath, nil) coverArtChan <- coverArtResult{data: artwork, err: artErr} }() diff --git a/cmd/jivedrop/standalone.go b/cmd/jivedrop/standalone.go index ca9c46e..5a8e81a 100644 --- a/cmd/jivedrop/standalone.go +++ b/cmd/jivedrop/standalone.go @@ -28,14 +28,14 @@ func (s *StandaloneWorkflow) Validate() error { return fmt.Errorf("standalone mode requires --cover flag (cover art path)") } - // Validate audio file exists - if _, err := os.Stat(CLI.AudioFile); os.IsNotExist(err) { - return fmt.Errorf("audio file not found: %s", CLI.AudioFile) + // Validate audio file exists and is accessible + if _, err := os.Stat(CLI.AudioFile); err != nil { + return fmt.Errorf("audio file not accessible: %w", err) } - // Validate cover art exists - if _, err := os.Stat(CLI.Cover); os.IsNotExist(err) { - return fmt.Errorf("cover art not found: %s", CLI.Cover) + // Validate cover art exists and is accessible + if _, err := os.Stat(CLI.Cover); err != nil { + return fmt.Errorf("cover art not accessible: %w", err) } return nil diff --git a/cmd/jivedrop/standalone_test.go b/cmd/jivedrop/standalone_test.go index 7370a91..e7108e2 100644 --- a/cmd/jivedrop/standalone_test.go +++ b/cmd/jivedrop/standalone_test.go @@ -251,7 +251,7 @@ func TestStandaloneWorkflowValidate(t *testing.T) { // For valid cases we only check argument validation, not file existence. // File-not-found errors are acceptable here since the cover files do not exist on disk. - if err != nil && !strings.Contains(err.Error(), "not found") { + if err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Title=%q, Num=%q, Cover=%q", err, tt.title, tt.num, tt.cover) } @@ -364,7 +364,7 @@ func TestStandaloneWorkflowValidate_Integration(t *testing.T) { t.Errorf("StandaloneWorkflow.Validate() expected error but got nil\n Description: %s\n Title=%q, Num=%q, Cover=%q", tt.description, tt.title, tt.num, tt.cover) } - if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") { + if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Description: %s\n Title=%q, Num=%q, Cover=%q", err, tt.description, tt.title, tt.num, tt.cover) }