bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]


Following my patch from a few months ago which added the
`doc-view-mutool-user-stylesheet' defcustom to Doc-View making use of
mutools ability to apply a user specified CSS file when converting a
EPUB file.  Here is a patch to reconvert existing EPUB buffers when the
value of that variable (renamed to `doc-view-epub-user-stylesheet' as
discussed after my last patch) is changed, or the specified file is
changed, therefore applying the new CSS immediately.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Reconvert EPUB buffers whenn CSS is changed --]
[-- Type: text/x-patch, Size: 4431 bytes --]

From f7e243d02a8f19f4c433a8e9590da8364ca7f5cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kjartan=20=C3=93li=20=C3=81g=C3=BAstsson?=
 <kjartanoli@outlook.com>
Date: Mon, 6 Jun 2022 18:34:05 +0000
Subject: [PATCH] Reconvert EPUB buffers when user CSS is changed

* lisp/doc-view.el (doc-view-epub-user-stylesheet): Add
`doc-view-custom-set-mutool-user-stylesheet' as setter, change name.
(doc-view-custom-set-epub-font-size): Factor reconvert logic out
(doc-view--epub-reconvert): Add defun
(doc-view--epub-stylesheet-watcher): Add defvar
(doc-view-custom-set-epub-user-stylesheet): Add defun
---
 lisp/doc-view.el | 58 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 9d27347360..fd0ee4bbb5 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -144,6 +144,7 @@
 (require 'dired)
 (require 'image-mode)
 (require 'jka-compr)
+(require 'filenotify)
 (eval-when-compile (require 'subr-x))
 
 ;;;; Customization Options
@@ -226,17 +227,49 @@ doc-view-resolution
 Higher values result in larger images."
   :type 'number)
 
-(defcustom doc-view-mutool-user-stylesheet nil
-  "User stylesheet to use when converting EPUB documents to PDF."
-  :type '(choice (const nil)
-                 (file :must-match t))
-  :version "29.1")
-
 (defvar doc-view-doc-type nil
   "The type of document in the current buffer.
 Can be `dvi', `pdf', `ps', `djvu', `odf', `epub', `cbz', `fb2',
 `xps' or `oxps'.")
 
+(defvar doc-view--epub-stylesheet-watcher nil
+  "File watcher for `doc-view-epub-user-stylesheet'.")
+
+(defun doc-view--epub-reconvert (&optional event)
+  "Reconvert all epub buffers.
+
+EVENT is unused, but neccesary to work with the filenotify API"
+  (dolist (x (buffer-list))
+    (with-current-buffer x
+      (when (eq doc-view-doc-type 'epub)
+        (doc-view-reconvert-doc)))))
+
+(defun doc-view-custom-set-epub-user-stylesheet (option-name new-value)
+  "Setter for `doc-view-epub-user-stylesheet'.
+
+Reconverts existing epub buffers when the file used as a user
+stylesheet is switched."
+  (set-default option-name new-value)
+  (file-notify-rm-watch doc-view--epub-stylesheet-watcher)
+  (doc-view--epub-reconvert)
+  (setq doc-view--epub-stylesheet-watcher
+         (when doc-view-epub-user-stylesheet
+           (file-notify-add-watch doc-view-epub-user-stylesheet '(change) #'doc-view--epub-reconvert))))
+
+(defcustom doc-view-epub-user-stylesheet nil
+  "User stylesheet to use when converting EPUB documents to PDF."
+  :type '(choice (const nil)
+                 (file :must-match t))
+  :version "29.1"
+  :set #'doc-view-custom-set-epub-user-stylesheet)
+
+(defvar-local doc-view--current-cache-dir nil
+  "Only used internally.")
+
+(defun doc-view-custom-set-epub-font-size (option-name new-value)
+  (set-default option-name new-value)
+  (doc-view--epub-reconvert))
+
 ;; FIXME: The doc-view-current-* definitions below are macros because they
 ;; map to accessors which we want to use via `setf' as well!
 (defmacro doc-view-current-page (&optional win)
@@ -249,15 +282,6 @@ doc-view-current-slice
 (defvar-local doc-view--current-cache-dir nil
   "Only used internally.")
 
-(defun doc-view-custom-set-epub-font-size (option-name new-value)
-  (set-default option-name new-value)
-  (dolist (x (buffer-list))
-    (with-current-buffer x
-      (when (eq doc-view-doc-type 'epub)
-        (delete-directory doc-view--current-cache-dir t)
-        (doc-view-initiate-display)
-        (doc-view-goto-page (doc-view-current-page))))))
-
 (defcustom doc-view-epub-font-size nil
   "Font size in points for EPUB layout."
   :type '(choice (const nil) integer)
@@ -1178,12 +1202,12 @@ doc-view-pdf->png-converter-mupdf
       (when doc-view-epub-font-size
         (setq options (append options
                               (list (format "-S%s" doc-view-epub-font-size)))))
-      (when doc-view-mutool-user-stylesheet
+      (when doc-view-epub-user-stylesheet
         (setq options
               (append options
                       (list (format "-U%s"
                                     (expand-file-name
-                                     doc-view-mutool-user-stylesheet)))))))
+                                     doc-view-epub-user-stylesheet)))))))
     (doc-view-start-process
      "pdf->png" doc-view-pdfdraw-program
      `(,@(doc-view-pdfdraw-program-subcommand)
-- 
2.36.1


[-- Attachment #3: Type: text/plain, Size: 108 bytes --]


-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Lars Ingebrigtsen]

Kjartan Oli Agustsson <kjartanoli@outlook.com> writes:

> Following my patch from a few months ago which added the
> `doc-view-mutool-user-stylesheet' defcustom to Doc-View making use of
> mutools ability to apply a user specified CSS file when converting a
> EPUB file.  Here is a patch to reconvert existing EPUB buffers when the
> value of that variable (renamed to `doc-view-epub-user-stylesheet' as
> discussed after my last patch) is changed, or the specified file is
> changed, therefore applying the new CSS immediately.

Did you mean to remove the doc-view-mutool-user-stylesheet user option
instead of renaming it?

In any case, the patch has the following warnings:

In doc-view--epub-reconvert:
doc-view.el:238:44: Warning: Unused lexical argument `event'
In doc-view-custom-set-epub-user-stylesheet:
doc-view.el:256:16: Warning: reference to free variable `doc-view-epub-user-stylesheet'


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Eli Zaretskii]

