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

[Gregory Heytings <gregory@heytings.org>]

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


>> Aren't these problems orthogonal to the problem at hand?  It seems to 
>> me that this is not different from the traditional way of passing 
>> arguments to a function; of course something unexpected can happen when 
>> they are evaluated, before the function is entered, but that's 
>> something outside the responsibility of the function.
>
> No, the problem is not "can someone change `minibuffer-completion-table` 
> before we get to its intended consumer" but "will the let-binding of 
> `minibuffer-completion-table` also affect code which was not the 
> intended consumer". This problem does not exist with 
> traditional/explicit argument passing.
>

Again, it seems to me that this problem is not directly related to the 
problem at hand.  If the caller of read-from-minibuffer is not careful 
enough and binds minibuffer-completion-* too far from the call, in such a 
way that other code is unexpectedly affected (or could unexpectedly be 
affected) by this binding, it's the responsibility of that caller to fix 
that problem.

Anyway, I attach a last version of my patch, in which all the dancing 
happens at the C level.  It is probably also possible to finally get rid 
of the 15 lines with the minibuffer-completing-file-name = Qlambda hack in 
read_minibuf().

[-- 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: 5859 bytes --]

From 33dcdd6ea992e88614baa77b42c3e53bf9f6a08a Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
Date: Sat, 24 Apr 2021 08:43:45 +0000
Subject: [PATCH] Make it possible to disable a completion backend in recursive
 minibuffers

---
 lisp/minibuffer.el |  3 ++-
 src/minibuf.c      | 49 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 7da3c39e6b..379dadef9d 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3884,7 +3884,8 @@ completing-read-default
                 ;; `read-from-minibuffer' uses 1-based index.
                 (1+ (cdr initial-input)))))
 
-  (let* ((minibuffer-completion-table collection)
+  (let* ((minibuffer-local-completion t)
+         (minibuffer-completion-table collection)
          (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 c4482d7f1e..d0b804cdff 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -68,6 +68,9 @@ Copyright (C) 1985-1986, 1993-2021 Free Software Foundation, Inc.
 /* Width of current mini-buffer prompt.  Only set after display_line
    of the line that contains the prompt.  */
 
+static Lisp_Object minibuffer_completion_table, minibuffer_completion_predicate,
+  minibuffer_completion_confirm;
+
 static ptrdiff_t minibuf_prompt_width;
 
 static Lisp_Object nth_minibuffer (EMACS_INT depth);
@@ -862,6 +865,16 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if (STRINGP (input_method) && !NILP (Ffboundp (Qactivate_input_method)))
     call1 (Qactivate_input_method, input_method);
 
+  if (! EQ (Vminibuffer_local_completion, Qnil)) {
+    Fmake_local_variable (Qminibuffer_completion_table);
+    Fset (Qminibuffer_completion_table, minibuffer_completion_table);
+    Fmake_local_variable (Qminibuffer_completion_predicate);
+    Fset (Qminibuffer_completion_predicate, minibuffer_completion_predicate);
+    Fmake_local_variable (Qminibuffer_completion_confirm);
+    Fset (Qminibuffer_completion_confirm, minibuffer_completion_confirm);
+    Vminibuffer_local_completion = Qnil;
+  }
+
   run_hook (Qminibuffer_setup_hook);
 
   /* Don't allow the user to undo past this point.  */
@@ -1291,6 +1304,7 @@ DEFUN ("read-from-minibuffer", Fread_from_minibuffer,
   (Lisp_Object prompt, Lisp_Object initial_contents, Lisp_Object keymap, Lisp_Object read, Lisp_Object hist, Lisp_Object default_value, Lisp_Object inherit_input_method)
 {
   Lisp_Object histvar, histpos, val;
+  ptrdiff_t count;
 
   barf_if_interaction_inhibited ();
 
@@ -1315,11 +1329,25 @@ DEFUN ("read-from-minibuffer", Fread_from_minibuffer,
   if (NILP (histpos))
     XSETFASTINT (histpos, 0);
 
+  count = SPECPDL_INDEX ();
+
+  if (! EQ (Vminibuffer_local_completion, Qnil)) {
+    minibuffer_completion_table = Vminibuffer_completion_table;
+    minibuffer_completion_predicate = Vminibuffer_completion_predicate;
+    minibuffer_completion_confirm = Vminibuffer_completion_confirm;
+    specbind (Qminibuffer_completion_table, Qnil);
+    specbind (Qminibuffer_completion_predicate, Qnil);
+    specbind (Qminibuffer_completion_confirm, Qnil);
+  }
+
   val = read_minibuf (keymap, initial_contents, prompt,
 		      !NILP (read),
 		      histvar, histpos, default_value,
 		      minibuffer_allow_text_properties,
 		      !NILP (inherit_input_method));
+
+  unbind_to (count, Qnil);
+
   return val;
 }
 
@@ -1345,11 +1373,9 @@ 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);
+  specbind (Qminibuffer_completion_predicate, Qnil);
+  specbind (Qminibuffer_completion_confirm, Qnil);
 
   val = Fread_from_minibuffer (prompt, initial_input, Qnil,
 			       Qnil, history, default_value,
@@ -2281,6 +2307,9 @@ syms_of_minibuf (void)
   Fset (Qminibuffer_default, Qnil);
 
   DEFSYM (Qminibuffer_completion_table, "minibuffer-completion-table");
+  DEFSYM (Qminibuffer_completion_predicate, "minibuffer-completion-predicate");
+  DEFSYM (Qminibuffer_completion_confirm, "minibuffer-completion-confirm");
+  DEFSYM (Qminibuffer_local_completion, "minibuffer-local-completion");
 
   staticpro (&last_minibuf_string);
 
@@ -2414,6 +2443,18 @@ syms_of_minibuf (void)
  completion commands listed in `minibuffer-confirm-exit-commands'.  */);
   Vminibuffer_completion_confirm = Qnil;
 
+  DEFVAR_LISP ("minibuffer-local-completion", Vminibuffer_local_completion,
+	       doc: /* Whether minibuffer completion elements should become buffer-local.
+The default is nil for Emacs 28.  Setting this variable in Emacs 29 will have no effect;
+the value t will be assumed.
+When t, `minibuffer-completion-table', `minibuffer-completion-predicate' and
+`minibuffer-completion-confirm' become buffer-local upon entering the minibuffer,
+and are nil in recursive invocations of the minibuffer, unless they have been let-bound
+to a value.
+When nil, their values is shared between the recursive invocations of the minibuffer,
+unless they have been let-bound to another value.  */);
+  Vminibuffer_local_completion = Qnil;
+
   DEFVAR_LISP ("minibuffer-completing-file-name",
 	       Vminibuffer_completing_file_name,
 	       doc: /* Non-nil means completing file names.  */);
-- 
2.30.2