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

[miha]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> This follows up on changes proposed in bug#45474.
>>
>> Thanks, the first patch looks good to me (assuming it works ;-).
>
> So I've now applied it to Emacs 29.  It didn't lead to any obvious
> regressions (or test suite failures) that I can see, which is a good
> sign.
>
>>> 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.)
>>
>> The patch is fundamentally right, but as you say it's a bit more
>> controversial because it risks exposing bugs.  Hmm...
>>
>> To be on the safer side, I guess we could replace the
>>
>>     specbind (Qminibuffer_completion_table, Qnil);
>>
>> with a use of `minibuffer-with-setup-hook` that sets the var to nil in
>> the new minibuffer.  But doing it in C is awkward so it would best be
>> done by moving the function to subr.el.
>
> Sounds like a good idea to me.  Miha, could you do that?

Okay, patch attached.

From 70fb493398d4961be0fa997684261554822e66b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Rihtar=C5=A1i=C4=8D?= <miha@kamnitnik.top>
Date: Thu, 11 Nov 2021 11:38:03 +0100
Subject: [PATCH] Set minibuffer-completion-table buffer-locally in read-string

* src/callint.c (Fcall_interactively):
* src/minibuf.c (Fread_string): Move function subr.el and use
minibuffer-with-setup-hook to set minibuffer-completion-table
buffer-locally.
---
 lisp/subr.el  | 26 ++++++++++++++++++++++++++
 src/callint.c | 10 ++++------
 src/minibuf.c | 35 -----------------------------------
 3 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 5a5842d428..75f00f33d4 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -3507,6 +3507,32 @@ y-or-n-p
         (message "%s%c" prompt (if ret ?y ?n)))
       ret)))
 
+(defun read-string ( prompt &optional initial-input history
+                     default-value inherit-input-method)
+  "Read a string from the minibuffer, prompting with string PROMPT.
+If non-nil, second arg INITIAL-INPUT is a string to insert before reading.
+  This argument has been superseded by DEFAULT-VALUE and should normally be nil
+  in new code.  It behaves as INITIAL-CONTENTS in `read-from-minibuffer' (which
+  see).
+The third arg HISTORY, if non-nil, specifies a history list
+  and optionally the initial position in the list.
+See `read-from-minibuffer' for details of HISTORY argument.
+Fourth arg DEFAULT-VALUE is the default value or the list of default values.
+ If non-nil, it is used for history commands, and as the value (or the first
+ element of the list of default values) to return if the user enters the
+ empty string.
+Fifth arg INHERIT-INPUT-METHOD, if non-nil, means the minibuffer inherits
+ the current input method and the setting of `enable-multibyte-characters'."
+  (minibuffer-with-setup-hook
+      (lambda ()
+        (setq-local minibuffer-completion-table nil))
+    (let ((ret (read-from-minibuffer prompt initial-input nil nil
+                                     history default-value
+                                     inherit-input-method)))
+      (if (and default-value (equal "" ret))
+          (if (consp default-value) (car default-value) default-value)
+        ret))))
+

 ;;; Atomic change groups.
 
diff --git a/src/callint.c b/src/callint.c
index 44dae361c1..4e80d510ce 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -631,8 +631,8 @@ DEFUN ("call-interactively", Fcall_interactively, 
Scall_interactively, 1, 3, 0,
 
        case 'M':               /* String read via minibuffer with
                                   inheriting the current input method.  */
-         args[i] = Fread_string (callint_message,
-                                 Qnil, Qnil, Qnil, Qt);
+         args[i] = call5 (intern ("read-string"),
+                          callint_message, Qnil, Qnil, Qnil, Qt);
          break;
 
        case 'N':     /* Prefix arg as number, else number from minibuffer.  */
@@ -672,13 +672,11 @@ DEFUN ("call-interactively", Fcall_interactively, 
Scall_interactively, 1, 3, 0,
 
        case 's':               /* String read via minibuffer without
                                   inheriting the current input method.  */
-         args[i] = Fread_string (callint_message,
-                                 Qnil, Qnil, Qnil, Qnil);
+         args[i] = call1 (intern ("read-string"), callint_message);
          break;
 
        case 'S':               /* Any symbol.  */
-         visargs[i] = Fread_string (callint_message,
-                                    Qnil, Qnil, Qnil, Qnil);
+         visargs[i] = call1 (intern ("read-string"), callint_message);
          args[i] = Fintern (visargs[i], Qnil);
          break;
 
diff --git a/src/minibuf.c b/src/minibuf.c
index 6c0cd358c5..f0f08d97c0 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1366,40 +1366,6 @@ DEFUN ("read-from-minibuffer", Fread_from_minibuffer,
 
 /* Functions that use the minibuffer to read various things.  */
 
-DEFUN ("read-string", Fread_string, Sread_string, 1, 5, 0,
-       doc: /* Read a string from the minibuffer, prompting with string PROMPT.
-If non-nil, second arg INITIAL-INPUT is a string to insert before reading.
-  This argument has been superseded by DEFAULT-VALUE and should normally be nil
-  in new code.  It behaves as INITIAL-CONTENTS in `read-from-minibuffer' (which
-  see).
-The third arg HISTORY, if non-nil, specifies a history list
-  and optionally the initial position in the list.
-See `read-from-minibuffer' for details of HISTORY argument.
-Fourth arg DEFAULT-VALUE is the default value or the list of default values.
- If non-nil, it is used for history commands, and as the value (or the first
- element of the list of default values) to return if the user enters the
- empty string.
-Fifth arg INHERIT-INPUT-METHOD, if non-nil, means the minibuffer inherits
- the current input method and the setting of `enable-multibyte-characters'.  
*/)
-  (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);
-}
-
 DEFUN ("read-command", Fread_command, Sread_command, 1, 2, 0,
        doc: /* Read the name of a command and return as a symbol.
 Prompt with PROMPT.  By default, return DEFAULT-VALUE or its first element
@@ -2513,7 +2479,6 @@ syms_of_minibuf (void)
   defsubr (&Sactive_minibuffer_window);
   defsubr (&Sset_minibuffer_window);
   defsubr (&Sread_from_minibuffer);
-  defsubr (&Sread_string);
   defsubr (&Sread_command);
   defsubr (&Sread_variable);
   defsubr (&Sinternal_complete_buffer);
-- 
2.33.1

signature.asc
Description: PGP signature