From b7c6b2c59a2ad74e8e886471ea57b2e87f812d4a Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Tue, 12 May 2026 07:17:29 -0500 Subject: fix(nov): center the EPUB text by setting window margins directly The 80% width from `4d9a206' wasn't actually narrowing the page: `cj/nov-apply-preferences' set `nov-text-width' to t (nov renders the text unfilled, one long line per paragraph) and counted on `visual-fill-column-mode' to set the window's display margins, but those margins never got applied in nov-mode buffers (even after manually re-running the layout), so the text wrapped at the full window width. The cause is still unknown. This drops `visual-fill-column' from nov entirely: - `nov-text-width' is a column count (~80% of the window's natural width), so nov's `shr' fills the text itself. - `cj/nov-update-layout' sets the window's left/right margins directly to `(natural - text-width) / 2' each, centering the block, and pushes the fringes out to the window edge so they don't show as thin lines beside the text. When the width changes it re-renders, restoring the reading position approximately. - `cj/nov-apply-preferences' adds a `kill-buffer-hook' that drops the margins and fringes when the EPUB buffer goes away, so a later buffer in that window isn't left indented. - `+'/`=' and `-'/`_' adjust `cj/nov-margin-percent' and re-flow + re-center. The text-width math moved into a `cj/nov--natural-window-width' helper alongside the existing `cj/nov--text-width'. Known nit: the centering is a touch left-of-center because shr wraps at word boundaries, so the rendered text is a bit narrower than `nov-text-width' and the right margin ends up slightly larger. Logged as a follow-up. --- tests/test-calibredb-epub-config.el | 61 ++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/test-calibredb-epub-config.el b/tests/test-calibredb-epub-config.el index e12abdaa..b7b9ccaf 100644 --- a/tests/test-calibredb-epub-config.el +++ b/tests/test-calibredb-epub-config.el @@ -20,6 +20,21 @@ (declare-function cj/nov--text-width "calibredb-epub-config" (total-cols)) +(defmacro test-calibredb-epub--in-nov-buffer (&rest body) + "Run BODY in a temp buffer faking `nov-mode' and a 200-column window. +`get-buffer-window' / `window-body-width' / `window-margins' / +`set-window-margins' / `set-window-fringes' are stubbed; BODY must stub +`nov-render-document' before anything that reaches it." + (declare (indent 0)) + `(with-temp-buffer + (setq-local major-mode 'nov-mode) + (cl-letf (((symbol-function 'get-buffer-window) (lambda (&rest _) 'win)) + ((symbol-function 'window-body-width) (lambda (_) 200)) + ((symbol-function 'window-margins) (lambda (_) '(nil . nil))) + ((symbol-function 'set-window-margins) (lambda (&rest _) nil)) + ((symbol-function 'set-window-fringes) (lambda (&rest _) nil))) + ,@body))) + ;;; ----------------------------- cj/nov--text-width --------------------------- (ert-deftest test-calibredb-epub-nov-text-width-applies-margin () @@ -87,6 +102,42 @@ this, every layout pass would shave the column by another margin fraction." "Normal: `cj/nov-update-layout' can be invoked with `M-x'." (should (commandp #'cj/nov-update-layout))) +(ert-deftest test-calibredb-epub-nov-update-layout-reflows-when-width-changes () + "Normal: a changed text width updates `nov-text-width' and re-renders. +nov fills the text to `nov-text-width' itself, so a width change requires a +re-render of the document." + (let ((cj/nov-margin-percent 10) + rendered) + (test-calibredb-epub--in-nov-buffer + (setq-local nov-text-width 50) + (cl-letf (((symbol-function 'nov-render-document) (lambda () (setq rendered t)))) + (cj/nov-update-layout)) + (should (= 160 nov-text-width)) ; 80% of the 200-column window + (should rendered)))) + +(ert-deftest test-calibredb-epub-nov-update-layout-skips-reflow-when-width-unchanged () + "Boundary: when the width is already current, do not re-render the document." + (let ((cj/nov-margin-percent 10) + rendered) + (test-calibredb-epub--in-nov-buffer + (setq-local nov-text-width 160) ; already 80% of 200 + (cl-letf (((symbol-function 'nov-render-document) (lambda () (setq rendered t)))) + (cj/nov-update-layout)) + (should (= 160 nov-text-width)) + (should-not rendered)))) + +(ert-deftest test-calibredb-epub-nov-update-layout-centers-with-equal-margins () + "Normal: the text block is centered with equal left/right window margins." + (let ((cj/nov-margin-percent 10) + margins) + (test-calibredb-epub--in-nov-buffer + (cl-letf (((symbol-function 'nov-render-document) #'ignore) + ((symbol-function 'set-window-margins) + (lambda (_win l r) (setq margins (list l r))))) + (cj/nov-update-layout)) + ;; (200 - 160) / 2 = 20 columns each side + (should (equal margins '(20 20)))))) + ;;; --------------------- cj/nov-widen-text / cj/nov-narrow-text --------------- (ert-deftest test-calibredb-epub-nov-adjust-margin-steps-and-clamps () @@ -133,13 +184,21 @@ this, every layout pass would shave the column by another margin fraction." (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." +lands at the width 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))))) +(ert-deftest test-calibredb-epub-nov-apply-preferences-sets-integer-text-width () + "Normal: applying preferences sets `nov-text-width' to a column count, not t, +so nov's `shr' fills the text itself rather than relying on visual-fill-column." + (cl-letf (((symbol-function 'nov-render-document) #'ignore)) + (with-temp-buffer + (cj/nov-apply-preferences) + (should (integerp nov-text-width))))) + ;;; ----------------------------- cj/nov-open-external ------------------------- (ert-deftest test-calibredb-epub-open-external-uses-zathura () -- cgit v1.2.3