aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-12 06:07:20 -0500
committerCraig Jennings <c@cjennings.net>2026-05-12 06:07:20 -0500
commit1c5c8bd4df3bd4fd71fad13b2b57e670a4e74355 (patch)
tree4637ff8bb00cdaca337b8a31f3741bfee67f6fe3
parenta032f45fe30ce300163bc052257c3c5c993c85d5 (diff)
downloaddotemacs-1c5c8bd4df3bd4fd71fad13b2b57e670a4e74355.tar.gz
dotemacs-1c5c8bd4df3bd4fd71fad13b2b57e670a4e74355.zip
fix(nov): rework the EPUB reading-width layout
`cj/nov--text-width-for-window' computed the target column as a percentage of `(window-body-width)'. But body width is the column count *after* the display margins. `cj/nov-update-layout' runs from `window-configuration-change-hook': it sets `visual-fill-column''s margins, which changes the body width, which fires the hook, which re-runs the layout against the now-narrower body, and so on. It's a shrinking feedback loop that bottoms out at `cj/nov-min-text-width' (40 columns) no matter what `cj/nov-margin-percent' is. That's why the column was a thin strip regardless of the margin setting. The width is now computed from the window's *natural* column count (body width plus any margins already set), so re-running the layout is idempotent. The margin math moved into a pure `cj/nov--text-width' helper, which is what the unit tests drive, and there's a regression test that the result is the same whether or not margins are already in place. Also: - `+'/`=' (`cj/nov-widen-text') and `-'/`_' (`cj/nov-narrow-text') step `cj/nov-margin-percent' by `cj/nov-margin-step' and re-lay-out, reporting the new percentage. `cj/nov-margin-percent' is now clamped to 0..25, so the text column runs from 50% (the floor) to 100% (the full window). - `cj/nov-margin-percent' default is 12 (≈76% text) for a comfortable starting width. - `cj/nov-apply-preferences' re-renders the document at the end again. `b3b537f' removed that on the theory `visual-fill-column' would re-trigger the render. The first page came up off-center until a manual resize, so it's back. - `cj/nov-update-layout' is now a command. The visible result (a ~75% centered column on first open, `+`/`-` to adjust) needs a restart to confirm. The tests cover the width math and clamping, idempotency, the adjust commands and their keybindings, the command status, and the re-render.
-rw-r--r--modules/calibredb-epub-config.el75
-rw-r--r--tests/test-calibredb-epub-config.el147
2 files changed, 180 insertions, 42 deletions
diff --git a/modules/calibredb-epub-config.el b/modules/calibredb-epub-config.el
index 43032381..2e02d1a9 100644
--- a/modules/calibredb-epub-config.el
+++ b/modules/calibredb-epub-config.el
@@ -49,6 +49,7 @@
(declare-function cj/open-file-with-command "system-utils" (command))
(declare-function visual-fill-column-mode "visual-fill-column" (&optional arg))
(declare-function visual-fill-column--adjust-window "visual-fill-column" ())
+(declare-function nov-render-document "nov" ())
;; -------------------------- CalibreDB Ebook Manager --------------------------
@@ -89,13 +90,18 @@
;; ------------------------------ Nov Epub Reader ------------------------------
-(defvar cj/nov-margin-percent 25
- "Percentage of window width to use as margins on each side when reading epubs.
-For example, 25 means 25% left margin + 25% right margin, with 50% for text.")
+(defvar cj/nov-margin-percent 12
+ "Percent of the window's natural width used as a margin on each side in epubs.
+12 leaves about 76% of the columns for text. Clamped to 0..25, so the text
+column runs from 50% (margin 25) to 100% (margin 0) of the window.
+Adjust it live with `cj/nov-widen-text' and `cj/nov-narrow-text'.")
(defvar cj/nov-min-text-width 40
"Minimum text width in columns for Nov reading buffers.")
+(defvar cj/nov-margin-step 2
+ "Percentage points each `cj/nov-widen-text'/`cj/nov-narrow-text' press changes.")
+
;; Prevent magic-fallback-mode-alist from opening epub as archive-mode
;; Advise set-auto-mode to force nov-mode for .epub files before magic-fallback runs
(defun cj/force-nov-mode-for-epub (orig-fun &rest args)
@@ -127,21 +133,35 @@ For example, 25 means 25% left margin + 25% right margin, with 50% for text.")
(forward-paragraph)
(recenter))
+(defun cj/nov--text-width (total-cols)
+ "Return the Nov text-column width for TOTAL-COLS of usable window width.
+`cj/nov-margin-percent' is clamped to 0..25 and taken off each side; the
+result is at least `cj/nov-min-text-width'."
+ (let* ((margin-percent (max 0 (min 25 cj/nov-margin-percent)))
+ (text-width-ratio (- 1.0 (* 2 (/ margin-percent 100.0)))))
+ (max cj/nov-min-text-width
+ (floor (* text-width-ratio total-cols)))))
+
(defun cj/nov--text-width-for-window (&optional window)
"Return preferred Nov text width for WINDOW.
-The width uses `cj/nov-margin-percent' while keeping a readable minimum and
-clamping excessive margin percentages."
+Computed from the window's natural column count -- its current body width
+plus any margins already set -- so that re-running `cj/nov-update-layout' is
+idempotent: each pass would otherwise shave the column by another margin
+fraction, since setting margins narrows the body width."
(let* ((window (or window (get-buffer-window (current-buffer) t)))
- (window-width (if window (window-body-width window) 80))
- (margin-percent (max 0 (min 45 cj/nov-margin-percent)))
- (text-width-ratio (- 1.0 (* 2 (/ margin-percent 100.0)))))
- (max cj/nov-min-text-width
- (floor (* text-width-ratio window-width)))))
+ (margins (and window (window-margins window)))
+ (natural-cols (if window
+ (+ (window-body-width window)
+ (or (car margins) 0)
+ (or (cdr margins) 0))
+ 80)))
+ (cj/nov--text-width natural-cols)))
(defun cj/nov-update-layout (&optional _frame)
- "Recalculate Nov text layout for the current buffer.
-Suitable for `window-configuration-change-hook' or
-`window-size-change-functions'."
+ "Recalculate Nov's centered text column (width + margins) for this buffer.
+Also runs from `window-configuration-change-hook' and
+`window-size-change-functions' to stay responsive to splits and resizes."
+ (interactive)
(when (derived-mode-p 'nov-mode)
(when (require 'visual-fill-column nil t)
(setq-local visual-fill-column-center-text t)
@@ -150,6 +170,25 @@ Suitable for `window-configuration-change-hook' or
(when (bound-and-true-p visual-fill-column-mode)
(visual-fill-column--adjust-window)))))
+(defun cj/--nov-adjust-margin (delta)
+ "Add DELTA to `cj/nov-margin-percent' (clamped 0..25), re-lay-out, and report.
+A positive DELTA narrows the text column; a negative DELTA widens it."
+ (setq cj/nov-margin-percent
+ (max 0 (min 25 (+ cj/nov-margin-percent delta))))
+ (cj/nov-update-layout)
+ (message "EPUB text width: %d%% of the window (margin %d%% each side)"
+ (- 100 (* 2 cj/nov-margin-percent)) cj/nov-margin-percent))
+
+(defun cj/nov-widen-text ()
+ "Give the EPUB text column more of the window, up to the full width."
+ (interactive)
+ (cj/--nov-adjust-margin (- cj/nov-margin-step)))
+
+(defun cj/nov-narrow-text ()
+ "Give the EPUB text column less of the window, down to 50%."
+ (interactive)
+ (cj/--nov-adjust-margin cj/nov-margin-step))
+
(defun cj/nov-apply-preferences ()
"Apply preferences after nov-mode has launched."
(interactive)
@@ -168,7 +207,10 @@ Suitable for `window-configuration-change-hook' or
;; Enable visual-fill-column for centered text with margins
(cj/nov-update-layout)
;; Keep centered text width responsive after splits/resizes.
- (add-hook 'window-configuration-change-hook #'cj/nov-update-layout nil t))
+ (add-hook 'window-configuration-change-hook #'cj/nov-update-layout nil t)
+ ;; Re-render so the first page is laid out inside the margins just set;
+ ;; nov-mode's initial render ran before `nov-text-width'/visual-fill-column.
+ (nov-render-document))
(defun cj/nov-open-external ()
"Open the current EPUB with zathura."
@@ -191,6 +233,11 @@ Suitable for `window-configuration-change-hook' or
("<" . nov-history-back)
(">" . nov-history-forward)
("," . backward-paragraph)
+ ;; +/= widen the text column, -/_ narrow it (50%..100% of the window)
+ ("+" . cj/nov-widen-text)
+ ("=" . cj/nov-widen-text)
+ ("-" . cj/nov-narrow-text)
+ ("_" . cj/nov-narrow-text)
;; open current EPUB with zathura (same key in pdf-view)
("z" . cj/nov-open-external)
("t" . nov-goto-toc)
diff --git a/tests/test-calibredb-epub-config.el b/tests/test-calibredb-epub-config.el
index 53d2e78f..88c4452f 100644
--- a/tests/test-calibredb-epub-config.el
+++ b/tests/test-calibredb-epub-config.el
@@ -2,54 +2,145 @@
;;; Commentary:
;; Focuses on project-owned helpers in calibredb-epub-config rather than
-;; CalibreDB/Nov internals.
+;; CalibreDB/Nov internals. The Nov layout helpers get the most attention:
+;; the text-width math, the idempotency of `cj/nov-update-layout' (it must not
+;; shrink the column each time it runs), and the cold-open re-render.
;;; Code:
(require 'ert)
(require 'cl-lib)
+(require 'package)
+(setq package-user-dir (expand-file-name "elpa" user-emacs-directory))
+(package-initialize)
(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
(require 'calibredb-epub-config)
+(require 'nov nil t) ; for the nov-mode-map keybinding test; harmless if absent
-(ert-deftest test-calibredb-epub-nov-text-width-default-window ()
- "Normal: text width uses the configured margins against the current window."
+(declare-function cj/nov--text-width "calibredb-epub-config" (total-cols))
+
+;;; ----------------------------- cj/nov--text-width ---------------------------
+
+(ert-deftest test-calibredb-epub-nov-text-width-applies-margin ()
+ "Normal: 25% margins leave 50% of the usable columns for text."
(let ((cj/nov-margin-percent 25)
(cj/nov-min-text-width 40))
- (cl-letf (((symbol-function 'get-buffer-window)
- (lambda (&rest _) 'window))
- ((symbol-function 'window-body-width)
- (lambda (_) 120)))
- (should (= 60 (cj/nov--text-width-for-window))))))
+ (should (= 60 (cj/nov--text-width 120)))))
(ert-deftest test-calibredb-epub-nov-text-width-clamps-large-margin ()
- "Boundary: excessive margins are clamped to keep a readable text column."
+ "Boundary: a margin percent above 25 is clamped to 25, so text never drops
+below 50% of the usable columns."
(let ((cj/nov-margin-percent 80)
(cj/nov-min-text-width 40))
- (cl-letf (((symbol-function 'get-buffer-window)
- (lambda (&rest _) 'window))
- ((symbol-function 'window-body-width)
- (lambda (_) 120)))
- (should (= 40 (cj/nov--text-width-for-window))))))
+ (should (= 60 (cj/nov--text-width 120)))))
-(ert-deftest test-calibredb-epub-nov-text-width-fallback-without-window ()
- "Boundary: a buffer without a visible window still gets a usable width."
+(ert-deftest test-calibredb-epub-nov-text-width-clamps-negative-margin ()
+ "Boundary: a negative margin percent is clamped up to 0 (text takes everything)."
+ (let ((cj/nov-margin-percent -10)
+ (cj/nov-min-text-width 40))
+ (should (= 120 (cj/nov--text-width 120)))))
+
+(ert-deftest test-calibredb-epub-nov-text-width-honours-minimum ()
+ "Boundary: a narrow window still yields at least `cj/nov-min-text-width'."
(let ((cj/nov-margin-percent 25)
(cj/nov-min-text-width 40))
- (cl-letf (((symbol-function 'get-buffer-window)
- (lambda (&rest _) nil)))
- (should (= 40 (cj/nov--text-width-for-window))))))
+ (should (= 40 (cj/nov--text-width 50)))))
-(ert-deftest test-calibredb-epub-nov-text-width-clamps-negative-margin ()
- "Boundary: a negative margin percent is clamped up to 0, so the text takes
-the full window width."
- (let ((cj/nov-margin-percent -10)
+(ert-deftest test-calibredb-epub-nov-default-margin-gives-roughly-three-quarter-text ()
+ "Normal: the default `cj/nov-margin-percent' leaves ~3/4 of the window for text."
+ (should (= 76 (cj/nov--text-width 100))))
+
+;;; ----------------------- cj/nov--text-width-for-window ----------------------
+
+(ert-deftest test-calibredb-epub-nov-text-width-for-window-fresh ()
+ "Normal: with no margins set yet, the natural width is the body width."
+ (let ((cj/nov-margin-percent 25)
+ (cj/nov-min-text-width 40))
+ (cl-letf (((symbol-function 'get-buffer-window) (lambda (&rest _) 'win))
+ ((symbol-function 'window-body-width) (lambda (_) 120))
+ ((symbol-function 'window-margins) (lambda (_) '(nil . nil))))
+ (should (= 60 (cj/nov--text-width-for-window))))))
+
+(ert-deftest test-calibredb-epub-nov-text-width-for-window-idempotent ()
+ "Boundary: re-running with margins already set returns the same width.
+The body width is now narrower because margins were applied, but the natural
+width (body + margins) is unchanged, so the column does not shrink. Without
+this, every layout pass would shave the column by another margin fraction."
+ (let ((cj/nov-margin-percent 25)
+ (cj/nov-min-text-width 40))
+ (cl-letf (((symbol-function 'get-buffer-window) (lambda (&rest _) 'win))
+ ((symbol-function 'window-body-width) (lambda (_) 60))
+ ((symbol-function 'window-margins) (lambda (_) '(30 . 30))))
+ (should (= 60 (cj/nov--text-width-for-window))))))
+
+(ert-deftest test-calibredb-epub-nov-text-width-for-window-no-window ()
+ "Boundary: a buffer with no visible window still gets a usable width."
+ (let ((cj/nov-margin-percent 25)
(cj/nov-min-text-width 40))
- (cl-letf (((symbol-function 'get-buffer-window)
- (lambda (&rest _) 'window))
- ((symbol-function 'window-body-width)
- (lambda (_) 120)))
- (should (= 120 (cj/nov--text-width-for-window))))))
+ (cl-letf (((symbol-function 'get-buffer-window) (lambda (&rest _) nil)))
+ (should (= 40 (cj/nov--text-width-for-window))))))
+
+;;; ---------------------------- cj/nov-update-layout --------------------------
+
+(ert-deftest test-calibredb-epub-nov-update-layout-is-a-command ()
+ "Normal: `cj/nov-update-layout' can be invoked with `M-x'."
+ (should (commandp #'cj/nov-update-layout)))
+
+;;; --------------------- cj/nov-widen-text / cj/nov-narrow-text ---------------
+
+(ert-deftest test-calibredb-epub-nov-adjust-margin-steps-and-clamps ()
+ "Normal/Boundary: adjusting the margin moves by DELTA, clamped to 0..25."
+ (cl-letf (((symbol-function 'message) (lambda (&rest _) nil)))
+ (let ((cj/nov-margin-percent 12))
+ (cj/--nov-adjust-margin -2)
+ (should (= 10 cj/nov-margin-percent))
+ (cj/--nov-adjust-margin 100)
+ (should (= 25 cj/nov-margin-percent)) ; 50%-text floor
+ (cj/--nov-adjust-margin -100)
+ (should (= 0 cj/nov-margin-percent))))) ; 100%-text ceiling
+
+(ert-deftest test-calibredb-epub-nov-widen-text-decreases-margin ()
+ "Normal: `cj/nov-widen-text' gives the column more of the window."
+ (cl-letf (((symbol-function 'message) (lambda (&rest _) nil)))
+ (let ((cj/nov-margin-percent 12)
+ (cj/nov-margin-step 2))
+ (cj/nov-widen-text)
+ (should (= 10 cj/nov-margin-percent)))))
+
+(ert-deftest test-calibredb-epub-nov-narrow-text-increases-margin ()
+ "Normal: `cj/nov-narrow-text' gives the column less of the window."
+ (cl-letf (((symbol-function 'message) (lambda (&rest _) nil)))
+ (let ((cj/nov-margin-percent 12)
+ (cj/nov-margin-step 2))
+ (cj/nov-narrow-text)
+ (should (= 14 cj/nov-margin-percent)))))
+
+(ert-deftest test-calibredb-epub-nov-width-commands-are-commands ()
+ "Normal: the width-adjust commands are `M-x'-able."
+ (should (commandp #'cj/nov-widen-text))
+ (should (commandp #'cj/nov-narrow-text)))
+
+(ert-deftest test-calibredb-epub-nov-width-commands-bound-in-nov-mode-map ()
+ "Normal: +/= widen and -/_ narrow the text column in `nov-mode-map'."
+ (skip-unless (and (require 'nov nil t) (boundp 'nov-mode-map)))
+ (should (eq (keymap-lookup nov-mode-map "+") #'cj/nov-widen-text))
+ (should (eq (keymap-lookup nov-mode-map "=") #'cj/nov-widen-text))
+ (should (eq (keymap-lookup nov-mode-map "-") #'cj/nov-narrow-text))
+ (should (eq (keymap-lookup nov-mode-map "_") #'cj/nov-narrow-text)))
+
+;;; -------------------------- cj/nov-apply-preferences ------------------------
+
+(ert-deftest test-calibredb-epub-nov-apply-preferences-rerenders-document ()
+ "Normal: applying preferences re-renders the document so the first page
+lands inside the margins it just configured."
+ (let (rendered)
+ (cl-letf (((symbol-function 'nov-render-document) (lambda () (setq rendered t))))
+ (with-temp-buffer
+ (cj/nov-apply-preferences)
+ (should rendered)))))
+
+;;; ----------------------------- cj/nov-open-external -------------------------
(ert-deftest test-calibredb-epub-open-external-uses-zathura ()
"Normal: named Nov external-open command delegates to zathura."