bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

I was recently trying to modify `goto-line' so that it uses a
buffer-local variable as input history for the minibuffer (so that line
numbers don't get mixed up with other inputted values, and each buffer
has its own input history) (see: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38282).

First, I modified `read-number' and added a new HIST parameter. Then I
just forwarded that value to `read-from-minibuffer'.

Then, in simple.el, I added a new buffer-local variable
`goto-line--hist' and passed it to `read-number' in `goto-line'. I also
added I call to `add-to-history` to record the value read. All of this
did not work as expected: the input history for all buffers was shared
for all uses of `goto-line'. Inspecting `goto-line--hist' with M-:
showed the correct list of values, however.

The shortest way I've found of reproducing the problem is:

(defvar-local local-hist nil)

(defun my-command ()
  (interactive)
  (add-to-history 'local-hist (read-from-minibuffer "> " nil nil nil 'local-hist)))

In this case, the variable `local-hist' will end up containing all the
inputted values (per buffer), but trying to use M-p or M-n to step
through the input history after invoking `my-command' will not work ("no
preceding item"). So to summarize, `read-from-minibuffer' doesn't seem
to work correctly when HIST is buffer-local.

Related: https://emacs.stackexchange.com/questions/53892/buffer-local-input-history-for-read-from-minibuffer/53898#53898

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> In this case, the variable `local-hist' will end up containing all the
> inputted values (per buffer), but trying to use M-p or M-n to step
> through the input history after invoking `my-command' will not work ("no
> preceding item"). So to summarize, `read-from-minibuffer' doesn't seem
> to work correctly when HIST is buffer-local.

Looking at the code in read_minibuf, it does seem to access the
buffer-local value of the HIST variable, but my guess is that the buffer
it's looking at is the minibuffer?  And the variable isn't buffer-local
there.

There doesn't seem to be any way to tell read_minibuf what buffer it
should be looking at to get at the buffer-local value?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Looking at the code in read_minibuf, it does seem to access the
> buffer-local value of the HIST variable, but my guess is that the buffer
> it's looking at is the minibuffer?  And the variable isn't buffer-local
> there.

The problem is M-p: this command is called with the minibuffer current.
The helper `goto-history-element' calls (symbol-value
minibuffer-history-variable) and that returns the binding of the
variable in the minibuffer.  Adding to the buffer local history already
works as expected OTOH, so the behavior is inconsistent.

Dunno if there are more inconsistencies to expect when trying to fix
M-p.  My personal point of view is that when we can make it work without
risking breakage of anything else it would be nice to fix this.

Michael.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[martin rudalics]

 > There doesn't seem to be any way to tell read_minibuf what buffer it
 > should be looking at to get at the buffer-local value?

The one from the buffer of 'minibuffer-selected-window'?

martin

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> There doesn't seem to be any way to tell read_minibuf what buffer it
>> should be looking at to get at the buffer-local value?
>
> The one from the buffer of 'minibuffer-selected-window'?

Unconditionally looking at that buffer wouldn't work, I think?  Because
you may genuinely have a buffer-local version of the variable in the
minibuffer?  Or is that unlikely?

I guess that's quite unlikely.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

I've been looking into the problem, what I found so far is:

`read-from-minibuffer' works normally if HIST is not buffer-local.

If HIST is buffer-local, then the history variable doesn't seem to be
updated correctly after returning (so using M-p and M-n doesn't do
anything).

My test was:

