Setting `minibuffer-completion-table` buffer-locally

[Stefan Monnier]

Following a long discussion in bug#45474, I'm warming up to the idea to
install the patch below which changes `completing-read(-default)` such
that it sets the `minibuffer-completion-*` variables buffer-locally in
the mini-buffer instead of let-binding them globally.

I've had such a change in the back of my mind for a while, and the
discussion around bug#45474 convinced me that setting the vars
buffer-locally is the only right solution.  It's also necessary in order
for the multiple active minibuffers that can now be displayed
simultaneously to work correctly.

I was a bit scared of the potential for breaking existing packages,
since this is an incompatible change, but my analysis of all the code
I could find in MELPA and GNU ELPA suggests that there should be very
little breakage, if any.

It worked fine in my tests with the default UI, Icomplete, Vertico,
and Ivy.

If people could test it (and report here) against their favorite funky
completion package with their personal configuration (Helm, Ivy,
Selectrum, Icicles, Younameit, ...) I'd be most welcome.

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.


        Stefan


diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index 7cf2fcf68f2..2d284f5b85d 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -1188,9 +1188,9 @@ Completion Commands
 @defvar minibuffer-completion-table
 The value of this variable is the completion table (@pxref{Basic
 Completion}) used for completion in the minibuffer.  This is the
-global variable that contains what @code{completing-read} passes to
+buffer-local variable that contains what @code{completing-read} passes to
 @code{try-completion}.  It is used by minibuffer completion commands
-such as @code{minibuffer-complete-word}.
+such as @code{minibuffer-complete}.
 @end defvar
 
 @defvar minibuffer-completion-predicate
@@ -1201,7 +1201,7 @@ Completion Commands
 
 @defvar minibuffer-completion-confirm
 This variable determines whether Emacs asks for confirmation before
-exiting the minibuffer; @code{completing-read} binds this variable,
+exiting the minibuffer; @code{completing-read} sets this variable,
 and the function @code{minibuffer-complete-and-exit} checks the value
 before exiting.  If the value is @code{nil}, confirmation is not
 required.  If the value is @code{confirm}, the user may exit with an
diff --git a/etc/NEWS b/etc/NEWS
index 7d600eb374d..709c74b38df 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2427,6 +2427,11 @@ This is to keep the same behavior as Eshell.
 
 * Incompatible Lisp Changes in Emacs 28.1
 
++++
+** 'completing-read-default' sets completion variables buffer-locally.
+'minibuffer-completion-table' and related variables are now set buffer-locally
+in the minibuffer instead of being set via a global let-binding.
+
 +++
 ** The use of positional arguments in 'define-minor-mode' is obsolete.
 These were actually rendered obsolete in Emacs-21 but were never
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 51e0519d489..2eafc12d882 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3884,13 +3884,7 @@ completing-read-default
                 ;; `read-from-minibuffer' uses 1-based index.
                 (1+ (cdr initial-input)))))
 
-  (let* ((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)
-                                          require-match))
-         (minibuffer--require-match require-match)
-         (base-keymap (if require-match
+  (let* ((base-keymap (if require-match
                          minibuffer-local-must-match-map
                         minibuffer-local-completion-map))
          (keymap (if (memq minibuffer-completing-file-name '(nil lambda))
@@ -3903,8 +3897,17 @@ completing-read-default
                     ;; in minibuffer-local-filename-completion-map can
                     ;; override bindings in base-keymap.
                     base-keymap)))
-         (result (read-from-minibuffer prompt initial-input keymap
-                                       nil hist def inherit-input-method)))
+         (result
+          (minibuffer-with-setup-hook
+              (lambda ()
+                (setq-local minibuffer-completion-table collection)
+                (setq-local minibuffer-completion-predicate predicate)
+                ;; FIXME: Remove/rename this var, see the next one.
+                (setq-local minibuffer-completion-confirm
+                            (unless (eq require-match t) require-match))
+                (setq-local minibuffer--require-match require-match))
+            (read-from-minibuffer prompt initial-input keymap
+                                  nil hist def inherit-input-method))))
     (when (and (equal result "") def)
       (setq result (if (consp def) (car def) def)))
     result))