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

[Gregory Heytings <gregory@heytings.org>]

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


>> 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.
>
> Ah, I see.  But that's based on "blacklisting" those places that don't 
> want to use minibuffer-completion-table, so it requires changes in many 
> places (including outside of Emacs's codebase).
>

It would be possible to use whitelisting instead by renaming the current 
read-from-minibuffer to internal-read-from-minibuffer, which would be 
wrapped in a macro read-from-minibuffer.  The change would be transparent, 
except for those places (e.g. completing-read-default) where what we 
actually want is to use internal-read-from-minibuffer.  But this change 
would be slightly more invasive than what follows.

>> 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.
>
> The patch below shows one way to do what I suggest.
>

Thanks.  Somehow I feel that using the keymap to decide whether the 
completion table should be used isn't safe enough, it's possible (at least 
in theory) that a minibuffer with a certain keymap uses completion tables 
and another one using the same keymap does not.  ISTM that it's safer (and 
more explicit) to use the current minibuffer depth for that purpose; see 
attached patch.

>
> Just like your approach, I think this is only a temporary fix until we 
> can solve the problem for real by making `minibuffer-completion-table` 
> buffer-local
>

I'm not sure I fully understand why this is necessary, but is 
"Fmake_variable_buffer_local (Qminibuffer_completion_table);" just after 
"if ... specbind (Qminibuffer_completion_table, Qnil);" not enough for 
this?

[-- Attachment #2: Type: text/x-diff, Size: 2918 bytes --]

From 4654c5251cab6703e4dcd94ecaa900bb970bb589 Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sun, 18 Apr 2021 22:15:08 +0000
Subject: [PATCH] Make it possible to disable completion in recursive
 minibuffers

---
 lisp/minibuffer.el |  1 +
 src/minibuf.c      | 15 +++++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index c900b0d7ce..991ed9dc93 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3825,6 +3825,7 @@ completing-read-default
                 (1+ (cdr initial-input)))))
 
   (let* ((minibuffer-completion-table collection)
+         (minibuffer--depth-for-completion-table (minibuffer-depth))
          (minibuffer-completion-predicate predicate)
          ;; FIXME: Remove/rename this var, see the next one.
          (minibuffer-completion-confirm (unless (eq require-match t)
diff --git a/src/minibuf.c b/src/minibuf.c
index c9831fd50f..77992e1fea 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -559,6 +559,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
 
+  if (!EQ (Vminibuffer__depth_for_completion_table, make_fixnum (minibuf_level)))
+    specbind (Qminibuffer_completion_table, Qnil);
+
   /* If Vminibuffer_completing_file_name is `lambda' on entry, it was t
      in previous recursive minibuffer, but was not set explicitly
      to t for this invocation, so set it to nil in this minibuffer.
@@ -1338,12 +1341,6 @@ DEFUN ("read-string", Fread_string, Sread_string, 1, 5, 0,
   Lisp_Object val;
   ptrdiff_t count = SPECPDL_INDEX ();
 
-  /* Just in case we're in a recursive minibuffer, make it clear that the
-     previous minibuffer's completion table does not apply to the new
-     minibuffer.
-     FIXME: `minibuffer-completion-table' should be buffer-local instead.  */
-  specbind (Qminibuffer_completion_table, Qnil);
-
   val = Fread_from_minibuffer (prompt, initial_input, Qnil,
 			       Qnil, history, default_value,
 			       inherit_input_method);
@@ -2394,6 +2391,12 @@ syms_of_minibuf (void)
 variable is non-nil. */);
   enable_recursive_minibuffers = 0;
 
+  DEFVAR_LISP ("minibuffer--depth-for-completion-table", Vminibuffer__depth_for_completion_table,
+               doc: /* Minibuffer depth used together with `minibuffer-completion-table'.
+`read-from-minibuffer' compares it with the current minibuffer depth to
+determine if the completion table was intended for it or not.  */);
+  Vminibuffer__depth_for_completion_table = Qnil;
+
   DEFVAR_LISP ("minibuffer-completion-table", Vminibuffer_completion_table,
 	       doc: /* Alist or obarray used for completion in the minibuffer.
 This becomes the ALIST argument to `try-completion' and `all-completions'.
-- 
2.30.2