(setq test-history nil)
(make-variable-buffer-local 'test-history)

(read-from-minibuffer "" nil nil nil 'test-history)

After running that and inputting "hello", I checked:

(buffer-local-value 'test-history (current-buffer))            ;; -> nil
(buffer-local-value 'test-history (get-buffer " *Minibuf-0*")) ;; -> nil
(buffer-local-value 'test-history (get-buffer " *Minibuf-1*")) ;; -> nil

So although `read_minibuf' does call `add-to-history' before returning
(and after setting the current buffer to be the minibuffer's), my input
wasn't added to any list (that I know of). This is strange, since
`add-to-history' seemed to work correctly with buffer-local variables
when I tested it separately. I would've expected the new history item to
be added to one of the minibuffer's `test-history' local variable.

So I think the first step would be to make `read_minibuf' add the new
history item into the right place when HIST is buffer-local.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

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

> So although `read_minibuf' does call `add-to-history' before returning
> (and after setting the current buffer to be the minibuffer's), my input
> wasn't added to any list (that I know of). This is strange, since
> `add-to-history' seemed to work correctly with buffer-local variables
> when I tested it separately. I would've expected the new history item to
> be added to one of the minibuffer's `test-history' local variable.
>
> So I think the first step would be to make `read_minibuf' add the new
> history item into the right place when HIST is buffer-local.

Turns out `get_minibuffer' sets the minibuffer's buffer mode to
`minibuffer-inactive-mode', which resets the minibuffer's local
variables (though not 100% sure about this). I think the history items
were being added to the minibuffer's local version of the variable, and
then being deleted by that.

I created a small patch that sets back the original buffer at the end of
`read_minibuf', so that the history item is added in the right
place. Then, I modified `goto-history-element' as Michael suggested, so
that the function reads the buffer-local version of the variable, but
not the minibuffer's (and updated `minibuffer-history-isearch-wrap' just
in case as well). With this, M-p and M-n works again.

I'm attaching a draft patch in case anyone wants to provide some
feedback. Thanks!

- Fede


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 3901 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index c61ccd511c..e5f9e9e785 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2028,7 +2028,7 @@ previous-matching-history-element
 	     (null minibuffer-text-before-history))
 	(setq minibuffer-text-before-history
 	      (minibuffer-contents-no-properties)))
-    (let ((history (symbol-value minibuffer-history-variable))
+    (let ((history (minibuffer-history-values))
 	  (case-fold-search
 	   (if (isearch-no-upper-case-p regexp t) ; assume isearch.el is dumped
 	       ;; On some systems, ignore case for file names.
@@ -2128,6 +2128,14 @@ minibuffer-default-add-completions
 	(append def all)
       (cons def (delete def all)))))
 
+(defun minibuffer-history-values ()
+  "Return the minibuffer input history values.
+If `minibuffer-history-variable' points to a buffer-local variable and
+the minibuffer is active, return the buffer-local value for the buffer
+selected in the window returned by `minibuffer-selected-window'."
+  (buffer-local-value minibuffer-history-variable
+                      (window-buffer (minibuffer-selected-window))))
+
 (defun goto-history-element (nabs)
   "Puts element of the minibuffer history in the minibuffer.
 The argument NABS specifies the absolute history position in
@@ -2156,8 +2164,8 @@ goto-history-element
 	(user-error (if minibuffer-default
                         "End of defaults; no next item"
                       "End of history; no default available")))
-    (if (> nabs (if (listp (symbol-value minibuffer-history-variable))
-                    (length (symbol-value minibuffer-history-variable))
+    (if (> nabs (if (listp (minibuffer-history-values))
+                    (length (minibuffer-history-values))
                   0))
 	(user-error "Beginning of history; no preceding item"))
     (unless (memq last-command '(next-history-element
@@ -2179,7 +2187,7 @@ goto-history-element
 	   (setq minibuffer-returned-to-present t)
 	   (setq minibuffer-text-before-history nil))
 	  (t (setq elt (nth (1- minibuffer-history-position)
-			    (symbol-value minibuffer-history-variable)))))
+			    (minibuffer-history-values)))))
     (insert
      (if (and (eq minibuffer-history-sexp-flag (minibuffer-depth))
 	      (not minibuffer-returned-to-present))
@@ -2432,7 +2440,7 @@ minibuffer-history-isearch-wrap
   ;; beginning/end of the history, wrap the search to the first/last
   ;; minibuffer history element.
   (if isearch-forward
-      (goto-history-element (length (symbol-value minibuffer-history-variable)))
+      (goto-history-element (length (minibuffer-history-values)))
     (goto-history-element 0))
   (setq isearch-success t)
   (goto-char (if isearch-forward (minibuffer-prompt-end) (point-max))))
diff --git a/src/minibuf.c b/src/minibuf.c
index 1e87c5044a..c4d2fae9b4 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -353,7 +353,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 	      Lisp_Object histvar, Lisp_Object histpos, Lisp_Object defalt,
 	      bool allow_props, bool inherit_input_method)
 {
-  Lisp_Object val;
+  Lisp_Object val, previous_buffer = Fcurrent_buffer ();
   ptrdiff_t count = SPECPDL_INDEX ();
   Lisp_Object mini_frame, ambient_dir, minibuffer, input_method;
   Lisp_Object enable_multibyte;
@@ -696,7 +696,12 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   else
     histstring = Qnil;
 
-  /* Add the value to the appropriate history list, if any.  */
+  /* Add the value to the appropriate history list, if any.  If
+   possible, switch back to the previous buffer first, in case the
+   history variable was buffer-local.  */
+  if (BUFFER_LIVE_P (XBUFFER (previous_buffer)))
+    Fset_buffer (previous_buffer);
+
   if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
     call2 (intern ("add-to-history"), Vminibuffer_history_variable, histstring);
 

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

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

> I'm attaching a draft patch in case anyone wants to provide some
> feedback. Thanks!

I'm now attaching a final version of my patch.

I've tested the following commands with it, using `read-from-minibuffer'
with a buffer-local HIST:

- previous-history-element (M-p)
- next-history-element (M-n)
- previous-matching-history-element (M-r)
- next-matching-history-element (M-s)
- isearch-forward (C-s)
- isearch-backward (C-r)

They've all worked correctly and used only the buffer's local history
value.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 5653 bytes --]

From 496fbc75e02e016d50d3e1a21154e4da844a1140 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Tue, 26 Nov 2019 22:39:34 +0100
Subject: [PATCH 1/1] Make HIST arg of read-from-minibuffer work with
 buffer-local vars

* lisp/simple.el (minibuffer-history-values): New function, should be
used to access the minibuffer input history variable when the
minibuffer might be active. If the variable is buffer-local, the
previous buffer's value will be used.
(goto-history-element): Use the new function to access the minibuffer
history.
(minibuffer-history-isearch-wrap): Use the new function to access the
minibuffer history.
* src/minibuf.c (read_minibuf): Switch to previous buffer temporarily
before updating history list (Bug#38317).
* etc/NEWS: Announce changes.
---
 etc/NEWS       |  6 ++++++
 lisp/simple.el | 18 +++++++++++++-----
 src/minibuf.c  | 17 +++++++++++++++--
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d3331daf17..9dbf36ae40 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -518,6 +518,12 @@ Note that this key binding will not work on MS-Windows systems if
 key binding with an upper case letter - if you can type it, you can
 bind it.
 
+---
+** 'read-from-minibuffer' now works with buffer-local history variables
+The HIST argument of 'read-from-minibuffer' now works correctly with
+buffer-local variables. This means that different buffers can have
+their own separated input history list if desired.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/lisp/simple.el b/lisp/simple.el
index 2aac557154..0b1d7b73ab 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2041,7 +2041,7 @@ previous-matching-history-element
 	     (null minibuffer-text-before-history))
 	(setq minibuffer-text-before-history
 	      (minibuffer-contents-no-properties)))
-    (let ((history (symbol-value minibuffer-history-variable))
+    (let ((history (minibuffer-history-values))
 	  (case-fold-search
 	   (if (isearch-no-upper-case-p regexp t) ; assume isearch.el is dumped
 	       ;; On some systems, ignore case for file names.
@@ -2141,6 +2141,14 @@ minibuffer-default-add-completions
 	(append def all)
       (cons def (delete def all)))))
 
+(defun minibuffer-history-values ()
+  "Return the minibuffer input history values.
+If `minibuffer-history-variable' points to a buffer-local variable and
+the minibuffer is active, return the buffer-local value for the buffer
+selected in the window returned by `minibuffer-selected-window'."
+  (buffer-local-value minibuffer-history-variable
+                      (window-buffer (minibuffer-selected-window))))
+
 (defun goto-history-element (nabs)
   "Puts element of the minibuffer history in the minibuffer.
 The argument NABS specifies the absolute history position in
@@ -2169,8 +2177,8 @@ goto-history-element
 	(user-error (if minibuffer-default
                         "End of defaults; no next item"
                       "End of history; no default available")))
-    (if (> nabs (if (listp (symbol-value minibuffer-history-variable))
-                    (length (symbol-value minibuffer-history-variable))
+    (if (> nabs (if (listp (minibuffer-history-values))
+                    (length (minibuffer-history-values))
                   0))
 	(user-error "Beginning of history; no preceding item"))
     (unless (memq last-command '(next-history-element
@@ -2192,7 +2200,7 @@ goto-history-element
 	   (setq minibuffer-returned-to-present t)
 	   (setq minibuffer-text-before-history nil))
 	  (t (setq elt (nth (1- minibuffer-history-position)
-			    (symbol-value minibuffer-history-variable)))))
+			    (minibuffer-history-values)))))
     (insert
      (if (and (eq minibuffer-history-sexp-flag (minibuffer-depth))
 	      (not minibuffer-returned-to-present))
@@ -2445,7 +2453,7 @@ minibuffer-history-isearch-wrap
   ;; beginning/end of the history, wrap the search to the first/last
   ;; minibuffer history element.
   (if isearch-forward
-      (goto-history-element (length (symbol-value minibuffer-history-variable)))
+      (goto-history-element (length (minibuffer-history-values)))
     (goto-history-element 0))
   (setq isearch-success t)
   (goto-char (if isearch-forward (minibuffer-prompt-end) (point-max))))
diff --git a/src/minibuf.c b/src/minibuf.c
index 1e87c5044a..2fbfb31b99 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -353,7 +353,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 	      Lisp_Object histvar, Lisp_Object histpos, Lisp_Object defalt,
 	      bool allow_props, bool inherit_input_method)
 {
-  Lisp_Object val;
+  Lisp_Object val, previous_buffer = Fcurrent_buffer ();
   ptrdiff_t count = SPECPDL_INDEX ();
   Lisp_Object mini_frame, ambient_dir, minibuffer, input_method;
   Lisp_Object enable_multibyte;
@@ -698,7 +698,20 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   /* Add the value to the appropriate history list, if any.  */
   if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
-    call2 (intern ("add-to-history"), Vminibuffer_history_variable, histstring);
+    {
+      ptrdiff_t count2 = SPECPDL_INDEX ();
+
+      /* If possible, switch back to the previous buffer first, in
+	 case the history variable is buffer-local.  */
+      if (BUFFER_LIVE_P (XBUFFER (previous_buffer)))
+	{
+	  record_unwind_current_buffer ();
+	  Fset_buffer (previous_buffer);
+	}
+
+      call2 (intern ("add-to-history"), Vminibuffer_history_variable, histstring);
+      unbind_to (count2, Qnil);
+    }
 
   /* If Lisp form desired instead of string, parse it.  */
   if (expflag)
-- 
2.17.1

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Robert Pluim]

>>>>> On Tue, 26 Nov 2019 22:54:29 +0100, Federico Tedin <federicotedin@gmail.com> said:

    >> I'm attaching a draft patch in case anyone wants to provide some
    >> feedback. Thanks!

    Federico> I'm now attaching a final version of my patch.

    Federico> I've tested the following commands with it, using `read-from-minibuffer'
    Federico> with a buffer-local HIST:

    Federico> - previous-history-element (M-p)
    Federico> - next-history-element (M-n)
    Federico> - previous-matching-history-element (M-r)
    Federico> - next-matching-history-element (M-s)
    Federico> - isearch-forward (C-s)
    Federico> - isearch-backward (C-r)

    Federico> They've all worked correctly and used only the buffer's local history
    Federico> value.

    Federico> Thanks!

    Federico> From 496fbc75e02e016d50d3e1a21154e4da844a1140 Mon Sep 17 00:00:00 2001
    Federico> From: Federico Tedin <federicotedin@gmail.com>
    Federico> Date: Tue, 26 Nov 2019 22:39:34 +0100
    Federico> Subject: [PATCH 1/1] Make HIST arg of read-from-minibuffer work with
    Federico>  buffer-local vars

    Federico> * lisp/simple.el (minibuffer-history-values): New function, should be
    Federico> used to access the minibuffer input history variable when the
    Federico> minibuffer might be active. If the variable is buffer-local, the
    Federico> previous buffer's value will be used.

2 spaces after '.' (here and elsewhere).

    Federico> (goto-history-element): Use the new function to access the minibuffer
    Federico> history.
    Federico> (minibuffer-history-isearch-wrap): Use the new function to access the
    Federico> minibuffer history.
    Federico> * src/minibuf.c (read_minibuf): Switch to previous buffer temporarily
    Federico> before updating history list (Bug#38317).
    Federico> * etc/NEWS: Announce changes.
    Federico> ---
    Federico>  etc/NEWS       |  6 ++++++
    Federico>  lisp/simple.el | 18 +++++++++++++-----
    Federico>  src/minibuf.c  | 17 +++++++++++++++--
    Federico>  3 files changed, 34 insertions(+), 7 deletions(-)

    Federico> diff --git a/etc/NEWS b/etc/NEWS
    Federico> index d3331daf17..9dbf36ae40 100644
    Federico> --- a/etc/NEWS
    Federico> +++ b/etc/NEWS
    Federico> @@ -518,6 +518,12 @@ Note that this key binding will not work on MS-Windows systems if
    Federico>  key binding with an upper case letter - if you can type it, you can
    Federico>  bind it.
 
    Federico> +---
    Federico> +** 'read-from-minibuffer' now works with buffer-local history variables

'.' at end of sentence.

    Federico> +The HIST argument of 'read-from-minibuffer' now works correctly with
    Federico> +buffer-local variables. This means that different buffers can have
    Federico> +their own separated input history list if desired.
    Federico> +
    Federico>  \f
    Federico>  * Editing Changes in Emacs 27.1
 
    Federico> diff --git a/lisp/simple.el b/lisp/simple.el
    Federico> index 2aac557154..0b1d7b73ab 100644
    Federico> --- a/lisp/simple.el
    Federico> +++ b/lisp/simple.el
    Federico> @@ -2041,7 +2041,7 @@ previous-matching-history-element
    Federico>  	     (null minibuffer-text-before-history))
    Federico>  	(setq minibuffer-text-before-history
    Federico>  	      (minibuffer-contents-no-properties)))
    Federico> -    (let ((history (symbol-value minibuffer-history-variable))
    Federico> +    (let ((history (minibuffer-history-values))
    Federico>  	  (case-fold-search
    Federico>  	   (if (isearch-no-upper-case-p regexp t) ; assume isearch.el is dumped
    Federico>  	       ;; On some systems, ignore case for file names.
    Federico> @@ -2141,6 +2141,14 @@ minibuffer-default-add-completions
    Federico>  	(append def all)
    Federico>        (cons def (delete def all)))))
 
    Federico> +(defun minibuffer-history-values ()
    Federico> +  "Return the minibuffer input history values.
    Federico> +If `minibuffer-history-variable' points to a buffer-local variable and
    Federico> +the minibuffer is active, return the buffer-local value for the buffer
    Federico> +selected in the window returned by
    Federico> `minibuffer-selected-window'."

This is a true description of what the code does, but perhaps not
clear to users. How about:

"for the buffer selected when the minibuffer was activated."

Robert

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Drew Adams]

> "for the buffer selected when the minibuffer was activated."

A buffer isn't "selected"; it's "current".

In Emacs jargon, windows and frames are selected;
buffers are made current.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Robert Pluim]

>>>>> On Wed, 27 Nov 2019 07:24:46 -0800 (PST), Drew Adams <drew.adams@oracle.com> said:

    >> "for the buffer selected when the minibuffer was activated."
    Drew> A buffer isn't "selected"; it's "current".

    Drew> In Emacs jargon, windows and frames are selected;
    Drew> buffers are made current.

Sure, that works as well.

Robert

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> I've tested the following commands with it, using `read-from-minibuffer'
> with a buffer-local HIST:
>
> - previous-history-element (M-p)
> - next-history-element (M-n)
> - previous-matching-history-element (M-r)
> - next-matching-history-element (M-s)
> - isearch-forward (C-s)
> - isearch-backward (C-r)
>
> They've all worked correctly and used only the buffer's local history
> value.

I haven't tested the code, but it makes sense to me.  I do wonder
whether there are any instances of the variables that are local to the
minibuffer...  but I guess that's unlikely?

> +** 'read-from-minibuffer' now works with buffer-local history variables
> +The HIST argument of 'read-from-minibuffer' now works correctly with
> +buffer-local variables. This means that different buffers can have
> +their own separated input history list if desired.

This should probably be documented in the manual, too?

The patch otherwise looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

> I haven't tested the code, but it makes sense to me.  I do wonder
> whether there are any instances of the variables that are local to the
> minibuffer...  but I guess that's unlikely?

I agree that it would be unlikely. I am not sure what the use-case for
having a history local to the minibuffer itself would be. It makes much
more sense for the history to be global or to be determined by the
context i.e. the current buffer (like I want to do with `goto-line').

>> +** 'read-from-minibuffer' now works with buffer-local history variables
>> +The HIST argument of 'read-from-minibuffer' now works correctly with
>> +buffer-local variables. This means that different buffers can have
>> +their own separated input history list if desired.
>
> This should probably be documented in the manual, too?
>
> The patch otherwise looks good to me.

Thanks. In what part of the manual would you document this? I was
thinking of either in:

1) The "Minibuffer History" section (but it doesn't mention the actual
HIST variable anywhere in there, though, and never mentions any specific
minibuffer-reading function)

2) In "Reading Text Strings with the Minibuffer", in the entry for
`read-from-minibuffer' (right before the link to "Minibuffer
History"). Maybe if we add it there users can then infer that other
minibuffer commands with a HIST argument also accept buffer-local
values.

Should I update the docstring for `read-from-minibuffer' as well? I
would like to add two things: that the value can be buffer-local, and
that the newly read item will be added to the history list. IMO this
wasn't clear enough so I ended up calling `add-to-history' myself, which
was redundant since `read_minibuf' does that already.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Eli Zaretskii]

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Wed, 27 Nov 2019 19:50:00 +0100
> Cc: 38317@debbugs.gnu.org
> 
> Thanks. In what part of the manual would you document this? I was
> thinking of either in:
> 
> 1) The "Minibuffer History" section (but it doesn't mention the actual
> HIST variable anywhere in there, though, and never mentions any specific
> minibuffer-reading function)

That section does mention the history variables, and even includes a
list of them.  So I'm not sure why you say it doesn't.

> Should I update the docstring for `read-from-minibuffer' as well? I
> would like to add two things: that the value can be buffer-local, and
> that the newly read item will be added to the history list.

That it adds to the history is worth mentioning, but the fact that it
can be buffer-local we don't normally say explicitly, because it's
expected to "just work".

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

Eli Zaretskii <eliz@gnu.org> writes:
> That section does mention the history variables, and even includes a
> list of them.  So I'm not sure why you say it doesn't.

You're right, I was looking at the Emacs manual instead of the Elisp
manual (the sections have the same title). I can see that the Elisp manual is
more specific about the history list variable. I could mention the
possibility of using a buffer-local variable there.

> That it adds to the history is worth mentioning, but the fact that it
> can be buffer-local we don't normally say explicitly, because it's
> expected to "just work".

Ok, thanks.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

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

I'm now attaching a new patch with feedback corrections from everyone in
the thread (threads?).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 7252 bytes --]

From 481f85d70463207c5ca5c43c287f1edcc6ed288e Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Thu, 28 Nov 2019 12:59:52 +0100
Subject: [PATCH 1/1] Make HIST arg of read-from-minibuffer work with
 buffer-local vars

* lisp/simple.el (minibuffer-history-values): New function, should be
used to access the minibuffer input history variable when the
minibuffer might be active.  If the variable is buffer-local, the
previous buffer's value will be used.
(goto-history-element): Use the new function to access the minibuffer
history.
(minibuffer-history-isearch-wrap): Use the new function to access the
minibuffer history.
* src/minibuf.c (read_minibuf): Switch to previous buffer temporarily
before updating history list (Bug#38317).
(read-from-minibuffer): Extend documentation to mention that the
result of using the command will be added to the history list by
default.
* doc/lispref/minibuf.texi (Minibuffer History): Mention the
possibility of using a buffer-local variable as history.
* etc/NEWS: Announce changes.
---
 doc/lispref/minibuf.texi |  3 ++-
 etc/NEWS                 |  6 ++++++
 lisp/simple.el           | 18 +++++++++++++-----
 src/minibuf.c            | 20 ++++++++++++++++++--
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index 4a218fe737..dde30ce67f 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -547,7 +547,8 @@ Minibuffer History
   If you don't specify @var{history}, then the default history list
 @code{minibuffer-history} is used.  For other standard history lists,
 see below.  You can also create your own history list variable; just
-initialize it to @code{nil} before the first use.
+initialize it to @code{nil} before the first use.  If the variable is
+buffer local, then each buffer will have its own input history list.
 
   Both @code{read-from-minibuffer} and @code{completing-read} add new
 elements to the history list automatically, and provide commands to
diff --git a/etc/NEWS b/etc/NEWS
index cb73e46358..bc088129f8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -522,6 +522,12 @@ Note that this key binding will not work on MS-Windows systems if
 key binding with an upper case letter - if you can type it, you can
 bind it.
 
++++
+** 'read-from-minibuffer' now works with buffer-local history variables.
+The HIST argument of 'read-from-minibuffer' now works correctly with
+buffer-local variables.  This means that different buffers can have
+their own separated input history list if desired.
+
 \f
 * Editing Changes in Emacs 27.1
 
diff --git a/lisp/simple.el b/lisp/simple.el
index 2aac557154..ef36ff9f8a 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2041,7 +2041,7 @@ previous-matching-history-element
 	     (null minibuffer-text-before-history))
 	(setq minibuffer-text-before-history
 	      (minibuffer-contents-no-properties)))
-    (let ((history (symbol-value minibuffer-history-variable))
+    (let ((history (minibuffer-history-value))
 	  (case-fold-search
 	   (if (isearch-no-upper-case-p regexp t) ; assume isearch.el is dumped
 	       ;; On some systems, ignore case for file names.
@@ -2141,6 +2141,14 @@ minibuffer-default-add-completions
 	(append def all)
       (cons def (delete def all)))))
 
+(defun minibuffer-history-value ()
+  "Return the value of the minibuffer input history list.
+If `minibuffer-history-variable' points to a buffer-local variable and
+the minibuffer is active, return the buffer-local value for the buffer
+that was current when the minibuffer was activated."
+  (buffer-local-value minibuffer-history-variable
+                      (window-buffer (minibuffer-selected-window))))
+
 (defun goto-history-element (nabs)
   "Puts element of the minibuffer history in the minibuffer.
 The argument NABS specifies the absolute history position in
@@ -2169,8 +2177,8 @@ goto-history-element
 	(user-error (if minibuffer-default
                         "End of defaults; no next item"
                       "End of history; no default available")))
-    (if (> nabs (if (listp (symbol-value minibuffer-history-variable))
-                    (length (symbol-value minibuffer-history-variable))
+    (if (> nabs (if (listp (minibuffer-history-value))
+                    (length (minibuffer-history-value))
                   0))
 	(user-error "Beginning of history; no preceding item"))
     (unless (memq last-command '(next-history-element
@@ -2192,7 +2200,7 @@ goto-history-element
 	   (setq minibuffer-returned-to-present t)
 	   (setq minibuffer-text-before-history nil))
 	  (t (setq elt (nth (1- minibuffer-history-position)
-			    (symbol-value minibuffer-history-variable)))))
+			    (minibuffer-history-value)))))
     (insert
      (if (and (eq minibuffer-history-sexp-flag (minibuffer-depth))
 	      (not minibuffer-returned-to-present))
@@ -2445,7 +2453,7 @@ minibuffer-history-isearch-wrap
   ;; beginning/end of the history, wrap the search to the first/last
   ;; minibuffer history element.
   (if isearch-forward
-      (goto-history-element (length (symbol-value minibuffer-history-variable)))
+      (goto-history-element (length (minibuffer-history-value)))
     (goto-history-element 0))
   (setq isearch-success t)
   (goto-char (if isearch-forward (minibuffer-prompt-end) (point-max))))
diff --git a/src/minibuf.c b/src/minibuf.c
index 1e87c5044a..bdae01dbc5 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -353,7 +353,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 	      Lisp_Object histvar, Lisp_Object histpos, Lisp_Object defalt,
 	      bool allow_props, bool inherit_input_method)
 {
-  Lisp_Object val;
+  Lisp_Object val, previous_buffer = Fcurrent_buffer ();
   ptrdiff_t count = SPECPDL_INDEX ();
   Lisp_Object mini_frame, ambient_dir, minibuffer, input_method;
   Lisp_Object enable_multibyte;
@@ -698,7 +698,20 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   /* Add the value to the appropriate history list, if any.  */
   if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
-    call2 (intern ("add-to-history"), Vminibuffer_history_variable, histstring);
+    {
+      ptrdiff_t count2 = SPECPDL_INDEX ();
+
+      /* If possible, switch back to the previous buffer first, in
+	 case the history variable is buffer-local.  */
+      if (BUFFER_LIVE_P (XBUFFER (previous_buffer)))
+	{
+	  record_unwind_current_buffer ();
+	  Fset_buffer (previous_buffer);
+	}
+
+      call2 (intern ("add-to-history"), Vminibuffer_history_variable, histstring);
+      unbind_to (count2, Qnil);
+    }
 
   /* If Lisp form desired instead of string, parse it.  */
   if (expflag)
@@ -879,6 +892,9 @@ Fifth arg HIST, if non-nil, specifies a history list and optionally
   starting from 1 at the beginning of the list.  If HIST is the symbol
   `t', history is not recorded.
 
+  If `history-add-new-input' is non-nil (the default), the result will
+  be added to the history list using `add-to-history'.
+
 Sixth arg DEFAULT-VALUE, if non-nil, should be a string, which is used
   as the default to `read' if READ is non-nil and the user enters
   empty input.  But if READ is nil, this function does _not_ return
-- 
2.17.1

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Lars Ingebrigtsen]

Federico Tedin <federicotedin@gmail.com> writes:

> I'm now attaching a new patch with feedback corrections from everyone in
> the thread (threads?).

Looks good to me, and after some cursory testing I've applied this to
Emacs 27.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > You're right, I was looking at the Emacs manual instead of the Elisp
  > manual (the sections have the same title). I can see that the Elisp manual is
  > more specific about the history list variable. I could mention the
  > possibility of using a buffer-local variable there.

Since the two sections have the same name, would it be helpful to give
the Emacs Manual section a cross reference to the Elisp Manual section?
Perhaps, "See also the section of the same name, @xref{...}."
-- 
Dr Richard Stallman
Founder, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: eliz@gnu.org, larsi@gnus.org, 38317@debbugs.gnu.org
> Date: Fri, 29 Nov 2019 00:22:21 -0500
> 
> Since the two sections have the same name, would it be helpful to give
> the Emacs Manual section a cross reference to the Elisp Manual section?
> Perhaps, "See also the section of the same name, @xref{...}."

They have identical names, but discuss radically different subjects.

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Federico Tedin]

>> Since the two sections have the same name, would it be helpful to give
>> the Emacs Manual section a cross reference to the Elisp Manual section?
>> Perhaps, "See also the section of the same name, @xref{...}."
>
> They have identical names, but discuss radically different subjects.

Would the following titles make sense?

emacs/Minibuffer History -> emacs/Minibuffer History Variable[s]

elisp/Minibuffer History -> elisp/Navigating the Minibuffer History
elisp/Minibuffer History -> elisp/Minibuffer History Commands

Obviously it's an unimportant thing to change, but I think it would help
users searching for "emacs minibuffer history" on the Internet.

- Fede

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Since the two sections have the same name, would it be helpful to give
  > the Emacs Manual section a cross reference to the Elisp Manual section?
  > Perhaps, "See also the section of the same name, @xref{...}."

Someone responded that the two sections are not about the same topic.
It may nonetheless be useful to make each one mention the other
in a notice to prevent this kind of confusion.

-- 
Dr Richard Stallman
Founder, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#38317: Buffer-local variables don't work as history for read-from-minibuffer

[Juri Linkov]

> I'm now attaching a final version of my patch.
>
> I've tested the following commands with it, using `read-from-minibuffer'
> with a buffer-local HIST:
>
> - previous-history-element (M-p)
> - next-history-element (M-n)
> - previous-matching-history-element (M-r)
> - next-matching-history-element (M-s)
> - isearch-forward (C-s)
> - isearch-backward (C-r)

Thanks, only one question about the function name:

> +(defun minibuffer-history-values ()
> +  "Return the minibuffer input history values.

Why "values" in the plural form?  This looks like a multivalued function.
It would be more clear with a singular "value".