> From: Kjartan Oli Agustsson <kjartanoli@outlook.com>
> Date: Mon, 06 Jun 2022 19:20:04 +0000
> 
> +(defun doc-view-custom-set-epub-user-stylesheet (option-name new-value)
> +  "Setter for `doc-view-epub-user-stylesheet'.
> +
> +Reconverts existing epub buffers when the file used as a user
> +stylesheet is switched."
> +  (set-default option-name new-value)
> +  (file-notify-rm-watch doc-view--epub-stylesheet-watcher)
> +  (doc-view--epub-reconvert)
> +  (setq doc-view--epub-stylesheet-watcher
> +         (when doc-view-epub-user-stylesheet
> +           (file-notify-add-watch doc-view-epub-user-stylesheet '(change) #'doc-view--epub-reconvert))))

Are you sure 'change is TRT here?  What kinds of changes did you want
to watch in this case (it isn't clear from the doc string or the log
message)?  'change is a synthetic event, and it means slightly
different things with each file-notification back-end, so I wonder
whether 'change is portable enough here?

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Did you mean to remove the doc-view-mutool-user-stylesheet user option
> instead of renaming it?

I did rename it, I believe I also moved it a few lines down as well to
make sure some other variable/function was defined first (It's been a
while since I wrote that part of the patch, I can't remember the exact
reason).

> In any case, the patch has the following warnings:
>
> In doc-view--epub-reconvert:
> doc-view.el:238:44: Warning: Unused lexical argument `event'

Yes, I'm not sure what the best way to get rid of this one is.  If
`doc-view--epub-reconvert' doesn't take `event' as an argument it will
cause a warning about being called with to many arguments when used as
the callback for the file watcher set up in
`doc-view-custom-set-epub-user-stylesheet'

> In doc-view-custom-set-epub-user-stylesheet:
> doc-view.el:256:16: Warning: reference to free variable `doc-view-epub-user-stylesheet'

If I recall correctly then defining the variable before the function
would cause that definition to either emit a warning, or error, because
the function (which is passed as the `:set' argument to defcustom)
hasn't been defined when said defcustom is evaluated.  Any advice on how
to get rid of either of these warnings would be greatly appreciated.

-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 691 bytes --]

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> Are you sure 'change is TRT here?  What kinds of changes did you want
> to watch in this case

