Re: master 65108998b1e: docview: imenu: check return value of 'mutool'

[Arash Esbati]

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