feat(converter): add --keep-filenames flag to preserve original page filenames

Add a new `--keep-filenames` boolean flag (no shorthand, default false) on both `optimize` and `watch` commands. When enabled, page entries in the output CBZ preserve their original base filenames instead of being renumbered to `%04d` sequential indices. Extension is swapped on format conversion (e.g. `001.png` → `001.webp`), split pages get `-NN` suffixes on the original stem, and duplicate base names fall back to `<stem>_<index>.<ext>`. Watch mode supports the flag via the `keep-filenames` viper config key. Design: data-driven `OriginalName` field on `manga.PageFile`, set at extraction in `cbz_loader.ExtractChapter`. The `Converter` interface is unchanged. Flag off → behavior identical to before. Closes #216
2026-07-22 03:15:41 +02:00 · 2026-07-21 19:38:17 -04:00
parent 650623b340
commit f3ba585c13
12 changed files with 245 additions and 37 deletions
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path/filepath"
+	"strings"
 	"time"

 	"github.com/belphemur/CBZOptimizer/v2/internal/manga"
@@ -12,6 +14,49 @@ import (
 	"github.com/rs/zerolog/log"
 )

+// resolvePageName picks the final archive entry name for a page.
+//
+// Order of resolution:
+//  1. When page.OriginalName is set (keep-filenames mode), use its stem with
+//     the current page.Extension. Split pages append the -NN suffix so all
+//     parts of the same source still land in the archive.
+//  2. If that final name has already been used in this archive, fall back to
+//     the indexed form (stem + "_%04d" + extension) so the zip stays valid.
+//  3. Otherwise (OriginalName is empty), use the historical %04d / %04d-NN
+//     sequential naming.
+//
+// usedNames tracks every name already chosen for this archive and is mutated
+// in place so the caller can keep a single map across the whole chapter.
+func resolvePageName(page *manga.PageFile, usedNames map[string]struct{}) string {
+	if page.OriginalName != "" {
+		stem := strings.TrimSuffix(page.OriginalName, filepath.Ext(page.OriginalName))
+		var candidate string
+		if page.IsSplitted {
+			candidate = fmt.Sprintf("%s-%02d%s", stem, page.SplitPartIndex, page.Extension)
+		} else {
+			candidate = stem + page.Extension
+		}
+		if _, taken := usedNames[candidate]; !taken {
+			usedNames[candidate] = struct{}{}
+			return candidate
+		}
+		// Collision: fall back to the indexed form so the entry still gets
+		// written, but make it visibly distinct from the preserved one.
+		fallback := fmt.Sprintf("%s_%04d%s", stem, page.Index, page.Extension)
+		usedNames[fallback] = struct{}{}
+		return fallback
+	}
+
+	if page.IsSplitted {
+		name := fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
+		usedNames[name] = struct{}{}
+		return name
+	}
+	name := fmt.Sprintf("%04d%s", page.Index, page.Extension)
+	usedNames[name] = struct{}{}
+	return name
+}
+
 // WriteChapterToCBZ creates a CBZ file from a Chapter by streaming page files
 // from disk directly into the zip archive. No image data is held in memory.
 func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error) {
@@ -34,14 +79,14 @@ func WriteChapterToCBZ(chapter *manga.Chapter, outputFilePath string) (err error
 	zipWriter := zip.NewWriter(zipFile)
 	defer errs.Capture(&err, zipWriter.Close, "failed to close .cbz writer")

-	// Write each page to the archive by streaming from disk
+	// Write each page to the archive by streaming from disk.
+	// Final name resolution: when a page carries an OriginalName (recorded by
+	// ExtractChapter when --keep-filenames is on), preserve its stem and only
+	// swap the extension to the current page.Extension. Duplicates in the
+	// archive fall back to the indexed naming so the output stays a valid zip.
+	usedNames := make(map[string]struct{}, len(chapter.Pages))
 	for _, page := range chapter.Pages {
-		var fileName string
-		if page.IsSplitted {
-			fileName = fmt.Sprintf("%04d-%02d%s", page.Index, page.SplitPartIndex, page.Extension)
-		} else {
-			fileName = fmt.Sprintf("%04d%s", page.Index, page.Extension)
-		}
+		fileName := resolvePageName(page, usedNames)

 		log.Debug().
 			Str("output_path", outputFilePath).
@@ -101,6 +101,70 @@ func TestWriteChapterToCBZ(t *testing.T) {
 			},
 			expectedFiles: []string{"0000-01.jpg"},
 		},
+		{
+			// --keep-filenames: the page stem is reused as-is with the
+			// current Extension, regardless of the source extension.
+			name: "Preserve original filename, extension swap",
+			chapter: func(t *testing.T, dir string) *manga.Chapter {
+				return &manga.Chapter{
+					Pages: []*manga.PageFile{
+						{
+							Index:        0,
+							Extension:    ".webp",
+							FilePath:     createTempPage(t, dir, "image data", ".webp"),
+							OriginalName: "page01.png",
+						},
+					},
+				}
+			},
+			expectedFiles: []string{"page01.webp"},
+		},
+		{
+			// --keep-filenames for a split page: stem is preserved, the
+			// split part index still gets appended to keep parts distinct.
+			name: "Split page with OriginalName",
+			chapter: func(t *testing.T, dir string) *manga.Chapter {
+				return &manga.Chapter{
+					Pages: []*manga.PageFile{
+						{
+							Index:          0,
+							Extension:      ".webp",
+							FilePath:       createTempPage(t, dir, "split data", ".webp"),
+							IsSplitted:     true,
+							SplitPartIndex: 2,
+							OriginalName:   "tall.png",
+						},
+					},
+				}
+			},
+			expectedFiles: []string{"tall-02.webp"},
+		},
+		{
+			// Two pages share the same OriginalName (rare but possible
+			// with subdirectories): the first keeps the stem, the second
+			// falls back to the indexed form to avoid duplicate zip
+			// entries.
+			name: "Duplicate OriginalName falls back to indexed form",
+			chapter: func(t *testing.T, dir string) *manga.Chapter {
+				return &manga.Chapter{
+					Pages: []*manga.PageFile{
+						{
+							Index:        0,
+							Extension:    ".webp",
+							FilePath:     createTempPage(t, dir, "first", ".webp"),
+							OriginalName: "cover.png",
+						},
+						{
+							Index:        1,
+							Extension:    ".webp",
+							FilePath:     createTempPage(t, dir, "second", ".webp"),
+							OriginalName: "cover.png",
+						},
+					},
+				}
+			},
+			expectedFiles: []string{"cover.webp", "cover_0001.webp"},
+		},
 	}

 	for _, tc := range testCases {
@@ -142,7 +142,13 @@ func IsAlreadyConverted(ctx context.Context, filePath string) (converted bool, e
 // ExtractChapter extracts an archive (CBZ/CBR) to a temp directory on disk.
 // Pages are streamed directly to files — no image data is held in memory.
 // Returns a Chapter with PageFile entries pointing to extracted files.
-func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error) {
+//
+// When keepFilenames is true, each PageFile has its OriginalName set to the
+// base filename of the entry inside the archive. Downstream code uses that
+// name to preserve the original page identity in the output CBZ (with the
+// extension swapped for format conversion). When false, OriginalName stays
+// empty and the sequential %04d naming convention is used instead.
+func ExtractChapter(ctx context.Context, filePath string, keepFilenames bool) (*manga.Chapter, error) {
 	log.Debug().Str("file_path", filePath).Msg("Extracting chapter to disk")

 	// Create temp directory for extraction
@@ -279,6 +285,9 @@ func ExtractChapter(ctx context.Context, filePath string) (*manga.Chapter, error
 			Extension: ext,
 			FilePath:  outputPath,
 		}
+		if keepFilenames {
+			page.OriginalName = filepath.Base(path)
+		}
 		chapter.Pages = append(chapter.Pages, page)

 		log.Debug().
@@ -320,8 +329,9 @@ func isJunkFile(path string) bool {
 }

 // LoadChapter extracts the chapter from a CBZ/CBR file to disk.
-// It delegates to ExtractChapter and always extracts all pages.
-// Use IsAlreadyConverted for a fast conversion status check without extraction.
+// It delegates to ExtractChapter with keepFilenames=false and always
+// extracts all pages. Use IsAlreadyConverted for a fast conversion status
+// check without extraction.
 func LoadChapter(filePath string) (*manga.Chapter, error) {
-	return ExtractChapter(context.Background(), filePath)
+	return ExtractChapter(context.Background(), filePath, false)
 }
@@ -157,12 +157,12 @@ func TestIsAlreadyConverted_CBZWithNonDateComment(t *testing.T) {
 }

 func TestExtractChapter_NonexistentFile(t *testing.T) {
-	_, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz")
+	_, err := ExtractChapter(context.Background(), "/nonexistent/file.cbz", false)
 	require.Error(t, err)
 }

 func TestExtractChapter_PageExtensions(t *testing.T) {
-	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
+	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
 	require.NoError(t, err)
 	defer func() { _ = chapter.Cleanup() }()

@@ -174,17 +174,36 @@ func TestExtractChapter_PageExtensions(t *testing.T) {
 }

 func TestExtractChapter_PagesHaveSequentialIndices(t *testing.T) {
-	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
-	require.NoError(t, err)
-	defer func() { _ = chapter.Cleanup() }()
+	t.Run("default sequential naming", func(t *testing.T) {
+		chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
+		require.NoError(t, err)
+		defer func() { _ = chapter.Cleanup() }()

-	for i, page := range chapter.Pages {
-		assert.Equal(t, uint16(i), page.Index)
-	}
+		for i, page := range chapter.Pages {
+			assert.Equal(t, uint16(i), page.Index)
+			assert.Empty(t, page.OriginalName, "keep-filenames disabled: OriginalName must stay empty")
+		}
+	})
+
+	t.Run("keep-filenames records OriginalName", func(t *testing.T) {
+		chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", true)
+		require.NoError(t, err)
+		defer func() { _ = chapter.Cleanup() }()
+
+		require.NotEmpty(t, chapter.Pages)
+		for i, page := range chapter.Pages {
+			assert.Equal(t, uint16(i), page.Index)
+			assert.NotEmpty(t, page.OriginalName, "keep-filenames enabled: OriginalName must be set for every page")
+			// OriginalName must be a bare base name (no directory prefix), even
+			// when the source archive nests pages in subdirectories.
+			assert.Equal(t, filepath.Base(page.OriginalName), page.OriginalName,
+				"OriginalName should be a base filename with no path components")
+		}
+	})
 }

 func TestExtractChapter_Cleanup(t *testing.T) {
-	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz")
+	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 128.cbz", false)
 	require.NoError(t, err)

 	tempDir := chapter.TempDir
@@ -199,7 +218,7 @@ func TestExtractChapter_Cleanup(t *testing.T) {
 }

 func TestExtractChapter_CBR(t *testing.T) {
-	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr")
+	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 1.cbr", false)
 	require.NoError(t, err)
 	defer func() { _ = chapter.Cleanup() }()

@@ -212,7 +231,7 @@ func TestExtractChapter_CBR(t *testing.T) {
 }

 func TestExtractChapter_ConvertedStatus(t *testing.T) {
-	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz")
+	chapter, err := ExtractChapter(context.Background(), "../../testdata/Chapter 10_converted.cbz", false)
 	require.NoError(t, err)
 	defer func() { _ = chapter.Cleanup() }()

@@ -356,7 +375,7 @@ func TestExtractChapter_WithConvertedTxt(t *testing.T) {
 	_ = w.Close()
 	_ = f.Close()

-	chapter, err := ExtractChapter(context.Background(), cbzPath)
+	chapter, err := ExtractChapter(context.Background(), cbzPath, false)
 	require.NoError(t, err)
 	defer func() { _ = chapter.Cleanup() }()