GNU bug report logs - #64516
[PATCH] docview: Only enable imenu when supported

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 64516 in the body.
You can then email your comments to 64516 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: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] docview: Only enable imenu when supported
Date: Fri, 07 Jul 2023 11:24:01 -0400

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

Hello,

More info is in the commit message.

To re-create the error open up an EPUB file (or possibly any non-PDF
file in doc-view) and press 'M-g i' to run 'imenu'.  Doing that I get
the following error:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  string-match("\\`PK\\'" nil)
  imenu-find-default("PK" (("*Rescan*" . -99) nil))
  imenu--completion-buffer((("*Rescan*" . -99) nil) nil)
  imenu-choose-buffer-index()
  byte-code("\300 C\207" [imenu-choose-buffer-index] 1)
  command-execute(imenu)

[0001-docview-Only-enable-imenu-when-supported.patch (text/x-patch, attachment)]

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 64516 <at> debbugs.gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Fri, 07 Jul 2023 11:40:20 -0400

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

Sorry, here is a V2.  The previous patch would fail for files with funny
filenames that need escaping.  This I have removed the call to
'shell-quote-argument' as 'call-process' doesn't want escapes.

[0001-docview-Only-enable-imenu-when-supported.patch (text/x-patch, attachment)]

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: 64516 <at> debbugs.gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Fri, 07 Jul 2023 12:05:56 -0400

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

Sorry, I wasn't thinking very hard about the simplifications I made in
the V2 patch.  Now it would error out if '(or file-name
(buffer-file-name))' was nil which I assume can happen.

Here is a V3 patch which should be good now.

