* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer @ 2019-11-21 21:30 Federico Tedin 2019-11-22 13:33 ` Lars Ingebrigtsen 0 siblings, 1 reply; 22+ messages in thread From: Federico Tedin @ 2019-11-21 21:30 UTC (permalink / raw) To: 38317 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-21 21:30 bug#38317: Buffer-local variables don't work as history for read-from-minibuffer Federico Tedin @ 2019-11-22 13:33 ` Lars Ingebrigtsen 2019-11-22 15:23 ` Michael Heerdegen 2019-11-22 16:24 ` martin rudalics 0 siblings, 2 replies; 22+ messages in thread From: Lars Ingebrigtsen @ 2019-11-22 13:33 UTC (permalink / raw) To: Federico Tedin; +Cc: 38317 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-22 13:33 ` Lars Ingebrigtsen @ 2019-11-22 15:23 ` Michael Heerdegen 2019-11-22 16:24 ` martin rudalics 1 sibling, 0 replies; 22+ messages in thread From: Michael Heerdegen @ 2019-11-22 15:23 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 38317, Federico Tedin 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-22 13:33 ` Lars Ingebrigtsen 2019-11-22 15:23 ` Michael Heerdegen @ 2019-11-22 16:24 ` martin rudalics 2019-11-23 11:54 ` Lars Ingebrigtsen 1 sibling, 1 reply; 22+ messages in thread From: martin rudalics @ 2019-11-22 16:24 UTC (permalink / raw) To: Lars Ingebrigtsen, Federico Tedin; +Cc: 38317 > 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-22 16:24 ` martin rudalics @ 2019-11-23 11:54 ` Lars Ingebrigtsen 2019-11-23 20:36 ` Federico Tedin 0 siblings, 1 reply; 22+ messages in thread From: Lars Ingebrigtsen @ 2019-11-23 11:54 UTC (permalink / raw) To: martin rudalics; +Cc: 38317, Federico Tedin 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-23 11:54 ` Lars Ingebrigtsen @ 2019-11-23 20:36 ` Federico Tedin 2019-11-23 23:12 ` Federico Tedin 0 siblings, 1 reply; 22+ messages in thread From: Federico Tedin @ 2019-11-23 20:36 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 38317 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-23 20:36 ` Federico Tedin @ 2019-11-23 23:12 ` Federico Tedin 2019-11-26 21:54 ` Federico Tedin 0 siblings, 1 reply; 22+ messages in thread From: Federico Tedin @ 2019-11-23 23:12 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 38317 [-- 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); ^ permalink raw reply related [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-23 23:12 ` Federico Tedin @ 2019-11-26 21:54 ` Federico Tedin 2019-11-27 8:14 ` Robert Pluim ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Federico Tedin @ 2019-11-26 21:54 UTC (permalink / raw) To: 38317; +Cc: Lars Ingebrigtsen [-- 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-26 21:54 ` Federico Tedin @ 2019-11-27 8:14 ` Robert Pluim 2019-11-27 15:24 ` Drew Adams 2019-11-27 11:53 ` Lars Ingebrigtsen 2019-11-27 21:19 ` Juri Linkov 2 siblings, 1 reply; 22+ messages in thread From: Robert Pluim @ 2019-11-27 8:14 UTC (permalink / raw) To: Federico Tedin; +Cc: Lars Ingebrigtsen, 38317 >>>>> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 8:14 ` Robert Pluim @ 2019-11-27 15:24 ` Drew Adams 2019-11-27 15:28 ` Robert Pluim 0 siblings, 1 reply; 22+ messages in thread From: Drew Adams @ 2019-11-27 15:24 UTC (permalink / raw) To: Robert Pluim, Federico Tedin; +Cc: Lars Ingebrigtsen, 38317 > "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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 15:24 ` Drew Adams @ 2019-11-27 15:28 ` Robert Pluim 0 siblings, 0 replies; 22+ messages in thread From: Robert Pluim @ 2019-11-27 15:28 UTC (permalink / raw) To: Drew Adams; +Cc: Lars Ingebrigtsen, 38317, Federico Tedin >>>>> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-26 21:54 ` Federico Tedin 2019-11-27 8:14 ` Robert Pluim @ 2019-11-27 11:53 ` Lars Ingebrigtsen 2019-11-27 18:50 ` Federico Tedin 2019-11-27 21:19 ` Juri Linkov 2 siblings, 1 reply; 22+ messages in thread From: Lars Ingebrigtsen @ 2019-11-27 11:53 UTC (permalink / raw) To: Federico Tedin; +Cc: 38317 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 11:53 ` Lars Ingebrigtsen @ 2019-11-27 18:50 ` Federico Tedin 2019-11-27 19:25 ` Eli Zaretskii 0 siblings, 1 reply; 22+ messages in thread From: Federico Tedin @ 2019-11-27 18:50 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 38317 > 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 18:50 ` Federico Tedin @ 2019-11-27 19:25 ` Eli Zaretskii 2019-11-27 21:04 ` Federico Tedin 0 siblings, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2019-11-27 19:25 UTC (permalink / raw) To: Federico Tedin; +Cc: larsi, 38317 > 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". ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 19:25 ` Eli Zaretskii @ 2019-11-27 21:04 ` Federico Tedin 2019-11-28 12:05 ` Federico Tedin 2019-11-29 5:22 ` Richard Stallman 0 siblings, 2 replies; 22+ messages in thread From: Federico Tedin @ 2019-11-27 21:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 38317 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 21:04 ` Federico Tedin @ 2019-11-28 12:05 ` Federico Tedin 2019-12-05 9:31 ` Lars Ingebrigtsen 2019-11-29 5:22 ` Richard Stallman 1 sibling, 1 reply; 22+ messages in thread From: Federico Tedin @ 2019-11-28 12:05 UTC (permalink / raw) To: 38317; +Cc: larsi [-- 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-28 12:05 ` Federico Tedin @ 2019-12-05 9:31 ` Lars Ingebrigtsen 0 siblings, 0 replies; 22+ messages in thread From: Lars Ingebrigtsen @ 2019-12-05 9:31 UTC (permalink / raw) To: Federico Tedin; +Cc: 38317 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-27 21:04 ` Federico Tedin 2019-11-28 12:05 ` Federico Tedin @ 2019-11-29 5:22 ` Richard Stallman 2019-11-29 10:21 ` Eli Zaretskii 2019-12-01 5:59 ` Richard Stallman 1 sibling, 2 replies; 22+ messages in thread From: Richard Stallman @ 2019-11-29 5:22 UTC (permalink / raw) To: Federico Tedin; +Cc: 38317, larsi [[[ 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) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-29 5:22 ` Richard Stallman @ 2019-11-29 10:21 ` Eli Zaretskii 2019-11-29 17:02 ` Federico Tedin 2019-12-01 5:59 ` Richard Stallman 1 sibling, 1 reply; 22+ messages in thread From: Eli Zaretskii @ 2019-11-29 10:21 UTC (permalink / raw) To: rms; +Cc: larsi, 38317, federicotedin > 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-29 10:21 ` Eli Zaretskii @ 2019-11-29 17:02 ` Federico Tedin 0 siblings, 0 replies; 22+ messages in thread From: Federico Tedin @ 2019-11-29 17:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 38317, rms >> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-29 5:22 ` Richard Stallman 2019-11-29 10:21 ` Eli Zaretskii @ 2019-12-01 5:59 ` Richard Stallman 1 sibling, 0 replies; 22+ messages in thread From: Richard Stallman @ 2019-12-01 5:59 UTC (permalink / raw) To: federicotedin, 38317, larsi [[[ 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) ^ permalink raw reply [flat|nested] 22+ messages in thread
* bug#38317: Buffer-local variables don't work as history for read-from-minibuffer 2019-11-26 21:54 ` Federico Tedin 2019-11-27 8:14 ` Robert Pluim 2019-11-27 11:53 ` Lars Ingebrigtsen @ 2019-11-27 21:19 ` Juri Linkov 2 siblings, 0 replies; 22+ messages in thread From: Juri Linkov @ 2019-11-27 21:19 UTC (permalink / raw) To: Federico Tedin; +Cc: Lars Ingebrigtsen, 38317 > 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". ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2019-12-05 9:31 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-11-21 21:30 bug#38317: Buffer-local variables don't work as history for read-from-minibuffer Federico Tedin 2019-11-22 13:33 ` Lars Ingebrigtsen 2019-11-22 15:23 ` Michael Heerdegen 2019-11-22 16:24 ` martin rudalics 2019-11-23 11:54 ` Lars Ingebrigtsen 2019-11-23 20:36 ` Federico Tedin 2019-11-23 23:12 ` Federico Tedin 2019-11-26 21:54 ` Federico Tedin 2019-11-27 8:14 ` Robert Pluim 2019-11-27 15:24 ` Drew Adams 2019-11-27 15:28 ` Robert Pluim 2019-11-27 11:53 ` Lars Ingebrigtsen 2019-11-27 18:50 ` Federico Tedin 2019-11-27 19:25 ` Eli Zaretskii 2019-11-27 21:04 ` Federico Tedin 2019-11-28 12:05 ` Federico Tedin 2019-12-05 9:31 ` Lars Ingebrigtsen 2019-11-29 5:22 ` Richard Stallman 2019-11-29 10:21 ` Eli Zaretskii 2019-11-29 17:02 ` Federico Tedin 2019-12-01 5:59 ` Richard Stallman 2019-11-27 21:19 ` Juri Linkov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.