From e2577fbad47540eaf031b925d2c08558bbc089bf Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 28 Jun 2026 14:26:32 -0400 Subject: feat(reading): reformat PDF bookmark names like EPUBs Bookmark reformatting to "Author, Title" was nov-only, so PDF bookmarks kept the raw filename. PDFs open in pdf-view-mode, whose pdf-view-bookmark-make-record carried no advice. I added a parallel :filter-return advice there, reusing the same extension-agnostic filename parser, and renamed the helpers off the nov- prefix to reading- since they now serve both EPUB and PDF. New tests cover a PDF filename and a PDF-shaped record. --- modules/calibredb-epub-config.el | 48 ++++++----- tests/test-calibredb-epub-config--bookmark-name.el | 94 +++++++++++++--------- 2 files changed, 84 insertions(+), 58 deletions(-) diff --git a/modules/calibredb-epub-config.el b/modules/calibredb-epub-config.el index 10a78942c..8518ffe91 100644 --- a/modules/calibredb-epub-config.el +++ b/modules/calibredb-epub-config.el @@ -408,14 +408,15 @@ Try to use the Calibre book id from the parent folder name (for example, ;; ------------------------- Nov bookmark naming ------------------------------- ;; In a nov buffer "m" is bound to `bookmark-set' (above). nov's -;; `nov-bookmark-make-record' names the record after `(buffer-name)' -- the EPUB -;; filename, extension and all. Rebuild it as "Author, Title" parsed from the -;; filename: under Calibre's " - <Author>.epub" naming the filename is -;; more complete than the EPUB's embedded metadata (which carries truncated -;; titles and author-sort "Last, First" forms). - -(defun cj/--nov-clean-title (s) - "Clean a title or author S parsed from an EPUB filename, or nil when blank. +;; Both nov (EPUB) and pdf-view (PDF) name a new bookmark after the buffer -- +;; the file's name, extension and all. Rebuild it as "Author, Title" parsed +;; from the filename: under Calibre's "<Title> - <Author>.<ext>" naming the +;; filename is more complete than the file's embedded metadata (which carries +;; truncated titles and author-sort "Last, First" forms). One :filter-return +;; advice serves both record functions; the parser is extension-agnostic. + +(defun cj/--reading-clean-title (s) + "Clean a title or author S parsed from a book filename, or nil when blank. Restores a colon where Calibre sanitized \":\" to \"_\" (\"Frege_ A Guide\" -> \"Frege: A Guide\"), turns any leftover underscore into a space, and collapses runs of whitespace." @@ -425,34 +426,39 @@ collapses runs of whitespace." (out (string-trim (replace-regexp-in-string "[ \t]+" " " spaced)))) (and (not (string-empty-p out)) out)))) -(defun cj/--nov-bookmark-name-from-file (path) - "Return \"Author, Title\" derived from an EPUB PATH's filename, or nil. +(defun cj/--reading-bookmark-name-from-file (path) + "Return \"Author, Title\" derived from a book PATH's filename, or nil. Splits the filename (sans extension) on its last \" - \" into title and author per Calibre's \"<Title> - <Author>\" convention, restoring colons and reordering to \"Author, Title\". Falls back to the cleaned whole name when -there is no \" - \" separator." +there is no \" - \" separator. Extension-agnostic, so it serves EPUB and PDF." (when (and (stringp path) (not (string-empty-p path))) (let ((base (file-name-sans-extension (file-name-nondirectory path)))) (if (string-match "\\`\\(.+\\) - \\(.+\\)\\'" base) - (let ((title (cj/--nov-clean-title (match-string 1 base))) - (author (cj/--nov-clean-title (match-string 2 base)))) + (let ((title (cj/--reading-clean-title (match-string 1 base))) + (author (cj/--reading-clean-title (match-string 2 base)))) (cond ((and author title) (format "%s, %s" author title)) (title title) (author author) (t nil))) - (cj/--nov-clean-title base))))) - -(defun cj/--nov-bookmark-rename-record (record) - "Replace RECORD's bookmark name with \"Author, Title\" from its EPUB filename. -Advice (:filter-return) on `nov-bookmark-make-record'. RECORD is -\(NAME . ALIST) carrying a `filename'; left unchanged when no name derives." - (let ((name (cj/--nov-bookmark-name-from-file + (cj/--reading-clean-title base))))) + +(defun cj/--reading-bookmark-rename-record (record) + "Replace RECORD's bookmark name with \"Author, Title\" from its filename. +Advice (:filter-return) on `nov-bookmark-make-record' and +`pdf-view-bookmark-make-record'. RECORD is (NAME . ALIST) carrying a +`filename'; left unchanged when no name derives." + (let ((name (cj/--reading-bookmark-name-from-file (alist-get 'filename (cdr record))))) (if name (cons name (cdr record)) record))) (with-eval-after-load 'nov (advice-add 'nov-bookmark-make-record :filter-return - #'cj/--nov-bookmark-rename-record)) + #'cj/--reading-bookmark-rename-record)) + +(with-eval-after-load 'pdf-view + (advice-add 'pdf-view-bookmark-make-record :filter-return + #'cj/--reading-bookmark-rename-record)) (defun cj/--nov-image-padding-cols (col-width img-px font-width-px) "Return left-padding columns to center an IMG-PX-wide image in COL-WIDTH cols. diff --git a/tests/test-calibredb-epub-config--bookmark-name.el b/tests/test-calibredb-epub-config--bookmark-name.el index 2e1d253e9..7e9ffa345 100644 --- a/tests/test-calibredb-epub-config--bookmark-name.el +++ b/tests/test-calibredb-epub-config--bookmark-name.el @@ -1,10 +1,13 @@ -;;; test-calibredb-epub-config--bookmark-name.el --- Nov bookmark naming tests -*- lexical-binding: t; -*- +;;; test-calibredb-epub-config--bookmark-name.el --- Reading bookmark naming tests -*- lexical-binding: t; -*- ;;; Commentary: -;; Tests for the clean "Author, Title" bookmark naming that replaces nov.el's -;; filename-based default. The name is parsed from the EPUB filename (Calibre's -;; "<Title> - <Author>.epub" convention), restoring colons that Calibre -;; sanitized to underscores and reordering to "Author, Title". +;; Tests for the clean "Author, Title" bookmark naming that replaces the +;; filename-based default for both EPUB (nov) and PDF (pdf-view) bookmarks. +;; The name is parsed from the file's name (Calibre's "<Title> - <Author>.<ext>" +;; convention), restoring colons that Calibre sanitized to underscores and +;; reordering to "Author, Title". The parser is extension-agnostic, so the +;; same advice serves both nov-bookmark-make-record and +;; pdf-view-bookmark-make-record. ;;; Code: @@ -12,76 +15,93 @@ (add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) (require 'calibredb-epub-config) -;;; cj/--nov-clean-title +;;; cj/--reading-clean-title -(ert-deftest test-nov-clean-title-passthrough () +(ert-deftest test-reading-clean-title-passthrough () "Normal: a clean string is returned unchanged." - (should (equal (cj/--nov-clean-title "Agatha Christie") "Agatha Christie")) - (should (equal (cj/--nov-clean-title "The A.B.C. Murders") "The A.B.C. Murders"))) + (should (equal (cj/--reading-clean-title "Agatha Christie") "Agatha Christie")) + (should (equal (cj/--reading-clean-title "The A.B.C. Murders") "The A.B.C. Murders"))) -(ert-deftest test-nov-clean-title-restores-colon () +(ert-deftest test-reading-clean-title-restores-colon () "Boundary: Calibre's \"_ \" colon substitution is restored to \": \"." - (should (equal (cj/--nov-clean-title "Frege_ A Guide for the Perplexed") + (should (equal (cj/--reading-clean-title "Frege_ A Guide for the Perplexed") "Frege: A Guide for the Perplexed")) - (should (equal (cj/--nov-clean-title "The Fool's Progress_ An Honest Novel") + (should (equal (cj/--reading-clean-title "The Fool's Progress_ An Honest Novel") "The Fool's Progress: An Honest Novel"))) -(ert-deftest test-nov-clean-title-stray-underscore-and-whitespace () +(ert-deftest test-reading-clean-title-stray-underscore-and-whitespace () "Boundary: a non-colon underscore becomes a space; whitespace collapses." - (should (equal (cj/--nov-clean-title "a_b") "a b")) - (should (equal (cj/--nov-clean-title " x y ") "x y"))) + (should (equal (cj/--reading-clean-title "a_b") "a b")) + (should (equal (cj/--reading-clean-title " x y ") "x y"))) -(ert-deftest test-nov-clean-title-rejects-blank-and-nonstring () +(ert-deftest test-reading-clean-title-rejects-blank-and-nonstring () "Error: nil, empty, all-whitespace, or non-string yields nil." - (should-not (cj/--nov-clean-title nil)) - (should-not (cj/--nov-clean-title "")) - (should-not (cj/--nov-clean-title " ")) - (should-not (cj/--nov-clean-title 42))) + (should-not (cj/--reading-clean-title nil)) + (should-not (cj/--reading-clean-title "")) + (should-not (cj/--reading-clean-title " ")) + (should-not (cj/--reading-clean-title 42))) -;;; cj/--nov-bookmark-name-from-file +;;; cj/--reading-bookmark-name-from-file -(ert-deftest test-nov-bookmark-name-real-examples () +(ert-deftest test-reading-bookmark-name-real-examples () "Normal: real Calibre filenames become \"Author, Title\" with colons restored." - (should (equal (cj/--nov-bookmark-name-from-file + (should (equal (cj/--reading-bookmark-name-from-file "/books/Frege_ A Guide for the Perplexed - Edward Kanterian.epub") "Edward Kanterian, Frege: A Guide for the Perplexed")) - (should (equal (cj/--nov-bookmark-name-from-file + (should (equal (cj/--reading-bookmark-name-from-file "/books/The A.B.C. Murders - Agatha Christie.epub") "Agatha Christie, The A.B.C. Murders")) - (should (equal (cj/--nov-bookmark-name-from-file + (should (equal (cj/--reading-bookmark-name-from-file "/books/The Fool's Progress_ An Honest Novel - Edward Abbey.epub") "Edward Abbey, The Fool's Progress: An Honest Novel"))) -(ert-deftest test-nov-bookmark-name-splits-on-last-separator () +(ert-deftest test-reading-bookmark-name-pdf-extension () + "Normal: a PDF filename is parsed the same way -- the parser is extension- +agnostic, so PDF bookmarks reformat like EPUB ones." + (should (equal (cj/--reading-bookmark-name-from-file + "/books/Engines of Logic_ Mathematicians and the O - Martin Davis.pdf") + "Martin Davis, Engines of Logic: Mathematicians and the O"))) + +(ert-deftest test-reading-bookmark-name-splits-on-last-separator () "Boundary: a title containing \" - \" splits on the LAST separator." - (should (equal (cj/--nov-bookmark-name-from-file "/b/Title - Part Two - Some Author.epub") + (should (equal (cj/--reading-bookmark-name-from-file "/b/Title - Part Two - Some Author.epub") "Some Author, Title - Part Two"))) -(ert-deftest test-nov-bookmark-name-no-separator () +(ert-deftest test-reading-bookmark-name-no-separator () "Boundary: a filename with no \" - \" falls back to the cleaned whole name." - (should (equal (cj/--nov-bookmark-name-from-file "/b/Untitled_ Draft.epub") + (should (equal (cj/--reading-bookmark-name-from-file "/b/Untitled_ Draft.epub") "Untitled: Draft"))) -(ert-deftest test-nov-bookmark-name-nil-and-empty () +(ert-deftest test-reading-bookmark-name-nil-and-empty () "Error: nil or empty path yields nil." - (should-not (cj/--nov-bookmark-name-from-file nil)) - (should-not (cj/--nov-bookmark-name-from-file ""))) + (should-not (cj/--reading-bookmark-name-from-file nil)) + (should-not (cj/--reading-bookmark-name-from-file ""))) -;;; cj/--nov-bookmark-rename-record +;;; cj/--reading-bookmark-rename-record -(ert-deftest test-nov-bookmark-rename-record-replaces-name () +(ert-deftest test-reading-bookmark-rename-record-replaces-name () "Normal: the record's name is rebuilt from its filename; the alist is kept." (let* ((record (cons "The A.B.C. Murders - Agatha Christie.epub" '((filename . "/b/The A.B.C. Murders - Agatha Christie.epub") (index . 0)))) - (out (cj/--nov-bookmark-rename-record record))) + (out (cj/--reading-bookmark-rename-record record))) (should (equal (car out) "Agatha Christie, The A.B.C. Murders")) (should (equal (cdr out) (cdr record))))) -(ert-deftest test-nov-bookmark-rename-record-keeps-original-without-filename () +(ert-deftest test-reading-bookmark-rename-record-pdf () + "Normal: a PDF-shaped record (filename from `bookmark-make-record-default') +gets the same \"Author, Title\" rename." + (let* ((record (cons "Engines of Logic_ Mathematicians and the O - Martin Davis.pdf" + '((filename . "/b/Engines of Logic_ Mathematicians and the O - Martin Davis.pdf") + (page . 12)))) + (out (cj/--reading-bookmark-rename-record record))) + (should (equal (car out) "Martin Davis, Engines of Logic: Mathematicians and the O")) + (should (equal (cdr out) (cdr record))))) + +(ert-deftest test-reading-bookmark-rename-record-keeps-original-without-filename () "Boundary: a record with no usable filename is returned unchanged." (let ((record (cons "whatever" '((index . 0))))) - (should (equal (cj/--nov-bookmark-rename-record record) record)))) + (should (equal (cj/--reading-bookmark-rename-record record) record)))) (provide 'test-calibredb-epub-config--bookmark-name) ;;; test-calibredb-epub-config--bookmark-name.el ends here -- cgit v1.2.3