Subject: bug#48925: [PATCH] Set `minibuffer-completion-*` variables buffer-locally in a few more places

From: [email-masked]
Date: Tue, 08 Jun 2021 20:30:29 +0200

This follows up on changes proposed in bug#45474. The second patch is a
bit more controversial, but is probably required if we want more
reliable usage of completion commands in non-innermost minibuffers (that
is, with minibuffer-follows-selected-frame set to nil.)
From 049d57e6d10edca1d6a0af119f557e364d8ea93f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <[email-masked]>
Date: Tue, 8 Jun 2021 20:17:59 +0200
Subject: [PATCH 1/2] Set `minibuffer-completion-*` variables locally in more
 places

Follow-up to commit
2021-05-01 "* lisp/minibuffer.el (completing-read-default): Fix bug#45474"

* lisp/calc/calc-store.el (calc-read-var-name):
* lisp/emacs-lisp/crm.el (completing-read-multiple):
* lisp/progmodes/cc-styles.el (c-read-offset):
* lisp/window.el (read-buffer-to-switch):
Set `minibuffer-completion-*` variables buffer-locally instead of
using a global let-binding.
---
 lisp/calc/calc-store.el     | 15 +++++++-----
 lisp/emacs-lisp/crm.el      | 47 ++++++++++++++++++-------------------
 lisp/progmodes/cc-styles.el | 12 ++++++----
 lisp/window.el              |  2 +-
 4 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/lisp/calc/calc-store.el b/lisp/calc/calc-store.el