I'm trying to detect a change in the contents of the file, which would
probably mean that the CSS rules have been modified.

>(it isn't clear from the doc string or the log message)

Yes, I see now that I forgot to add a mention of this in the docstring
when I added the file watcher, I will rectify this.

> 'change is a synthetic event, and it means slightly different things
> with each file-notification back-end, so I wonder whether 'change is
> portable enough here?

From what I could see from the docstring of `file-notify-add-watch'
'change seemed the most appropriate, but if you think something else
would be better I will gladly modify the patch to use that instead.

-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 691 bytes --]

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Eli Zaretskii]

> From: Kjartan Oli Agustsson <kjartanoli@outlook.com>
> Cc: 55825@debbugs.gnu.org
> Date: Tue, 07 Jun 2022 21:51:46 +0000
> 
> > 'change is a synthetic event, and it means slightly different things
> > with each file-notification back-end, so I wonder whether 'change is
> > portable enough here?
> 
> From what I could see from the docstring of `file-notify-add-watch'
> 'change seemed the most appropriate, but if you think something else
> would be better I will gladly modify the patch to use that instead.

Did you try 'attribute-change?

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Eli Zaretskii]

> Cc: 55825@debbugs.gnu.org
> From: Kjartan Oli Agustsson <kjartanoli@outlook.com>
> Date: Tue, 07 Jun 2022 21:40:09 +0000
> 
> > In doc-view--epub-reconvert:
> > doc-view.el:238:44: Warning: Unused lexical argument `event'
> 
> Yes, I'm not sure what the best way to get rid of this one is.  If
> `doc-view--epub-reconvert' doesn't take `event' as an argument it will
> cause a warning about being called with to many arguments when used as
> the callback for the file watcher set up in
> `doc-view-custom-set-epub-user-stylesheet'

Use _event instead, it should take care of that.

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> Did you try 'attribute-change?

No, but it should catch the changes I'm trying to watch for so I'll try that.

-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 691 bytes --]

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]


Kjartan Oli Agustsson <kjartanoli@outlook.com> writes:

> No, but it should catch the changes I'm trying to watch for so I'll try that.

I just finished a basic test of using 'attribute-change, for some reason
it does not seem to detect when I change the contents of the specified
CSS file and save the buffer.

-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 691 bytes --]

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Eli Zaretskii]

> From: Kjartan Oli Agustsson <kjartanoli@outlook.com>
> Cc: 55825@debbugs.gnu.org
> Date: Wed, 08 Jun 2022 23:28:08 +0000
> 
> 
> [1:text/plain Hide]
> 
> 
> Kjartan Oli Agustsson <kjartanoli@outlook.com> writes:
> 
> > No, but it should catch the changes I'm trying to watch for so I'll try that.
> 
> I just finished a basic test of using 'attribute-change, for some reason
> it does not seem to detect when I change the contents of the specified
> CSS file and save the buffer.

Thanks, then I guess 'change it is.

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Kjartan Oli Agustsson]

[-- Attachment #1.1: Type: text/plain, Size: 215 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, then I guess 'change it is.

Having finally found the time to work on this again, I believe the
attached patch should fix all the issues identified by you and Lars.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: revised patch --]
[-- Type: text/x-patch, Size: 4536 bytes --]

From 32cc2061f9cdecc3bbd748097d2b7c8e64cff17a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kjartan=20=C3=93li=20=C3=81g=C3=BAstsson?=
 <kjartanoli@outlook.com>
Date: Mon, 6 Jun 2022 18:34:05 +0000
Subject: [PATCH] Reconvert EPUB buffers when user CSS is changed

* lisp/doc-view.el (doc-view-epub-user-stylesheet): Add
`doc-view-custom-set-mutool-user-stylesheet' as setter, change name.
(doc-view-custom-set-epub-font-size): Factor reconvert logic out
(doc-view--epub-reconvert): Add defun
(doc-view--epub-stylesheet-watcher): Add defvar
(doc-view-custom-set-epub-user-stylesheet): Add defun
---
 lisp/doc-view.el | 58 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 9d27347360..2bad50db79 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -144,6 +144,7 @@
 (require 'dired)
 (require 'image-mode)
 (require 'jka-compr)
+(require 'filenotify)
 (eval-when-compile (require 'subr-x))
 
 ;;;; Customization Options
@@ -226,17 +227,49 @@ doc-view-resolution
 Higher values result in larger images."
   :type 'number)
 
-(defcustom doc-view-mutool-user-stylesheet nil
-  "User stylesheet to use when converting EPUB documents to PDF."
-  :type '(choice (const nil)
-                 (file :must-match t))
-  :version "29.1")
-
 (defvar doc-view-doc-type nil
   "The type of document in the current buffer.
 Can be `dvi', `pdf', `ps', `djvu', `odf', `epub', `cbz', `fb2',
 `xps' or `oxps'.")
 
