bug#45474: Icomplete exhibiting in recursive minibuffer when it shouldn’t

[Gregory Heytings <gregory@heytings.org>]

[-- Attachment #1: Type: text/plain, Size: 1842 bytes --]


>> What's wrong with my approach, which disables the completion backend on 
>> demand?
>
> [ I'm not sure which is "your approach", sorry.  ]
>

See the attached patch.  I's a draft, as I said 
read-from-minibuffer-simple could probably renamed to something more 
elegant, and (at least some of) the other calls to read-from-minibuffer 
could be replaced by the macro.

>> A variant of it would be to add an eight argument to 
>> read-from-minibuffer.  AFAICS it's only the caller that can know 
>> whether the completion backend should be used, IOW, the only thing that 
>> the completion backend can do is to obey the caller.
>
> We should think hard before adding yet another argument.
>

Yes ;-)  Which is why I did not do that.

>
> I agree that the current design is problematic.  Basically, I think that 
> `minibuffer-completion-table` should be set buffer-locally in the 
> minibuffer instead of being "set" by dynamic scoping.
>
> Fixing the problem "right" might call for a significant redesign of 
> `read-from-minibuffer`s API and `completing-read`s implementation.
>
> A quick&dirty workaround for now would be for `completing-read` to set 
> some var alongside `minibuffer-completion-table` which indicates *which* 
> minibuffer this is for (e.g. some minibuffer-depth information). This 
> way `read-from-minibuffer` could distinguish a 
> `minibuffer-completion-table` passed for the current invocation from one 
> passed for the outer invocation and could hence temporarily rebind 
> `minibuffer-completion-table` to nil.
>
> Another approach would be for `completing-read` not to let-bind those 
> vars but instead to use `minibuffer-setup-hook` to set the vars 
> buffer-locally.
>

These are yet other possible approaches indeed, but it seems to me at 
first sight that they are more complex than the solution I suggest.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=Make-it-possible-to-disable-a-completion-backend-in-.patch, Size: 4780 bytes --]

From 99050c96e7ff522aa1b180920fac74e98a2d6c79 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Thu, 15 Apr 2021 23:50:24 +0000
Subject: [PATCH] Make it possible to disable a completion backend in recursive
 minibuffers

* lisp/subr.el (read-from-minibuffer-simple): New macro.
(disable-for-read-from-minibuffer-simple): New macro.
(read-from-minibuffer-simple): New internal variable.
(read-number, read-char-from-minibuffer, y-or-n-p): Use the new macro

* lisp/simple.el (read--expression): Use the new macro.

* lisp/icomplete.el (icomplete-minibuffer-setup): Use the new macro.
---
 lisp/icomplete.el |  1 +
 lisp/simple.el    |  6 +++---
 lisp/subr.el      | 28 ++++++++++++++++++++++++----
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index d5b6f76d7b..76313eb91d 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -446,6 +446,7 @@ icomplete-simple-completing-p
 (defun icomplete-minibuffer-setup ()
   "Run in minibuffer on activation to establish incremental completion.
 Usually run by inclusion in `minibuffer-setup-hook'."
+  (disable-for-read-from-minibuffer-simple icomplete-mode)
   (when (and icomplete-mode (icomplete-simple-completing-p))
     (setq-local icomplete--initial-input (icomplete--field-string))
     (setq-local completion-show-inline-help nil)
diff --git a/lisp/simple.el b/lisp/simple.el
index 999755a642..7a0953a939 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1755,9 +1755,9 @@ read--expression
           (add-hook 'completion-at-point-functions
                     #'elisp-completion-at-point nil t)
           (run-hooks 'eval-expression-minibuffer-setup-hook))
-      (read-from-minibuffer prompt initial-contents
-                            read-expression-map t
-                            'read-expression-history))))
+      (read-from-minibuffer-simple prompt initial-contents
+                                   read-expression-map t
+                                   'read-expression-history))))
 
 (defun read--expression-try-read ()
   "Try to read an Emacs Lisp expression in the minibuffer.
diff --git a/lisp/subr.el b/lisp/subr.el
index c2be26a15f..d2230f6034 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2791,6 +2791,26 @@ read-passwd
               ;; And of course, don't keep the sensitive data around.
               (erase-buffer))))))))
 
+(defvar read-from-minibuffer-simple nil
+  "Whether `read-from-minibuffer' should display completion candidates.")
+
+(defmacro read-from-minibuffer-simple (&rest body)
+  "Read a string from the minibuffer without completion.
+Set `read-from-minibuffer-simple' to t, and call `read-from-minibuffer',
+which see."
+  `(progn
+     (setq read-from-minibuffer-simple t)
+     (read-from-minibuffer ,@body)))
+
+(defmacro disable-for-read-from-minibuffer-simple (completion-mode)
+  "Set COMPLETION-MODE to nil for `read-from-minibuffer-simple'.
+This is meant to be used by completion backends to setup the minibuffer
+to conditionally activate completion in each recursive invocation of
+the minibuffer, without affecting other recursive invocations."
+  `(when read-from-minibuffer-simple
+     (setq-local ,completion-mode nil)
+     (setq read-from-minibuffer-simple nil)))
+
 (defvar read-number-history nil
   "The default history for the `read-number' function.")
 
@@ -2812,7 +2832,7 @@ read-number
 					prompt t t))))
     (while
 	(progn
-	  (let ((str (read-from-minibuffer
+	  (let ((str (read-from-minibuffer-simple
 		      prompt nil nil nil (or hist 'read-number-history)
 		      (when default
 			(if (consp default)
@@ -3052,8 +3072,8 @@ read-char-from-minibuffer
          ;; Protect this-command when called from pre-command-hook (bug#45029)
          (this-command this-command)
          (result
-          (read-from-minibuffer prompt nil map nil
-                                (or history 'empty-history)))
+          (read-from-minibuffer-simple prompt nil map nil
+                                       (or history 'empty-history)))
          (char
           (if (> (length result) 0)
               ;; We have a string (with one character), so return the first one.
@@ -3247,7 +3267,7 @@ y-or-n-p
                        map))
              ;; Protect this-command when called from pre-command-hook (bug#45029)
              (this-command this-command)
-             (str (read-from-minibuffer
+             (str (read-from-minibuffer-simple
                    prompt nil keymap nil
                    (or y-or-n-p-history-variable 'empty-history))))
         (setq answer (if (member str '("y" "Y")) 'act 'skip)))))
-- 
2.30.2