From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: dalanicolai Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] add epub support to doc-view Date: Tue, 11 Jan 2022 11:13:42 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000d92dd305d54bb55d" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="36586"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Emacs Devel To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jan 11 11:21:12 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n7EH9-0009Na-Pk for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Jan 2022 11:21:11 +0100 Original-Received: from localhost ([::1]:55264 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n7EH8-0007f9-Hm for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Jan 2022 05:21:10 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:39084) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n7EAY-0008Tc-KU for emacs-devel@gnu.org; Tue, 11 Jan 2022 05:14:22 -0500 Original-Received: from [2607:f8b0:4864:20::a33] (port=41904 helo=mail-vk1-xa33.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n7EAV-00027l-LY for emacs-devel@gnu.org; Tue, 11 Jan 2022 05:14:22 -0500 Original-Received: by mail-vk1-xa33.google.com with SMTP id e123so1013295vkb.8 for ; Tue, 11 Jan 2022 02:13:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3f9iZ4+qaWGGP33h1+p75XalKnCdbg2MNxW6ZTXG1TY=; b=AjuhrFLFTU/HC2IeqVmWIXDErI4QAKHJ979AsJvxqCb6xco/S2fvZP0YEMow1nSRem T1HgWH4v9aqv5wvA+uqASH+yXyJPr2SSsSkOgvIi6W3UAx2UYSJTfg0ibDOwv+dLB3fj Oa+RXqq2fjEcei7ZtlIHwR0aggk5TEHdOQYAI6PFVtalePUliVPsh2V0HktTF2bGlG+s 11HK+YSXxQMr20XDBzU3kj2jzU+pjS8sECzLZBAE9pkLVqaWnXHFe63kUU+VmCP65YOx ZbjcOb6Bx8FXd4stL57yRFhs4SzncbQn0U5PkFE/m7aG98tXqOV/OTvc0E/3OuvXPEl2 a+BA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3f9iZ4+qaWGGP33h1+p75XalKnCdbg2MNxW6ZTXG1TY=; b=tB5ClbENLhv6L0Le/kebZl7PjXA7GJqcqiIuHkmat96Ui0EV1vKQjuAWVnd7sBOXpv au2tmEvWannN4pgeR+ve5elWty9eThOIJbvpRFQ058SbzJQH+G4nPzJE/7/OAPL86RQP 7VBh2zPYB0Cl+vhtB+xU8JEA+yzSH2AnPY4Y8L+JdVjkHtAJSKySNmViq7ehcBZNi+vR 57WoQTCqIOnqczxIYyEw34dnjE5cM1+6ExTey3E3Vmy1P9lm+gfg0bKDIAdEIR/Tm9me xaqf+XImZfG6RkRhH9nVJb+GHwgiw90IpItqanyQuDtQ2p1IZGNFoIwsIsy3ZsW9+Li8 L7rQ== X-Gm-Message-State: AOAM530ycQL7qW6Z/Zq8yTqlu9zwFgIr1rP337s1ItgB+TfjT/Sa97Qn Xo9/Ihe7wRqqQyJBZMO+CCls/3GNfGwiRmFnr4YqgWMWsyU= X-Google-Smtp-Source: ABdhPJynSSs1eYNMrkpZKgOCrpnqX0kxT6K8Lm2y7u2gVR28ts893euOzEQ1kPpEEEohw3njwdDsdrSJ9GHY+Nvvp7A= X-Received: by 2002:a05:6122:c9e:: with SMTP id ba30mr1628251vkb.17.1641896033724; Tue, 11 Jan 2022 02:13:53 -0800 (PST) In-Reply-To: X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::a33 (failed) Received-SPF: pass client-ip=2607:f8b0:4864:20::a33; envelope-from=dalanicolai@gmail.com; helo=mail-vk1-xa33.google.com X-Spam_score_int: -12 X-Spam_score: -1.3 X-Spam_bar: - X-Spam_report: (-1.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:284591 Archived-At: --000000000000d92dd305d54bb55d Content-Type: text/plain; charset="UTF-8" Forgot to provide the link to the (info about the) pymupdf server, in case someone is interested... https://github.com/vedang/pdf-tools/pull/61 On Tue, 11 Jan 2022 at 10:59, dalanicolai wrote: > > (define-obsolete-variable-alias 'doc-view-unoconv-program >> > > - 'doc-view-odf->pdf-converter-program >> > - "24.4") >> > + 'doc-view-odf->pdf-converter-program >> > + "24.4") >> >> Thanks. At this point, the last two args can be placed on the same >> line ;-) > > > This is also due to the auto-indent, but I will correct it manually. > > > (defun doc-view-new-window-function (winprops) >> > + ;; TODO: write documentation! >> > ;; (message "New window %s for buf %s" (car winprops) >> (current-buffer)) >> > (cl-assert (or (eq t (car winprops)) >> > (eq (window-buffer (car winprops)) (current-buffer)))) >> >> I plead guilty, but if you give me a more specific question, I'll find >> it easier to answer it (even in the form of a docstring or comment). >> > > This I copied from pdf-tools. However, I am planning to enter this > documentation > myself, while I am implementing real continuous scroll. I have got that > working in > pdf-tools already, but I had to disable all kinds of minor modes there. So > I thought > let's start with a simpler task of implementing it in doc-view, however, > the same > difficulty, that I encountered also in pdf-tools is that somehow > `bookroll-mode-winprops-alist` (which is exactly the equivalent/copy of > `bookroll-mode-winprops-alist`) obtains a value `t`. I think I've spent > some hours > already investigating where it obtains it (somehow I fixed it after > debugging for > pdf-tools for few hours, but clearly I do not understand how, and have to > repeat the > same things again, well I guess it is a good exercise. Anyway, in the end > I hope to > add some useful documentation to prevent other hackers from 'wasting' > their time :) > Anyway, if I can keep it like this for now, that would be great, as the > TODO really > jumps out font lock wise, and also I can search for it. > > > @@ -738,7 +740,7 @@ doc-view-kill-proc >> > (interactive) >> > (while (consp doc-view--current-converter-processes) >> > (ignore-errors ;; Some entries might not be processes, and maybe >> > - ;; some are dead already? >> > + ;; some are dead already? >> > (kill-process (pop doc-view--current-converter-processes)))) >> > (when doc-view--current-timer >> > (cancel-timer doc-view--current-timer) >> >> Here the auto-indentation gets it wrong, sadly. >> > > You mean because it aligns with the line below so that it looks like it > belongs > to that one? Because otherwise, I think it does not look bad. > > > + ((memq type '(postscript ps eps pdf epub)) >> > + (if-let (command (and (memq type '(pdf epub)) >> (executable-find "mutool"))) >> > + command >> > + (unless (eq type 'epub) >> > + (and doc-view-ghostscript-program >> > + (executable-find doc-view-ghostscript-program))))) >> > ((eq type 'odf) >> > (and doc-view-odf->pdf-converter-program >> > (executable-find doc-view-odf->pdf-converter-program) >> >> Hmm... for PDF files, this changes the previous behavior where we >> checked for the presence of `doc-view-pdfdraw-program` rather than >> "mutool". >> Was there a strong reason to hardcode "mutool" here >> > > Ah yes, I was aware of that, but indeed I did that consciously. I have > very good experiences > with (py)mupdf (I have implemented an alternative pdf-tools server using > it, and indeed it is > much faster than the epdfinfo server, well at least the mupdf vs poppler > parts. > > really take a very quick look at the two histograms in this link > > . > So I decided this behavior really wouldn't be a problem, why not prefer > mutool when it is > available? > > Also, I think the rest of the code will still launch >> `doc-view-pdfdraw-program` rather than `mutool`, so there's >> something fishy. >> > > That I do not understand immediately, for me the behavior is still > somewhat opaque. But I > will be trying to understand things further today (anyway for implementing > the continuous scroll) > and try to understand how it all works (and correct it here). > > This error message needs to be shorter. It probably shouldn't talk >> about `mutool`. Even with this simplification I think we're going to >> have to be creative to make it short enough to fit in a single line of >> 80 columns. We should probably just say something like >> >> doc-view-pdf->png-converter-function invalid for EPub >> >> and then add in the docstring of `doc-view-pdf->png-converter-function` >> a note about the fact that only `doc-view-pdf->png-converter-mupdf` >> supports EPub. >> > > That sounds good indeed. I will take care of that. > > Thanks, for the feedback... I will try another 'round' probably sooner > than later :) > > > > > > On Tue, 11 Jan 2022 at 03:48, Stefan Monnier > wrote: > >> > Here is a patch to add epub support to doc-view, via the mutool >> > command of the mupdf library,I guess the patch needs no further >> > explanation. I've read the info about sending patches, however, >> > I don't understand the section about the changelog (it mentions that >> > there is a changelog or something, but it doesn't say where). Also, >> > the info tells us to write the commit log entries, but it does not say >> > where to do this.Looking at the changelog files, I guess that info is >> > just really outdated (am I right?). Anyway, the patch is in the >> > attachment. If there is some part of the 'protocol' that I did not >> > understand then please inform me about it. >> >> Looks pretty good, see below for some comments/questions. >> Regarding the changelog, we just need you to give us something like >> >> doc-view.el: Add support for EPub >> >> >> >> * lisp/doc-view.el (): . >> (): . >> ... >> >> * lisp/files.el (auto-mode-alist): Extend doc-view-mode-maybe >> regexp to match `.epub` files. >> >> > (define-obsolete-variable-alias 'doc-view-unoconv-program >> > - 'doc-view-odf->pdf-converter-program >> > - "24.4") >> > + 'doc-view-odf->pdf-converter-program >> > + "24.4") >> >> >> Thanks. At this point, the last two args can be placed on the same >> line ;-) >> >> > (defun doc-view-new-window-function (winprops) >> > + ;; TODO: write documentation! >> > ;; (message "New window %s for buf %s" (car winprops) >> (current-buffer)) >> > (cl-assert (or (eq t (car winprops)) >> > (eq (window-buffer (car winprops)) (current-buffer)))) >> >> I plead guilty, but if you give me a more specific question, I'll find >> it easier to answer it (even in the form of a docstring or comment). >> >> > @@ -738,7 +740,7 @@ doc-view-kill-proc >> > (interactive) >> > (while (consp doc-view--current-converter-processes) >> > (ignore-errors ;; Some entries might not be processes, and maybe >> > - ;; some are dead already? >> > + ;; some are dead already? >> > (kill-process (pop doc-view--current-converter-processes)))) >> > (when doc-view--current-timer >> > (cancel-timer doc-view--current-timer) >> >> Here the auto-indentation gets it wrong, sadly. >> >> > @@ -810,11 +812,12 @@ doc-view-mode-p >> > (executable-find doc-view-dvipdf-program)) >> > (and doc-view-dvipdfm-program >> > (executable-find doc-view-dvipdfm-program))))) >> > - ((memq type '(postscript ps eps pdf)) >> > - (or (and doc-view-ghostscript-program >> > - (executable-find doc-view-ghostscript-program)) >> > - (and doc-view-pdfdraw-program >> > - (executable-find doc-view-pdfdraw-program)))) >> > + ((memq type '(postscript ps eps pdf epub)) >> > + (if-let (command (and (memq type '(pdf epub)) >> (executable-find "mutool"))) >> > + command >> > + (unless (eq type 'epub) >> > + (and doc-view-ghostscript-program >> > + (executable-find doc-view-ghostscript-program))))) >> > ((eq type 'odf) >> > (and doc-view-odf->pdf-converter-program >> > (executable-find doc-view-odf->pdf-converter-program) >> >> Hmm... for PDF files, this changes the previous behavior where we >> checked for the presence of `doc-view-pdfdraw-program` rather than >> "mutool". >> Was there a strong reason to hardcode "mutool" here? >> >> Depending on the reason, we may be better off splitting the epub >> handling into its own `cond` branch. >> >> Also, I think the rest of the code will still launch >> `doc-view-pdfdraw-program` rather than `mutool`, so there's >> something fishy. >> >> > - ((or 'pdf 'djvu) >> > + ((or 'pdf 'epub 'djvu) >> > + (when (eq doc-view-doc-type 'epub) >> > + (unless (eq doc-view-pdf->png-converter-function >> 'doc-view-pdf->png-converter-mupdf) >> > + (user-error "Viewing epub documents requires the`mutool' >> command to be available, >> > +and `doc-view-pdf->png-converter-function' variable set to >> > +`doc-view-pdf->png-converter-mupdf'"))) >> >> This error message needs to be shorter. It probably shouldn't talk >> about `mutool`. Even with this simplification I think we're going to >> have to be creative to make it short enough to fit in a single line of >> 80 columns. We should probably just say something like >> >> doc-view-pdf->png-converter-function invalid for EPub >> >> and then add in the docstring of `doc-view-pdf->png-converter-function` >> a note about the fact that only `doc-view-pdf->png-converter-mupdf` >> supports EPub. >> >> The rest looks good. >> >> >> Stefan >> >> --000000000000d92dd305d54bb55d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Forgot to provide the link to the (info about the) py= mupdf server, in case someone is interested...