+(defvar doc-view--epub-stylesheet-watcher nil
+  "File watcher for `doc-view-epub-user-stylesheet'.")
+
+(defun doc-view--epub-reconvert (&optional _event)
+  "Reconvert all epub buffers.
+
+EVENT is unused, but neccesary to work with the filenotify API"
+  (dolist (x (buffer-list))
+    (with-current-buffer x
+      (when (eq doc-view-doc-type 'epub)
+        (doc-view-reconvert-doc)))))
+
+(defun doc-view-custom-set-epub-user-stylesheet (option-name new-value)
+  "Setter for `doc-view-epub-user-stylesheet'.
+
+Reconverts existing epub buffers when the file used as a user
+stylesheet is switched, or its contents modified."
+  (set-default option-name new-value)
+  (file-notify-rm-watch doc-view--epub-stylesheet-watcher)
+  (doc-view--epub-reconvert)
+  (setq doc-view--epub-stylesheet-watcher
+         (when new-value
+           (file-notify-add-watch new-value '(change) #'doc-view--epub-reconvert))))
+
+(defcustom doc-view-epub-user-stylesheet nil
+  "User stylesheet to use when converting EPUB documents to PDF."
+  :type '(choice (const nil)
+                 (file :must-match t))
+  :version "29.1"
+  :set #'doc-view-custom-set-epub-user-stylesheet)
+
+(defvar-local doc-view--current-cache-dir nil
+  "Only used internally.")
+
+(defun doc-view-custom-set-epub-font-size (option-name new-value)
+  (set-default option-name new-value)
+  (doc-view--epub-reconvert))
+
 ;; FIXME: The doc-view-current-* definitions below are macros because they
 ;; map to accessors which we want to use via `setf' as well!
 (defmacro doc-view-current-page (&optional win)
@@ -249,15 +282,6 @@ doc-view-current-slice
 (defvar-local doc-view--current-cache-dir nil
   "Only used internally.")
 
-(defun doc-view-custom-set-epub-font-size (option-name new-value)
-  (set-default option-name new-value)
-  (dolist (x (buffer-list))
-    (with-current-buffer x
-      (when (eq doc-view-doc-type 'epub)
-        (delete-directory doc-view--current-cache-dir t)
-        (doc-view-initiate-display)
-        (doc-view-goto-page (doc-view-current-page))))))
-
 (defcustom doc-view-epub-font-size nil
   "Font size in points for EPUB layout."
   :type '(choice (const nil) integer)
@@ -1178,12 +1202,12 @@ doc-view-pdf->png-converter-mupdf
       (when doc-view-epub-font-size
         (setq options (append options
                               (list (format "-S%s" doc-view-epub-font-size)))))
-      (when doc-view-mutool-user-stylesheet
+      (when doc-view-epub-user-stylesheet
         (setq options
               (append options
                       (list (format "-U%s"
                                     (expand-file-name
-                                     doc-view-mutool-user-stylesheet)))))))
+                                     doc-view-epub-user-stylesheet)))))))
     (doc-view-start-process
      "pdf->png" doc-view-pdfdraw-program
      `(,@(doc-view-pdfdraw-program-subcommand)
-- 
2.36.1


[-- Attachment #1.3: Type: text/plain, Size: 216 bytes --]


Assuming there is still interest in merging this, does this warrant a
NEWS entry/mention in the manual?

-- 
Kjartan Óli Ágústsson

GPG Key fingerprint: 4801 0D71 49C0 1DD6 E5FD  6AC9 D757 2FE3 605E E6B0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 691 bytes --]

bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed

[Lars Ingebrigtsen]

Kjartan Oli Agustsson <kjartanoli@outlook.com> writes:

> Having finally found the time to work on this again, I believe the
> attached patch should fix all the issues identified by you and Lars.

Thanks; pushed to Emacs 29.

> Assuming there is still interest in merging this, does this warrant a
> NEWS entry/mention in the manual?

No, I don't think that's necessary.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no