[0001-docview-Only-enable-imenu-when-supported.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>, Tassilo Horn <tsdh <at> gnu.org>
Cc: 64516 <at> debbugs.gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sat, 08 Jul 2023 12:14:40 +0300

> From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
> Date: Fri, 07 Jul 2023 11:24:01 -0400
> 
> More info is in the commit message.
> 
> To re-create the error open up an EPUB file (or possibly any non-PDF
> file in doc-view) and press 'M-g i' to run 'imenu'.  Doing that I get
> the following error:
> 
> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>   string-match("\\`PK\\'" nil)
>   imenu-find-default("PK" (("*Rescan*" . -99) nil))
>   imenu--completion-buffer((("*Rescan*" . -99) nil) nil)
>   imenu-choose-buffer-index()
>   byte-code("\300 C\207" [imenu-choose-buffer-index] 1)
>   command-execute(imenu)

Thanks.

Tassilo, any comments to the problem and the proposed patch?

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, 64516 <at> debbugs.gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sun, 09 Jul 2023 10:13:41 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Morgan & Eli,

>> To re-create the error open up an EPUB file (or possibly any non-PDF
>> file in doc-view) and press 'M-g i' to run 'imenu'.  Doing that I get
>> the following error:
>> 
>> Debugger entered--Lisp error: (wrong-type-argument stringp nil)
>>   string-match("\\`PK\\'" nil)
>>   imenu-find-default("PK" (("*Rescan*" . -99) nil))
>>   imenu--completion-buffer((("*Rescan*" . -99) nil) nil)
>>   imenu-choose-buffer-index()
>>   byte-code("\300 C\207" [imenu-choose-buffer-index] 1)
>>   command-execute(imenu)
>
> Thanks.
>
> Tassilo, any comments to the problem and the proposed patch?

I couldn't test it because all epub files I found result in

  imenu unavailable: "No items suitable for an index found in this buffer"

and not the described error.  Anyhow, I've looked at the patch (V3)
which basically looks good to me.

However, I'm not sure if setting doc-view-imenu-enabled to nil if mutool
is there but the doc is not a PDF is the right thing.  I think the only
difference is the error you get when invoking imenu, i.e., if we'd skip
that hunk of the patch and cater for the possibility that "mutool show"
might eventually support epub files, you'd get

  "Unable to create imenu index using `mutool'"

right now.  With that hunk, you get

  "This buffer cannot use ‘imenu-default-create-index-function’"

which is not really better.

So I'd suggest leave the doc-view-imenu-enabled initialization as-is and
in the "mutool show exits non-zero case", we signal a
imenu-unavailable-error (rather than a plain error) with the message
that's already there in the patch.

Bye,
Tassilo

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Tue, 11 Jul 2023 13:49:25 -0400

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

Hello,

Tassilo Horn <tsdh <at> gnu.org> writes:

>
> I couldn't test it because all epub files I found result in
>
>   imenu unavailable: "No items suitable for an index found in this buffer"
>
> and not the described error.

I feel like my reviewers often can't reproduce my errors.  I'm running
Emacs from a very recent commit and I have mutool installed.  I really
feel like the error should be reproducible for anyone that meets those
two requirements.  Do I need to include more information in my bug
reports?  If so, what information?


Anyways I modified my patch with Tassilo's feedback.  However, I
realized that it partially reverts commit
b23e062d7463b76d25dfd9ba4a80c1848a448e42

[0001-docview-imenu-check-return-value-of-mutool.patch (text/x-patch, attachment)]

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

I can't really seem to figure out why we call 'doc-view--pdf-outline' in
doc-view-imenu-setup even after reading the two bug reports that are
linked in the commit message.  For this reason I CC'd jao

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Tue, 11 Jul 2023 14:22:49 -0400

[0001-docview-imenu-check-return-value-of-mutool.patch (text/x-patch, attachment)]

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

Hello,

I apologize for my "oops I figured it out immediately after I sent the
email" syndrome.  I was trying to find the answer in the bug reports
when the answer was in the code and quite obvious.

Anyways here is a patch that unconditionally enables imenu but checks
the return value of mutool and will raise an imenu-unavailable-error
when it is not successful.  However, that error stops docview from
displaying the document so I decided to put the setup function in a hook
so the error occurs after the document is displayed.

I looked briefly for documentation on how errors work in hooks but could
not find any.  I don't want this error to prevent the other hooks from
being run (which would be user defined hooks.  we don't currently use
this hook).

Thanks,

Morgan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: jao <at> gnu.org, 64516 <at> debbugs.gnu.org, tsdh <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Tue, 11 Jul 2023 21:28:04 +0300

> From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  64516 <at> debbugs.gnu.org, jao <at> gnu.org
> Date: Tue, 11 Jul 2023 13:49:25 -0400
> 
> Anyways I modified my patch with Tassilo's feedback.  However, I
> realized that it partially reverts commit
> b23e062d7463b76d25dfd9ba4a80c1848a448e42

And reintroduces the problem which that commit fixed?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: jao <at> gnu.org, 64516 <at> debbugs.gnu.org, tsdh <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sat, 15 Jul 2023 11:05:11 +0300

> From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  64516 <at> debbugs.gnu.org,  jao <at> gnu.org
> Date: Tue, 11 Jul 2023 14:22:49 -0400
> 
> I apologize for my "oops I figured it out immediately after I sent the
> email" syndrome.  I was trying to find the answer in the bug reports
> when the answer was in the code and quite obvious.
> 
> Anyways here is a patch that unconditionally enables imenu but checks
> the return value of mutool and will raise an imenu-unavailable-error
> when it is not successful.  However, that error stops docview from
> displaying the document so I decided to put the setup function in a hook
> so the error occurs after the document is displayed.
> 
> I looked briefly for documentation on how errors work in hooks but could
> not find any.  I don't want this error to prevent the other hooks from
> being run (which would be user defined hooks.  we don't currently use
> this hook).

Tassilo, is this okay, in your opinion?

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, 64516 <at> debbugs.gnu.org,
 jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sat, 15 Jul 2023 20:39:20 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

Hi Morgan & Eli,

>> Anyways here is a patch that unconditionally enables imenu but checks
>> the return value of mutool and will raise an imenu-unavailable-error
>> when it is not successful.  However, that error stops docview from
>> displaying the document so I decided to put the setup function in a
>> hook so the error occurs after the document is displayed.
>
> Tassilo, is this okay, in your opinion?

Yes, almost.  I'd rather handle the error rather putting the setup
function in a hook and let the error hit top-level and flash the screen.

--8<---------------cut here---------------start------------->8---
1 file changed, 11 insertions(+), 5 deletions(-)
lisp/doc-view.el | 16 +++++++++++-----

modified   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 @@ doc-view-mupdf-use-svg
   :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")
@@ -1910,9 +1912,10 @@ doc-view--pdf-outline
   (let ((fn (or file-name (buffer-file-name))))
     (when fn
       (let ((outline nil)
-            (fn (shell-quote-argument (expand-file-name fn))))
+            (fn (expand-file-name fn)))
         (with-temp-buffer
-          (insert (shell-command-to-string (format "mutool show %s outline" fn)))
+          (unless (= 0 (call-process "mutool" nil (current-buffer) nil "show" fn "outline"))
+            (imenu-unavailable-error "Unable to create imenu index using `mutool'"))
           (goto-char (point-min))
           (while (re-search-forward doc-view--outline-rx nil t)
             (push `((level . ,(length (match-string 1)))
@@ -1961,7 +1964,7 @@ doc-view-imenu-index
 
 (defun doc-view-imenu-setup ()
   "Set up local state in the current buffer for imenu, if needed."
-  (when (and doc-view-imenu-enabled (executable-find "mutool"))
+  (when doc-view-imenu-enabled
     (setq-local imenu-create-index-function #'doc-view-imenu-index
                 imenu-submenus-on-top nil
                 imenu-sort-function nil
@@ -2236,7 +2239,10 @@ doc-view-mode
     (setq mode-name "DocView"
 	  buffer-read-only t
 	  major-mode 'doc-view-mode)
-    (doc-view-imenu-setup)
+    (condition-case imenu-error
+        (doc-view-imenu-setup)
+      (imenu-unavailable (message "imenu support unavailable: %s"
+                                  (cadr imenu-error))))
     (doc-view-initiate-display)
     ;; Switch off view-mode explicitly, because doc-view-mode is the
     ;; canonical view mode for PDF/PS/DVI files.  This could be
--8<---------------cut here---------------end--------------->8---

Bye,
Tassilo

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sat, 15 Jul 2023 19:50:52 -0400

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

Tassilo Horn <tsdh <at> gnu.org> writes:

>
> Yes, almost.  I'd rather handle the error rather putting the setup
> function in a hook and let the error hit top-level and flash the screen.
>

I've made the requested change.  Please see attached.

Thank you,

Morgan

[0001-docview-imenu-check-return-value-of-mutool.patch (text/x-patch, attachment)]

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sun, 16 Jul 2023 06:39:57 +0200 (GMT+02:00)

Hi Morgan,

Why do you ignore the error altogether? FWIW, just use the patch from my last message and we're good to commit.

Thanks,
Tassilo

16.07.2023 01:56:09 Morgan Smith <Morgan.J.Smith <at> outlook.com>:

> Tassilo Horn <tsdh <at> gnu.org> writes:
> 
>> 
>> Yes, almost.  I'd rather handle the error rather putting the setup
>> function in a hook and let the error hit top-level and flash the screen.
>> 
> 
> I've made the requested change.  Please see attached.
> 
> Thank you,
> 
> Morgan

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sun, 16 Jul 2023 11:03:12 -0400

Hello,

First of all I'd like to apologize for dragging this out and missing
some details.  If I paid a little more attention this would likely be
done by now :P

Tassilo Horn <tsdh <at> gnu.org> writes:

> Why do you ignore the error altogether?

So we have 3 scenarios: mutool not installed, mutool failed, and mutool
is successful.

Your patch would send a message on setup when mutool fails, but would
not say anything in the other two scenarios.

My patch would be silent on setup.

Both patches would give the user an error if they actually try to use
imenu.

In my opinion, we should either be completely silent (on setup) or send a message
when imenu is set up correctly.  If you wanted to send a message when it
fails I would accept that, but maybe we should consider also sending the
message when mutool is not installed.


Also, we have a macro 'with-demoted-errors' but we are missing a
'with-demoted-error' macro that lets you specify what error to demote.
It would be cool if you converted this part of your patch into a
definition of a 'with-demoted-error' macro.


--8<---------------cut here---------------start------------->8---
+    (condition-case imenu-error
+        (doc-view-imenu-setup)
+      (imenu-unavailable (message "imenu support unavailable: %s"
+                                  (cadr imenu-error))))
--8<---------------cut here---------------end--------------->8---

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Sun, 16 Jul 2023 17:24:31 +0200 (GMT+02:00)

Hi Morgan,

my reasoning was to have an explanation in the *Messages* buffer for people who ask themselves "where the heck is the Outline menu item"? If it's done by condition-case or with-demoted-errors doesn't really matter. Feel free to adapt it to your preference.

Thanks,
Tassilo

16.07.2023 17:03:05 Morgan Smith <Morgan.J.Smith <at> outlook.com>:

> Hello,
> 
> First of all I'd like to apologize for dragging this out and missing
> some details.  If I paid a little more attention this would likely be
> done by now :P
> 
> Tassilo Horn <tsdh <at> gnu.org> writes:
> 
>> Why do you ignore the error altogether?
> 
> So we have 3 scenarios: mutool not installed, mutool failed, and mutool
> is successful.
> 
> Your patch would send a message on setup when mutool fails, but would
> not say anything in the other two scenarios.
> 
> My patch would be silent on setup.
> 
> Both patches would give the user an error if they actually try to use
> imenu.
> 
> In my opinion, we should either be completely silent (on setup) or send a message
> when imenu is set up correctly.  If you wanted to send a message when it
> fails I would accept that, but maybe we should consider also sending the
> message when mutool is not installed.
> 
> 
> Also, we have a macro 'with-demoted-errors' but we are missing a
> 'with-demoted-error' macro that lets you specify what error to demote.
> It would be cool if you converted this part of your patch into a
> definition of a 'with-demoted-error' macro.
> 
> 
> --8<---------------cut here---------------start------------->8---
> +    (condition-case imenu-error
> +        (doc-view-imenu-setup)
> +      (imenu-unavailable (message "imenu support unavailable: %s"
> +                                  (cadr imenu-error))))
> --8<---------------cut here---------------end--------------->8---

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

From: Morgan Smith <Morgan.J.Smith <at> outlook.com>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Tue, 18 Jul 2023 14:40:12 -0400

Tassilo Horn <tsdh <at> gnu.org> writes:

> my reasoning was to have an explanation in the *Messages* buffer for people who
> ask themselves "where the heck is the Outline menu item"?

I didn't think of users using the toolbar.  I think you can go ahead and
use the patch you sent a couple emails back.  I'm happy with that one.

Thank you for all your help!

Morgan

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Tue, 18 Jul 2023 21:14:35 +0200

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

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

Hi Morgan,

>> my reasoning was to have an explanation in the *Messages* buffer for
>> people who ask themselves "where the heck is the Outline menu item"?
>
> I didn't think of users using the toolbar.

I've heared there are three of them.  But we are talking about the menu
bar of which there are a good dozen users. :-)

> I think you can go ahead and use the patch you sent a couple emails
> back.  I'm happy with that one.

Alright, I've amended your patch with my slight modifications.  Eli,
feel free to "git am" it where you see fit.  It should apply both on
master and on emacs-29.

Thanks,
Tassilo

[0001-docview-imenu-check-return-value-of-mutool.patch (text/x-patch, attachment)]

Message #55 received at 64516-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Morgan.J.Smith <at> outlook.com, jao <at> gnu.org, 64516-done <at> debbugs.gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Thu, 20 Jul 2023 19:05:38 +0300

> From: Tassilo Horn <tsdh <at> gnu.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 64516 <at> debbugs.gnu.org, jao <at> gnu.org
> Date: Tue, 18 Jul 2023 21:14:35 +0200
> 
> Alright, I've amended your patch with my slight modifications.  Eli,
> feel free to "git am" it where you see fit.  It should apply both on
> master and on emacs-29.

Thanks to both of you.  I've now installed this on the master branch,
and I'm closing the bug.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, Eli Zaretskii <eliz <at> gnu.org>,
 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Fri, 28 Jul 2023 12:14:14 +0200

> 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 @@
[...]
> +          (unless (= 0 (call-process "mutool" nil (current-buffer) nil "show" fn "outline"))
> +            (imenu-unavailable-error "Unable to create imenu index using `mutool'"))

Is call-process guaranteed to return a number?
Would eq/l be more suitable than =?

[ The same also appears in doc-view--revert-buffer. ]

Thanks,
-- 
Basil

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

From: Tassilo Horn <tsdh <at> gnu.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, Eli Zaretskii <eliz <at> gnu.org>,
 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Fri, 28 Jul 2023 12:19:20 +0200

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

>> 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 @@
> [...]
>> +          (unless (= 0 (call-process "mutool" nil (current-buffer) nil "show" fn "outline"))
>> +            (imenu-unavailable-error "Unable to create imenu index using `mutool'"))
>
> Is call-process guaranteed to return a number?
> Would eq/l be more suitable than =?

Good question, the docs say:

  If DESTINATION is 0, ‘call-process’ returns immediately with value nil.
  Otherwise it waits for PROGRAM to terminate
  and returns a numeric exit status or a signal description string.

I guess if mutool is killed externally while emacs runs it, it could be
such a "signal description string" in which case eql would be better
even though that situation seems unlikely.  I'll change that later.

Thanks for the heads-up,
Tassilo

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: Morgan Smith <Morgan.J.Smith <at> outlook.com>, Eli Zaretskii <eliz <at> gnu.org>,
 64516 <at> debbugs.gnu.org, jao <at> gnu.org
Subject: Re: bug#64516: [PATCH] docview: Only enable imenu when supported
Date: Fri, 28 Jul 2023 13:22:23 +0200

Tassilo Horn [2023-07-28 12:19 +0200] wrote:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Is call-process guaranteed to return a number?
>> Would eq/l be more suitable than =?
>
> Good question, the docs say:
>
>   If DESTINATION is 0, ‘call-process’ returns immediately with value nil.
>   Otherwise it waits for PROGRAM to terminate
>   and returns a numeric exit status or a signal description string.
>
> I guess if mutool is killed externally while emacs runs it, it could be
> such a "signal description string" in which case eql would be better
> even though that situation seems unlikely.

Right, and it's often hard to decide whether/how the user should be made
aware of such exceptional circumstances.

> I'll change that later.

Thanks,
-- 
Basil

Previous Next

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