index ee29c440fe..d96b40156d 100644
--- a/lisp/calc/calc-store.el
+++ b/lisp/calc/calc-store.el
@@ -188,12 +188,15 @@ calc-read-var-name
   (let* ((calc-store-opers store-opers)
          (var (concat
               "var-"
-              (let ((minibuffer-completion-table
-                     (mapcar (lambda (x) (substring x 4))
-                             (all-completions "var-" obarray)))
-                    (minibuffer-completion-predicate
-                     (lambda (x) (boundp (intern (concat "var-" x)))))
-                    (minibuffer-completion-confirm t))
+              (minibuffer-with-setup-hook
+                  (lambda ()
+                    (setq-local minibuffer-completion-table
+                                (mapcar (lambda (x) (substring x 4))
+                                        (all-completions "var-" obarray)))
+                    (setq-local minibuffer-completion-predicate
+                                (lambda (x)
+                                  (boundp (intern (concat "var-" x)))))
+                    (setq-local minibuffer-completion-confirm t))
                 (read-from-minibuffer
                  prompt nil calc-var-name-map nil
                  'calc-read-var-name-history)))))
diff --git a/lisp/emacs-lisp/crm.el b/lisp/emacs-lisp/crm.el
index e106815817..67464bc6db 100644
--- a/lisp/emacs-lisp/crm.el
+++ b/lisp/emacs-lisp/crm.el
@@ -245,30 +245,29 @@ completing-read-multiple
 
 This function returns a list of the strings that were read,
 with empty strings removed."
-  (unwind-protect
-      (progn
-	(add-hook 'choose-completion-string-functions
-		  'crm--choose-completion-string)
-	(let* ((minibuffer-completion-table #'crm--collection-fn)
-	       (minibuffer-completion-predicate predicate)
-	       ;; see completing_read in src/minibuf.c
-	       (minibuffer-completion-confirm
-		(unless (eq require-match t) require-match))
-	       (crm-completion-table table)
-	       (map (if require-match
-			crm-local-must-match-map
-		      crm-local-completion-map))
-	       ;; If the user enters empty input, `read-from-minibuffer'
-	       ;; returns the empty string, not DEF.
-	       (input (read-from-minibuffer
-		       prompt initial-input map
-		       nil hist def inherit-input-method)))
-	  (when (and def (string-equal input ""))
-	    (setq input (if (consp def) (car def) def)))
-          ;; Remove empty strings in the list of read strings.
-	  (split-string input crm-separator t)))
-    (remove-hook 'choose-completion-string-functions
-		 'crm--choose-completion-string)))
+  (let* ((map (if require-match
+                  crm-local-must-match-map
+                crm-local-completion-map))
+         input)
+    (minibuffer-with-setup-hook
+        (lambda ()
+          (add-hook 'choose-completion-string-functions
+                    'crm--choose-completion-string nil 'local)
+          (setq-local minibuffer-completion-table #'crm--collection-fn)
+          (setq-local minibuffer-completion-predicate predicate)
+          ;; see completing_read in src/minibuf.c
+          (setq-local minibuffer-completion-confirm
+                      (unless (eq require-match t) require-match))
+          (setq-local crm-completion-table table))
+      (setq input (read-from-minibuffer
+                   prompt initial-input map
+                   nil hist def inherit-input-method)))
+    ;; If the user enters empty input, `read-from-minibuffer'
+    ;; returns the empty string, not DEF.
+    (when (and def (string-equal input ""))
+      (setq input (if (consp def) (car def) def)))
+    ;; Remove empty strings in the list of read strings.
+    (split-string input crm-separator t)))
 
 ;; testing and debugging
 ;; (defun crm-init-test-environ ()
diff --git a/lisp/progmodes/cc-styles.el b/lisp/progmodes/cc-styles.el
index 8514434e9a..873682043c 100644
--- a/lisp/progmodes/cc-styles.el
+++ b/lisp/progmodes/cc-styles.el
@@ -444,17 +444,19 @@ c-read-offset
 			  defstr))
 	 (prompt (concat symname " offset " defstr))
 	 (keymap (make-sparse-keymap))
-	 (minibuffer-completion-table obarray)
-	 (minibuffer-completion-predicate 'fboundp)
 	 offset input)
     ;; In principle completing-read is used here, but SPC is unbound
     ;; to make it less annoying to enter lists.
     (set-keymap-parent keymap minibuffer-local-completion-map)
     (define-key keymap " " 'self-insert-command)
     (while (not offset)
-      (setq input (read-from-minibuffer prompt nil keymap t
-					'c-read-offset-history
-					(format "%s" oldoff)))
+      (minibuffer-with-setup-hook
+          (lambda ()
+            (setq-local minibuffer-completion-table obarray)
+            (setq-local minibuffer-completion-predicate 'fboundp))
+        (setq input (read-from-minibuffer prompt nil keymap t
+                                          'c-read-offset-history
+                                          (format "%s" oldoff))))
       (if (c-valid-offset input)
 	  (setq offset input)
 	;; error, but don't signal one, keep trying
diff --git a/lisp/window.el b/lisp/window.el
index fd1c617d6b..029202e350 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8376,7 +8376,7 @@ read-buffer-to-switch
   (let ((rbts-completion-table (internal-complete-buffer-except)))
     (minibuffer-with-setup-hook
         (lambda ()
-          (setq minibuffer-completion-table rbts-completion-table)
+          (setq-local minibuffer-completion-table rbts-completion-table)
           ;; Since rbts-completion-table is built dynamically, we
           ;; can't just add it to the default value of
           ;; icomplete-with-completion-tables, so we add it
-- 
2.31.1

From 466169b9f679a78aec00f9735335d90718c0d898 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <[email-masked]>
Date: Tue, 8 Jun 2021 20:19:44 +0200
Subject: [PATCH 2/2] Don't bind minibuffer-completion-table to nil in
 read-string

This reverts
2012-06-19 "* src/minibuf.c (Fread_string): Bind minibuffer-completion-table."

* src/minibuf.c (Fread_string): Don't bind minibuffer-completion-table
to nil.
---
 src/minibuf.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/minibuf.c b/src/minibuf.c
index 00069eabbe..adee471887 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1376,20 +1376,13 @@ DEFUN ("read-string", Fread_string, Sread_string, 1, 5, 0,
   (Lisp_Object prompt, Lisp_Object initial_input, Lisp_Object history, Lisp_Object default_value, Lisp_Object inherit_input_method)
 {
   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);
   if (STRINGP (val) && SCHARS (val) == 0 && ! NILP (default_value))
     val = CONSP (default_value) ? XCAR (default_value) : default_value;
-  return unbind_to (count, val);
+  return val;
 }
 
 DEFUN ("read-command", Fread_command, Sread_command, 1, 2, 0,
-- 
2.31.1