bug#72176: 30.0.60; icomplete-vertical-mode failed to work with Error

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

> I'm afraid you missed your mark, because AFAICT it is Stefan Monnier
> that's behind the root cause of this issue, which goes back way before
> Dmitry's change or mine :)

How do you dare to put in doubt the perfection of my code!

> The following diff fixes this for me (including the icomplete symptom),
> although I can't claim to fully understand completion--sifn-requote yet:
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 31c365bf850..d0eb6b43c80 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -3855,13 +3855,13 @@ completion--sifn-requote
>        ;; Second assumptions: If qpos is far from the end this can be a bit slow,
>        ;; so we speed it up by doing a first loop that skips a word at a time.
>        ;; This word-sized loop is careful not to cut in the middle of env-vars.
> -      (while (let ((boundary (string-match "\\(\\$+{?\\)?\\w+\\W*\\'" qstr)))
> -               (and boundary
> -                    (progn
> -                      (setq qprefix (substring qstr 0 boundary))
> -                      (string-prefix-p uprefix
> -                                       (substitute-in-file-name qprefix)))))
> -        (setq qstr qprefix))
> +      ;; (while (let ((boundary (string-match "\\(\\$+{?\\)?\\w+\\W*\\'" qstr)))
> +      ;;          (and boundary
> +      ;;               (progn
> +      ;;                 (setq qprefix (substring qstr 0 boundary))
> +      ;;                 (string-prefix-p uprefix
> +      ;;                                  (substitute-in-file-name qprefix)))))
> +      ;;   (setq qstr qprefix))
>        (let ((qpos (length qstr)))
>          (while (and (> qpos 0)
>                      (string-prefix-p uprefix

That's helpful.  So the "main assumption" is valid.

Indeed in your recipe the problem is that the while loop you comment out
(which implements the shortcut based on the "second assumption") ends up
throwing away the chunk we need.  This is by nature very hackish,
and there will still be cases where it back-fires.  Maybe indeed we
should just get rid of this shortcut, and maybe the resulting
performance is good enough.

Otherwise, the patch below should fix this particular occurrence.

BTW, while there's no doubt that the behavior displayed in your recipe
is undesirable, I consider it technically as "suboptimal" rather
than incorrect: the displayed completions are too verbose and confusing
but using them still ends up selecting the right file.
[ Which is why I changed the comment to clarify we want "the largest"
  QPOS: in your recipe we end up returning a valid QPOS but not the
  largest one.  ]


        Stefan


diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 9ad072daaf5..6fa04e9a062 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3556,7 +3556,7 @@ completion-file-name-table
     (file-error nil)))               ;PCM often calls with invalid directories.
 
 (defun completion--sifn-requote (upos qstr)
-  ;; We're looking for `qpos' such that:
+  ;; We're looking for (the largest) `qpos' such that:
   ;; (equal (substring (substitute-in-file-name qstr) 0 upos)
   ;;        (substitute-in-file-name (substring qstr 0 qpos)))
   ;; Big problem here: we have to reverse engineer substitute-in-file-name to
@@ -3586,11 +3586,12 @@ completion--sifn-requote
       ;; Main assumption: nothing after qpos should affect the text before upos,
       ;; so we can work our way backward from the end of qstr, one character
       ;; at a time.
-      ;; Second assumptions: If qpos is far from the end this can be a bit slow,
+      ;; Second assumption: If qpos is far from the end this can be a bit slow,
       ;; so we speed it up by doing a first loop that skips a word at a time.
       ;; This word-sized loop is careful not to cut in the middle of env-vars.
       (while (let ((boundary (string-match "\\(\\$+{?\\)?\\w+\\W*\\'" qstr)))
                (and boundary
+                    (not (string-match-p "/[/~]" qstr boundary))
                     (progn
                       (setq qprefix (substring qstr 0 boundary))
                       (string-prefix-p uprefix