From: Arash Esbati <arash@gnu.org>
To: emacs-devel@gnu.org
Cc: "Morgan J. Smith" <Morgan.J.Smith@outlook.com>
Subject: Re: master 65108998b1e: docview: imenu: check return value of 'mutool'
Date: Thu, 27 Jul 2023 20:49:27 +0200 [thread overview]
Message-ID: <86cz0dyytk.fsf@gnu.org> (raw)
In-Reply-To: <20230720160407.BFFFAC06C66@vcs2.savannah.gnu.org> (Eli Zaretskii's message of "Thu, 20 Jul 2023 12:04:07 -0400 (EDT)")
Eli Zaretskii <eliz@gnu.org> writes:
> branch: master
> commit 65108998b1ee0b42b57d478ba3b51f9040f04cd4
> Author: Morgan Smith <Morgan.J.Smith@outlook.com>
> Commit: Eli Zaretskii <eliz@gnu.org>
>
> docview: imenu: check return value of 'mutool'
>
> While 'mutool' supports many filetypes, 'mutool show' only
> supports PDF files. This would lead to cryptic imenu errors
> when opening other
> file types (like EPUB) since we would parse the error output.
> During my testing this caused 'imenu--index-alist' to have a
> value of '(nil).
>
> * lisp/doc-view.el (doc-view--pdf-outline): Error when 'mutool'
> returns an error. Use 'call-process' to get the return value and
> remove the call to 'shell-quote-argument' as 'call-process'
> doesn't want any escapes.
> (doc-view-mode): Handle possible error from 'doc-view-imenu-setup'.
> (doc-view-imenu-enabled): Remove superfluous (and ... t).
> (doc-view-imenu-setup): Remove check for mutool already ensured by
> 'doc-view-imenu-enabled' being non-nil.
> (Bug#64516)
> ---
> lisp/doc-view.el | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/doc-view.el b/lisp/doc-view.el
> index b14655fb274..847601872f5 100644
> --- a/lisp/doc-view.el
> +++ b/lisp/doc-view.el
> @@ -147,6 +147,8 @@
> (require 'filenotify)
> (eval-when-compile (require 'subr-x))
>
> +(autoload 'imenu-unavailable-error "imenu")
> +
> ;;;; Customization Options
>
> (defgroup doc-view nil
> @@ -214,7 +216,7 @@ are available (see Info node `(emacs)Document View')."
> :type 'boolean
> :version "30.1")
>
> -(defcustom doc-view-imenu-enabled (and (executable-find "mutool") t)
> +(defcustom doc-view-imenu-enabled (executable-find "mutool")
> "Whether to generate an imenu outline when \"mutool\" is available."
> :type 'boolean
> :version "29.1")
I'm not familiar with the code, so my apologies if this comment is off,
but the change above seems wrong. With this change,
`doc-view-imenu-enabled' becomes a string or nil which doesn't match the
boolean type. The (and ... t) part wasn't superfluous.
Best, Arash
next parent reply other threads:[~2023-07-27 18:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <168986904744.4030.15173437427788310246@vcs2.savannah.gnu.org>
[not found] ` <20230720160407.BFFFAC06C66@vcs2.savannah.gnu.org>
2023-07-27 18:49 ` Arash Esbati [this message]
2023-07-27 19:15 ` master 65108998b1e: docview: imenu: check return value of 'mutool' Eli Zaretskii
2023-07-28 4:56 ` Tassilo Horn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86cz0dyytk.fsf@gnu.org \
--to=arash@gnu.org \
--cc=Morgan.J.Smith@outlook.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).