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 10:59:31 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="00000000000021b83505d54b83cf" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31544"; 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:04:10 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 1n7E0f-0007x6-QW for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Jan 2022 11:04:10 +0100 Original-Received: from localhost ([::1]:59884 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n7E0d-0006aq-Pc for ged-emacs-devel@m.gmane-mx.org; Tue, 11 Jan 2022 05:04:07 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:35762) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n7DwQ-0004Rn-Db for emacs-devel@gnu.org; Tue, 11 Jan 2022 04:59:47 -0500 Original-Received: from [2607:f8b0:4864:20::935] (port=45807 helo=mail-ua1-x935.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n7DwN-0007y7-Rt for emacs-devel@gnu.org; Tue, 11 Jan 2022 04:59:46 -0500 Original-Received: by mail-ua1-x935.google.com with SMTP id x33so27296128uad.12 for ; Tue, 11 Jan 2022 01:59:43 -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=HWEFCKpbWy59RNsx1FNPRLWa69ETHUdtb+WeQFBHZMM=; b=KIoPLSDkdE1P8jZ9Ke4lEDyP10uwPOlzrvhLyPmLBoDgr4cPO5Du8swfP8Ed5mPxcy Fmps/q0ZwEhDJwMQX1KRyyX7txjENFhjH7BEbhO1Jx21BO+blpcy2S5ld0TmiiMBSZWU koxvwc7jd1teyptaiGtq/3dS8bUPO/zON/+p0KPhln2D16z1JhLs1poxOp8mlpynxL3/ 3Viwnhqkqt2CHxAELp8o84Gy+772J2fupCFpGPfC4+H35QQxFdPJ3pc9salpXki2pgcf gyshmWei/CfHO+lQEgtvQnp/0q8+kE7tjmquK5pbaXHxfmgwgMhwB7QqDfPtM6aYIq5v Rb5w== 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=HWEFCKpbWy59RNsx1FNPRLWa69ETHUdtb+WeQFBHZMM=; b=wF9ILW/v2FOcvrraVUa41HzaVEmMywYsv7JmKI1jBjLiPcb+e6Epqb8AWt6NEkyPbN WU2YHmhO9lZH/pxG0x/IBeCFnaoZD9QtXvbgMRcqWopQT44dk0pI9RpgfcPdgZcqoowl 3tNwaQBN9HeRvos9R5GMbBGAxwZHkeQPxb0HZ9sWlD4bsN27a3ftiT+qmSzj1fUM73Uc p/MdWwdMwuibdGpmN7X09B+RDZwbkCJpW/xz+qC33jzfZfsWKJjqhHPWvoR2f5pvI7ae 83iKFqnATHvqEr4yjLVOuKTc1oB54tCpDN3UyhLjX/xshYTq2u0IzpKP/Xt7mRygLg2i hTEg== X-Gm-Message-State: AOAM532BYLKoOoI6FpWS2we1A2eYi9GmvTa79amjtVRopMCt72J3XP0f OMpwVO0wVLaQpROPuvyvKpTnqvg/vzrhmguV9EA= X-Google-Smtp-Source: ABdhPJyDUx7Z3x1O5lNUYew+YvwASNnOZXIlnDKjToXhJNzJxlo8HuPjL2P+HO9XoSJdq0IWBjMxRFjpphVFPY5Wcfs= X-Received: by 2002:a05:6102:3f56:: with SMTP id l22mr1649120vsv.20.1641895182840; Tue, 11 Jan 2022 01:59:42 -0800 (PST) In-Reply-To: X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::935 (failed) Received-SPF: pass client-ip=2607:f8b0:4864:20::935; envelope-from=dalanicolai@gmail.com; helo=mail-ua1-x935.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:284583 Archived-At: --00000000000021b83505d54b83cf Content-Type: text/plain; charset="UTF-8" > (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 > > --00000000000021b83505d54b83cf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
>=C2=A0 (define-obsolete-variable-alias 'doc-view-unoconv-progra= m
> -=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 behavior really wouldn= 9;t be a problem, why not prefer mutool when it is
available?

Als= o, 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

--00000000000021b83505d54b83cf--