bug#47771: 27.1; `Info-read-node-name` throws error during completion

bug#47771: 27.1; `Info-read-node-name` throws error during completion

[Daniel Mendler]

The function `Info-read-node-name` uses the `Info-read-node-name-1`
completion table. The completion table calls `Info-find-file`, which may
throw a `user-error` "Info file ... does not exist". Icomplete (and many
other completion systems) makes the assumption that completion tables do
not fail. Furthermore, completion tables already have a mechanism to
report missing completions by returning nil and printing a message. The
issue can be reproduced as follows in emacs -Q:

1. M-x icomplete-mode
2. Press "C-h i"
3. Press "g" to run `Info-goto-node`
4. Enter "(boom)"
5. Move around with the cursor left to right or wait until the
`icomplete-post-command-hook` fails.

In order to fix the issue, the following patch can be applied:

diff --git a/info.el b/info-patched.el
index 3bed743..42a7fce 100644
--- a/info.el
+++ b/info-patched.el
@@ -1881,7 +1881,8 @@ See `completing-read' for a description of 
arguments and usage."
           (lambda (string pred action)
             (complete-with-action
              action
-            (Info-build-node-completions (Info-find-file file1 nil t))
+            (let ((file2 (Info-find-file file1 'noerror t)))
+              (and file2 (Info-build-node-completions file2)))
              string pred))
          nodename predicate code))))
     ;; Otherwise use Info-read-node-completion-table.

Alternatively wrap the whole line by `ignore-errors`.

The downside of ignoring the error is that now the default completion
system error message becomes slightly less meaningful. It shows "No
matches" instead of "Info ... not found".  In case it is desired to
retain the better error messages, a discussion is needed on how
completion tables could report their matching failure. Here it is
probably sufficient to demote the error to a message.

diff --git a/info.el b/info-patched.el
index 3bed743..c38abd7 100644
--- a/info.el
+++ b/info-patched.el
@@ -1881,7 +1881,8 @@ See `completing-read' for a description of 
arguments and usage."
           (lambda (string pred action)
             (complete-with-action
              action
-            (Info-build-node-completions (Info-find-file file1 nil t))
+            (with-demoted-errors "%S"
+              (Info-build-node-completions (Info-find-file file1 nil t)))
              string pred))
          nodename predicate code))))
     ;; Otherwise use Info-read-node-completion-table.

In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, 
cairo version 1.16.0)
  of 2021-02-09, modified by Debian built on 3df710f593d9
Repository revision: b0229d4bbaea7fcddffced393512c650212830db
Repository branch: deb/emacs/d/sid/master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)

Recent messages:
Mark set [2 times]
Auto-saving...done
Mark set [3 times]
next-line: End of buffer
Mark set
Icomplete mode enabled
Error in post-command-hook (icomplete-post-command-hook): (user-error 
"Info file emacs does not exist")
Quit

bug#47771: 27.1; `Info-read-node-name` throws error during completion

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> 1. M-x icomplete-mode
> 2. Press "C-h i"
> 3. Press "g" to run `Info-goto-node`
> 4. Enter "(boom)"
> 5. Move around with the cursor left to right or wait until the
> `icomplete-post-command-hook` fails.
>
> In order to fix the issue, the following patch can be applied:
>
> diff --git a/info.el b/info-patched.el
> index 3bed743..42a7fce 100644
> --- a/info.el
> +++ b/info-patched.el
> @@ -1881,7 +1881,8 @@ See `completing-read' for a description of
> arguments and usage."
>           (lambda (string pred action)
>             (complete-with-action
>              action
> -            (Info-build-node-completions (Info-find-file file1 nil t))
> +            (let ((file2 (Info-find-file file1 'noerror t)))
> +              (and file2 (Info-build-node-completions file2)))

Thanks; applied to Emacs 28 (with some minor changes).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47771: 27.1; `Info-read-node-name` throws error during completion

[Daniel Mendler]

On 5/5/21 4:59 PM, Lars Ingebrigtsen wrote:
> Thanks; applied to Emacs 28 (with some minor changes).

Thank you. There is one additional issue I missed in the bug report. If
you call `Info-build-node-completions` with a file without info tags, it
will also throw an error. So maybe it would be better to wrap
`(Info-build-node-completions (Info-find-file file1 nil t))` with
`ignore-errors`?

Daniel

bug#47771: 27.1; `Info-read-node-name` throws error during completion

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> Thank you. There is one additional issue I missed in the bug report. If
> you call `Info-build-node-completions` with a file without info tags, it
> will also throw an error. So maybe it would be better to wrap
> `(Info-build-node-completions (Info-find-file file1 nil t))` with
> `ignore-errors`?

Slapping an `ignore-errors' around all that code feels a bit excessive
(it makes debugging hard), so I've instead made
`Info-build-node-completions' a bit more resilient -- it'll catch errors
from `Info-goto-node' instead.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47771: 27.1; `Info-read-node-name` throws error during completion

[Daniel Mendler]

On 5/6/21 11:26 AM, Lars Ingebrigtsen wrote:
> Slapping an `ignore-errors' around all that code feels a bit excessive
> (it makes debugging hard), so I've instead made
> `Info-build-node-completions' a bit more resilient -- it'll catch errors
> from `Info-goto-node' instead.

I agree with you, the approach you took is better than the
`ignore-errors` sledgehammer. For the same reasons my initial proposal
was to pass noerror to `Info-find-file`. Thank you for applying the fixes!

Daniel