From: Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 69056@debbugs.gnu.org
Subject: bug#69056: 30.0.50; history-add-new-input and recursive minibuffers
Date: Thu, 15 Feb 2024 17:17:26 +0100 [thread overview]
Message-ID: <m1v86pln1l.fsf@dazzs-mbp.home> (raw)
In-Reply-To: <jwvplwxah1w.fsf-monnier+emacs@gnu.org> (Stefan Monnier's message of "Thu, 15 Feb 2024 10:24:45 -0500")
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 1. emacs -Q
>> 2. (setq enable-recursive-minibuffers t)
>> 3. M-y
>> 4. In the minibuffer (with the prompt "Yank from kill-ring: "),
>> type M-x calendar RET (or any other command).
>> 5. M-x M-p
>> Expected: "calendar" is inserted in the minibuffer.
>> Observed: error saying "Beginning of history; no preceding item".
>>
>> The problem is that the minibuffer history of M-x isn't recorded when
>> you invoke M-x from within the minibuffer of read-from-kill-ring (M-y).
>> The reason is that read-from-kill-ring let binds history-add-new-input,
>> and that affects all recursive minibuffers as well, so no minibuffer
>> history is recorded until you exit the first (non-recursive) minibuffer.
>
> I suspect this can bite in more cases, including cases which don't
> involve setting `enable-recursive-minibuffers` to t.
> We should probably change the way `history-add-new-input` works so it's
> attached to a particular minibuffer rather than being dynbound.
Thanks, that's what I thought too. Here's an attempt do just that:
[-- Attachment #2: 0001-Use-buffer-local-value-of-history-add-new-input-in-m.patch --]
[-- Type: text/x-patch, Size: 9221 bytes --]
From 35c7cf51102b5625a46bdb0dc5f7f2299659f699 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Sun, 11 Feb 2024 16:18:48 +0100
Subject: [PATCH] Use buffer local value of 'history-add-new-input' in
minibuffer
Avoid let-binding 'history-add-new-input', since that affects all nested
recursive minibuffers, and instead use a buffer-local setting in the
appropriate minibuffer.
* src/minibuf.c (read_minibuf): Use 'history-add-new-input' local value.
* lisp/isearch.el (isearch-edit-string)
* lisp/replace.el (query-replace-read-from)
(query-replace-read-to, read-regexp)
* lisp/simple.el (read-from-kill-ring): Set 'history-add-new-input'
locally in the minibuffer, instead of let-binding it.
* doc/lispref/minibuf.texi (Minibuffer History): Update.
---
doc/lispref/minibuf.texi | 6 +++---
lisp/isearch.el | 12 +++++++-----
lisp/replace.el | 36 +++++++++++++++++++-----------------
lisp/simple.el | 4 ++--
src/minibuf.c | 7 ++++++-
5 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/doc/lispref/minibuf.texi b/doc/lispref/minibuf.texi
index aa27de72ba0..2e8b21d7040 100644
--- a/doc/lispref/minibuf.texi
+++ b/doc/lispref/minibuf.texi
@@ -700,9 +700,9 @@ Minibuffer History
@end defun
@defvar history-add-new-input
-If the value of this variable is @code{nil}, standard functions that
-read from the minibuffer don't add new elements to the history list.
-This lets Lisp programs explicitly manage input history by using
+If the value of this variable is @code{nil} in a minibuffer, Emacs
+doesn't add new elements to the history list of that minibuffer. This
+lets Lisp programs explicitly manage input history by using
@code{add-to-history}. The default value is @code{t}.
@end defvar
diff --git a/lisp/isearch.el b/lisp/isearch.el
index a139a6fb84e..814ab919d5e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1844,10 +1844,6 @@ isearch-edit-string
(interactive)
(with-isearch-suspended
(let* ((message-log-max nil)
- ;; Don't add a new search string to the search ring here
- ;; in `read-from-minibuffer'. It should be added only
- ;; by `isearch-update-ring' called from `isearch-done'.
- (history-add-new-input nil)
;; Binding minibuffer-history-symbol to nil is a work-around
;; for some incompatibility with gmhist.
(minibuffer-history-symbol)
@@ -1855,7 +1851,13 @@ isearch-edit-string
(minibuffer-allow-text-properties t))
(setq isearch-new-string
(minibuffer-with-setup-hook
- (minibuffer-lazy-highlight-setup)
+ (let ((setup (minibuffer-lazy-highlight-setup)))
+ (lambda ()
+ ;; Don't add a new search string to the search ring here
+ ;; in `read-from-minibuffer'. It should be added only
+ ;; by `isearch-update-ring' called from `isearch-done'.
+ (setq-local history-add-new-input nil)
+ (funcall setup)))
(read-from-minibuffer
(isearch-message-prefix nil isearch-nonincremental)
(cons isearch-string (1+ (or (isearch-fail-pos)
diff --git a/lisp/replace.el b/lisp/replace.el
index fa460a16063..61a1cc7714c 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -205,8 +205,7 @@ query-replace-read-from
Prompt with PROMPT. REGEXP-FLAG non-nil means the response should be a regexp.
The return value can also be a pair (FROM . TO) indicating that the user
wants to replace FROM with TO."
- (let* ((history-add-new-input nil)
- (separator-string
+ (let* ((separator-string
(when query-replace-from-to-separator
;; Check if the first non-whitespace char is displayable
(if (char-displayable-p
@@ -254,7 +253,8 @@ query-replace-read-from
(lambda ()
(setq-local text-property-default-nonsticky
(append '((separator . t) (face . t))
- text-property-default-nonsticky)))
+ text-property-default-nonsticky)
+ history-add-new-input nil))
(if regexp-flag
(read-regexp
(if query-replace-read-from-regexp-default
@@ -342,11 +342,13 @@ query-replace-read-to
should a regexp."
(query-replace-compile-replacement
(save-excursion
- (let* ((history-add-new-input nil)
- (to (read-from-minibuffer
- (format "%s %s with: " prompt (query-replace-descr from))
- nil nil nil
- query-replace-to-history-variable from t)))
+ (let* ((to
+ (minibuffer-with-setup-hook
+ (lambda () (setq-local history-add-new-input nil))
+ (read-from-minibuffer
+ (format "%s %s with: " prompt (query-replace-descr from))
+ nil nil nil
+ query-replace-to-history-variable from t))))
(add-to-history query-replace-to-history-variable to nil t)
(add-to-history 'query-replace-defaults (cons from to) nil t)
to))
@@ -903,18 +905,18 @@ read-regexp
(suggestions (if (listp defaults) defaults (list defaults)))
(suggestions (append suggestions (read-regexp-suggestions)))
(suggestions (delete-dups (delq nil (delete "" suggestions))))
- ;; Do not automatically add default to the history for empty input.
- (history-add-new-input nil)
;; `read-regexp--case-fold' dynamically bound and may be
;; altered by `M-c'.
(read-regexp--case-fold case-fold-search)
- (input (read-from-minibuffer
- (if (string-match-p ":[ \t]*\\'" prompt)
- prompt
- (format-prompt prompt (and (length> default 0)
- (query-replace-descr default))))
- nil read-regexp-map
- nil (or history 'regexp-history) suggestions t))
+ (input (minibuffer-with-setup-hook
+ (lambda () (setq-local history-add-new-input nil))
+ (read-from-minibuffer
+ (if (string-match-p ":[ \t]*\\'" prompt)
+ prompt
+ (format-prompt prompt (and (length> default 0)
+ (query-replace-descr default))))
+ nil read-regexp-map
+ nil (or history 'regexp-history) suggestions t)))
(result (if (equal input "")
;; Return the default value when the user enters
;; empty input.
diff --git a/lisp/simple.el b/lisp/simple.el
index 9a33049f4ca..c5e7f24961c 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -6405,8 +6405,7 @@ read-from-kill-ring
PROMPT is a string to prompt with."
;; `current-kill' updates `kill-ring' with a possible interprogram-paste
(current-kill 0)
- (let* ((history-add-new-input nil)
- (history-pos (when yank-from-kill-ring-rotate
+ (let* ((history-pos (when yank-from-kill-ring-rotate
(- (length kill-ring)
(length kill-ring-yank-pointer))))
(ellipsis (if (char-displayable-p ?…) "…" "..."))
@@ -6441,6 +6440,7 @@ read-from-kill-ring
read-from-kill-ring-history)))
(minibuffer-with-setup-hook
(lambda ()
+ (setq-local history-add-new-input nil)
;; Allow ‘SPC’ to be self-inserting
(use-local-map
(let ((map (make-sparse-keymap)))
diff --git a/src/minibuf.c b/src/minibuf.c
index 7c0c9799a60..b6126fe5c87 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -585,6 +585,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
/* String to add to the history. */
Lisp_Object histstring;
Lisp_Object histval;
+ bool nohist = false;
Lisp_Object empty_minibuf;
@@ -902,6 +903,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
/* Don't allow the user to undo past this point. */
bset_undo_list (current_buffer, Qnil);
+ /* Cache the buffer-local value. */
+ nohist = NILP (find_symbol_value (Qhistory_add_new_input));
+
recursive_edit_1 ();
/* If cursor is on the minibuffer line,
@@ -965,7 +969,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
/* Add the value to the appropriate history list, if any. This is
done after the previous buffer has been made current again, in
case the history variable is buffer-local. */
- if (! (NILP (Vhistory_add_new_input) || NILP (histstring)))
+ if (! (nohist || NILP (histstring)))
call2 (Qadd_to_history, histvar, histstring);
/* If Lisp form desired instead of string, parse it. */
@@ -2311,6 +2315,7 @@ syms_of_minibuf (void)
Fset (Qcustom_variable_history, Qnil);
DEFSYM (Qminibuffer_history, "minibuffer-history");
+ DEFSYM (Qhistory_add_new_input, "history-add-new-input");
DEFSYM (Qbuffer_name_history, "buffer-name-history");
Fset (Qbuffer_name_history, Qnil);
--
2.42.0
next prev parent reply other threads:[~2024-02-15 16:17 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-11 15:54 bug#69056: 30.0.50; history-add-new-input and recursive minibuffers Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-11 16:47 ` Eli Zaretskii
2024-02-11 17:42 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-11 17:50 ` Eli Zaretskii
2024-02-15 8:19 ` Eli Zaretskii
2024-02-15 15:24 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-15 16:17 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-02-15 17:56 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-15 18:40 ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-15 19:20 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-15 19:34 ` Eli Zaretskii
2024-02-15 19:54 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-16 7:08 ` Eli Zaretskii
2024-02-15 19:27 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m1v86pln1l.fsf@dazzs-mbp.home \
--to=bug-gnu-emacs@gnu.org \
--cc=69056@debbugs.gnu.org \
--cc=me@eshelyaron.com \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).