GNU bug report logs - #55825
[PATCH] Reconvert EPUB buffers when user CSS is changed

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55825 in the body.
You can then email your comments to 55825 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Mon, 06 Jun 2022 19:20:04 +0000

[Message part 1 (text/plain, inline)]

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.

[0001-Reconvert-EPUB-buffers-when-user-CSS-is-changed.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

-- 
Kjartan Óli Ágústsson

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

Message #8 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Tue, 07 Jun 2022 11:57:16 +0200

Kjartan Oli Agustsson <kjartanoli <at> 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

Message #11 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Tue, 07 Jun 2022 14:06:10 +0300

> From: Kjartan Oli Agustsson <kjartanoli <at> 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?

Message #14 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Tue, 07 Jun 2022 21:40:09 +0000

[Message part 1 (text/plain, inline)]

Lars Ingebrigtsen <larsi <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Tue, 07 Jun 2022 21:51:46 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Wed, 08 Jun 2022 05:31:52 +0300

> From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
> Cc: 55825 <at> 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?

Message #23 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: larsi <at> gnus.org, 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Wed, 08 Jun 2022 05:35:52 +0300

> Cc: 55825 <at> debbugs.gnu.org
> From: Kjartan Oli Agustsson <kjartanoli <at> 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.

Message #26 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Wed, 08 Jun 2022 23:10:25 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #29 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Wed, 08 Jun 2022 23:28:08 +0000

[Message part 1 (text/plain, inline)]

Kjartan Oli Agustsson <kjartanoli <at> 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

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Thu, 09 Jun 2022 08:28:49 +0300

> From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
> Cc: 55825 <at> debbugs.gnu.org
> Date: Wed, 08 Jun 2022 23:28:08 +0000
> 
> 
> [1:text/plain Hide]
> 
> 
> Kjartan Oli Agustsson <kjartanoli <at> 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.

Message #37 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Sun, 26 Jun 2022 12:33:42 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> 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.

[0001-Reconvert-EPUB-buffers-when-user-CSS-is-changed.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

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

[signature.asc (application/pgp-signature, inline)]

Message #40 received at 55825 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Kjartan Oli Agustsson <kjartanoli <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 55825 <at> debbugs.gnu.org
Subject: Re: bug#55825: [PATCH] Reconvert EPUB buffers when user CSS is changed
Date: Sun, 26 Jun 2022 17:53:02 +0200

Kjartan Oli Agustsson <kjartanoli <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.