On Tue, 11 Jan 2022 at 10:59, dalanicolai &l= t;dalanicolai@gmail.com> wr= ote:
>=C2= =A0 (define-obsolete-variable-alias 'doc-view-unoconv-program
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'doc-view-odf->pdf-conver= ter-program
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "24.4")
> +=C2=A0 'doc-view-odf->pdf-converter-program
> +=C2=A0 "24.4")

Thanks.=C2=A0 At this point, the last two args can be placed on the same line ;-)

This is also due to the auto-inden= t, but I will correct it manually.

>=C2=A0 (defun doc-view-new-window-fu= nction (winprops)
> +=C2=A0 ;; TODO: write documentation!
>=C2=A0 =C2=A0 ;; (message "New window %s for buf %s" (car win= props) (current-buffer))
>=C2=A0 =C2=A0 (cl-assert (or (eq t (car winprops))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(e= q (window-buffer (car winprops)) (current-buffer))))

I plead guilty, but if you give me a more specific question, I'll find<= br> it easier to answer it (even in the form of a docstring or comment).

This I copied from pdf-tools. However, = I am planning to enter this documentation
myself, while I am impl= ementing real continuous scroll. I have got that working in
pdf-t= ools already, but I had to disable all kinds of minor modes there. So I tho= ught
let's start with a simpler task of implementing it in do= c-view, however, the same
difficulty, that I encountered also in = pdf-tools is that somehow
`bookroll-mode-winprops-alist` (which i= s exactly the equivalent/copy of
`bookroll-mode-winprops-alist`) = obtains a value `t`. I think I've spent some hours
already in= vestigating where it obtains it (somehow I fixed it after debugging for
pdf-tools for few hours, but clearly I do not understand how, and ha= ve to repeat the
same things again, well I guess it is a good exe= rcise. Anyway, in the end I hope to
add some useful documentation= to prevent other hackers from 'wasting' their time :)
An= yway, if I can keep it like this for now, that would be great, as the TODO = really
jumps out font lock wise, and also I can search for it.

&g= t; @@ -738,7 +740,7 @@ doc-view-kill-proc
>=C2=A0 =C2=A0 (interactive)
>=C2=A0 =C2=A0 (while (consp doc-view--current-converter-processes)
>=C2=A0 =C2=A0 =C2=A0 (ignore-errors ;; Some entries might not be proces= ses, and maybe
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; some are d= ead already?
> +=C2=A0 =C2=A0 =C2=A0 ;; some are dead already?
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 (kill-process (pop doc-view--current-conver= ter-processes))))
>=C2=A0 =C2=A0 (when doc-view--current-timer
>=C2=A0 =C2=A0 =C2=A0 (cancel-timer doc-view--current-timer)

Here the auto-indentation gets it wrong, sadly.

=
You mean because it aligns with the line below so that it looks = like it belongs
to that one? Because otherwise, I think it does n= ot look bad.

> +=C2=A0 =C2=A0 =C2=A0((memq type '(postscript ps eps = pdf epub))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if-let (command (and (memq type &#= 39;(pdf epub)) (executable-find "mutool")))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0command
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (unless (eq type 'epub)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(and doc-view-ghostsc= ript-program
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(executable-fi= nd doc-view-ghostscript-program)))))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0((eq type 'odf)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 (and doc-view-odf->pdf-converter-program=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(executable-find doc-vi= ew-odf->pdf-converter-program)

Hmm... for PDF files, this changes the previous behavior where we
checked for the presence of `doc-view-pdfdraw-program` rather than "mu= tool".
Was there a strong reason to hardcode "mutool" here

Ah yes, I was aware of that, but indeed I did that= consciously. I have very good experiences
with (py)mupdf (I have= implemented an alternative pdf-tools server using it, and indeed it is
much faster than the epdfinfo server, well at least the mupdf vs pop= pler parts.

really take a very quick look at the t= wo histograms in this link.
So I decided this behavio= r really wouldn't be a problem, why not prefer mutool when it is
<= div>available?

Also, I think the rest of the code will still launch
`doc-view-pdfdraw-program` rather than `mutool`, so there's
something fishy.

That I do not unders= tand immediately, for me the behavior is still somewhat opaque. But I
=
will be trying to understand things further today (anyway for implemen= ting the continuous scroll)
and try to understand how it all work= s (and correct it here).

This error message needs to be shorter.=C2=A0 It p= robably shouldn't talk
about `mutool`.=C2=A0 Even with this simplification I think we're going= to
have to be creative to make it short enough to fit in a single line of
80 columns.=C2=A0 We should probably just say something like

=C2=A0 =C2=A0 doc-view-pdf->png-converter-function invalid for EPub

and then add in the docstring of `doc-view-pdf->png-converter-function`<= br> a note about the fact that only `doc-view-pdf->png-converter-mupdf`
supports EPub.

That sounds good indee= d. I will take care of that.

Thanks, for the feedb= ack... I will try another 'round' probably sooner than later :)
=





= On Tue, 11 Jan 2022 at 03:48, Stefan Monnier <monnier@iro.umontreal.ca> wrote:=
> Here is a = patch to add epub support to doc-view, via the mutool
> command of the mupdf library,I guess the patch needs no further
> explanation.=C2=A0 I've read the info about sending patches, howev= er,
> I don't understand the section about the changelog (it mentions th= at
> there is a changelog or something, but it doesn't say where). Also= ,
> the info tells us to write the commit log entries, but it does not say=
> where to do this.Looking at the changelog files, I guess that info is<= br> > just really outdated (am I right?).=C2=A0 Anyway, the patch is in the<= br> > attachment.=C2=A0 If there is some part of the 'protocol' that= I did not
> understand then please inform me about it.

Looks pretty good, see below for some comments/questions.
Regarding the changelog, we just need you to give us something like

=C2=A0 =C2=A0 doc-view.el: Add support for EPub

=C2=A0 =C2=A0 <Some explanation for the general idea>

=C2=A0 =C2=A0 * lisp/doc-view.el (<fun1>): <Do this>.
=C2=A0 =C2=A0 (<var2>): <Do that>.
=C2=A0 =C2=A0 ...

=C2=A0 =C2=A0 * lisp/files.el (auto-mode-alist): Extend doc-view-mode-maybe=
=C2=A0 =C2=A0 regexp to match `.epub` files.

>=C2=A0 (define-obsolete-variable-alias 'doc-view-unoconv-program > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'doc-view-odf->pdf-conver= ter-program
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "24.4")
> +=C2=A0 'doc-view-odf->pdf-converter-program
> +=C2=A0 "24.4")


Thanks.=C2=A0 At this point, the last two args can be placed on the same line ;-)

>=C2=A0 (defun doc-view-new-window-function (winprops)
> +=C2=A0 ;; TODO: write documentation!
>=C2=A0 =C2=A0 ;; (message "New window %s for buf %s" (car win= props) (current-buffer))
>=C2=A0 =C2=A0 (cl-assert (or (eq t (car winprops))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(e= q (window-buffer (car winprops)) (current-buffer))))

I plead guilty, but if you give me a more specific question, I'll find<= br> it easier to answer it (even in the form of a docstring or comment).

> @@ -738,7 +740,7 @@ doc-view-kill-proc
>=C2=A0 =C2=A0 (interactive)
>=C2=A0 =C2=A0 (while (consp doc-view--current-converter-processes)
>=C2=A0 =C2=A0 =C2=A0 (ignore-errors ;; Some entries might not be proces= ses, and maybe
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; some are d= ead already?
> +=C2=A0 =C2=A0 =C2=A0 ;; some are dead already?
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 (kill-process (pop doc-view--current-conver= ter-processes))))
>=C2=A0 =C2=A0 (when doc-view--current-timer
>=C2=A0 =C2=A0 =C2=A0 (cancel-timer doc-view--current-timer)

Here the auto-indentation gets it wrong, sadly.

> @@ -810,11 +812,12 @@ doc-view-mode-p
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 (executable-find doc-view-dvipdf-program))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(and doc-= view-dvipdfm-program
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 (executable-find doc-view-dvipdfm-program)))))
> -=C2=A0 =C2=A0 =C2=A0((memq type '(postscript ps eps pdf))
> -=C2=A0 =C2=A0 =C2=A0 (or (and doc-view-ghostscript-program
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(executable-fi= nd doc-view-ghostscript-program))
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(and doc-view-pdfdraw= -program
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (execu= table-find doc-view-pdfdraw-program))))
> +=C2=A0 =C2=A0 =C2=A0((memq type '(postscript ps eps pdf epub)) > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if-let (command (and (memq type &#= 39;(pdf epub)) (executable-find "mutool")))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0command
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (unless (eq type 'epub)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(and doc-view-ghostsc= ript-program
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(executable-fi= nd doc-view-ghostscript-program)))))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0((eq type 'odf)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 (and doc-view-odf->pdf-converter-program=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(executable-find doc-vi= ew-odf->pdf-converter-program)

Hmm... for PDF files, this changes the previous behavior where we
checked for the presence of `doc-view-pdfdraw-program` rather than "mu= tool".
Was there a strong reason to hardcode "mutool" here?

Depending on the reason, we may be better off splitting the epub
handling into its own `cond` branch.

Also, I think the rest of the code will still launch
`doc-view-pdfdraw-program` rather than `mutool`, so there's
something fishy.

> -=C2=A0 =C2=A0 =C2=A0 ((or 'pdf 'djvu)
> +=C2=A0 =C2=A0 =C2=A0 ((or 'pdf 'epub 'djvu)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0(when (eq doc-view-doc-type 'epub)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(unless (eq doc-view-pdf->png-co= nverter-function 'doc-view-pdf->png-converter-mupdf)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(user-error "Viewing ep= ub documents requires the`mutool' command to be available,
> +and `doc-view-pdf->png-converter-function' variable set to
> +`doc-view-pdf->png-converter-mupdf'")))

This error message needs to be shorter.=C2=A0 It probably shouldn't tal= k
about `mutool`.=C2=A0 Even with this simplification I think we're going= to
have to be creative to make it short enough to fit in a single line of
80 columns.=C2=A0 We should probably just say something like

=C2=A0 =C2=A0 doc-view-pdf->png-converter-function invalid for EPub

and then add in the docstring of `doc-view-pdf->png-converter-function`<= br> a note about the fact that only `doc-view-pdf->png-converter-mupdf`
supports EPub.

The rest looks good.


=C2=A0 =C2=A0 =C2=A0 =C2=A0 Stefan

--000000000000d92dd305d54bb55d--