Re: Setting `minibuffer-completion-table` buffer-locally

[Gregory Heytings <gregory@heytings.org>]

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


>
> There's a "competing" patch from Gregory which does something similar 
> but in a slightly different way (less clean but potentially a bit more 
> backward compatible), and I think in order to decide which is the best 
> approach we need to have a more concrete information about the risk of 
> breakage and what we might need to do to minimize it.
>

In the interest of fairness in the discussion and comparison, and given 
the length of the discussion in bug#45474, here is the "competing" patch.

The five major differences between the two approaches are (m-c-* stands 
for the minibuffer-completion-* variables, r-f-m for read-from-minibuffer, 
and m-w-s-h for minibuffer-with-setup-hook):

(1) SM: m-c-* variables are passed explicitly as arguments to r-f-m with a 
m-w-s-h, they are stricly buffer-local

GH: m-c-* variables are passed implicitly as arguments to r-f-m, they 
automatically become buffer-local upon entering the minibuffer

(2) SM: it is and it will remain necessary to use a m-w-s-h with 
setq-locals to have buffer-local m-c-* variables in r-f-m

GH: for Emacs 28 it is necessary to let-bind minibuffer-local-completion 
to t around the call to r-f-m (to preserve backward compatibility), for 
Emacs 29 and above minibuffer-local-completion t will be assumed (or, with 
the strong version: the only possible behavior)

(3) SM: when m-c-* variables are strictly buffer-local (i.e., when they 
are passed in a m-w-s-h to r-f-m), they cannot interfere with code outside 
of the minibuffer

GH: because m-c-* variables become buffer-local only upon entering the 
minibuffer, they can possibly interfere with code that is executed around 
the call to r-f-m, between the moment they have been let-bound and the 
moment they become buffer-local

(4) SM: the semantics of r-f-m is not changed

GH: the semantics of r-f-m is changed, in Emacs 28 the m-c-* variables 
become buffer-local when minibuffer-local-completion is t, in Emacs 29 
they become buffer-local unless minibuffer-local-completion is nil (or, 
with the strong version: they become buffer-local)

(5) SM: the patch fixes the behavior of completing-read-default

GH: the patch fixes the behavior of completing-read-default and makes, in 
the medium term, that new behavior the default one for all uses of r-f-m

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

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..584afd5d04 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,23 @@ 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.
+
+[STRONG VERSION:] Setting this variable in Emacs 29 will have no effect;
+the value t will be assumed.
+
+[WEAK VERSION:] The default will be t for Emacs 29.
+
+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.  */);