From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#69056: 30.0.50; history-add-new-input and recursive minibuffers Date: Thu, 15 Feb 2024 19:40:29 +0100 Message-ID: References: Reply-To: Eshel Yaron Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26208"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 69056@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Feb 15 19:40:57 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ragfJ-0006Y3-5T for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 15 Feb 2024 19:40:57 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ragf7-0002Zd-Re; Thu, 15 Feb 2024 13:40:45 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ragf6-0002Z0-FZ for bug-gnu-emacs@gnu.org; Thu, 15 Feb 2024 13:40:44 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ragf6-0002yD-1G for bug-gnu-emacs@gnu.org; Thu, 15 Feb 2024 13:40:44 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ragfO-0003qJ-IJ for bug-gnu-emacs@gnu.org; Thu, 15 Feb 2024 13:41:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eshel Yaron Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 15 Feb 2024 18:41:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69056 X-GNU-PR-Package: emacs Original-Received: via spool by 69056-submit@debbugs.gnu.org id=B69056.170802245414739 (code B ref 69056); Thu, 15 Feb 2024 18:41:02 +0000 Original-Received: (at 69056) by debbugs.gnu.org; 15 Feb 2024 18:40:54 +0000 Original-Received: from localhost ([127.0.0.1]:57028 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ragfF-0003pf-FZ for submit@debbugs.gnu.org; Thu, 15 Feb 2024 13:40:54 -0500 Original-Received: from mail.eshelyaron.com ([107.175.124.16]:60922 helo=eshelyaron.com) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ragfD-0003pW-7d for 69056@debbugs.gnu.org; Thu, 15 Feb 2024 13:40:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=eshelyaron.com; s=mail; t=1708022431; bh=VvYE+E/GmpWVACKZr7it1Baw+pNFoaheM2ArwZIzxbc=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=oLYlaRRze5C3K524GNJqJUgnPjo8sohWjaVUa2zhdOl13RljD6ledzuLsaQCp8CB0 rnpFPV/6rGN2oB9eVF5ayYk+yOVW/7TZ6h87q+jybNrZX8xLl5r/6rqDW3zDxM8FGh X/Et23HzlAUcOXkW3zwDymNGJBrGR5GcTZiGkLOz2/1Xvxf6jiCqA4NhsoyLNnCx/7 Buy1Y6Wvfd8cAxylzm7x9CLExv+/NTxjo9PK/Qh5pLFl4TdwlQOLv7kfUzBtKzXDKg 6fGL8bfhuWnuRdq/WtJWUhRE/KzKIa0g+F2n4I4W0xGR74+rDu6vu1mx+TgFS4VsDS BnBkiVh1VHiEQ== In-Reply-To: (Stefan Monnier's message of "Thu, 15 Feb 2024 12:56:43 -0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:280072 Archived-At: --=-=-= Content-Type: text/plain Stefan Monnier writes: >> Thanks, that's what I thought too. Here's an attempt do just that: > > Looks pretty good. I do have some comments/questions: > >> @@ -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)); > > Why not use `Vhistory_add_new_input`? Good question, I guess for some reason I assumed that `NILP (Vfoo)` doesn't check the buffer-local value like `find_symbol_value (Qfoo)` does... > [ Also, it's not really "cache" (which implies it impacts only > performance). More like "remember". ] >> @@ -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. */ > > IIUC this change is needed because by the time we get here the > buffer-local value of `history-add-new-input` has been flushed by > `minibuffer-inactive-mode` called by `read_minibuf_unwind`, > itself run by the `unbind_to` a few lines above. > So maybe we can simplify this by just moving the above 2 lines before > the `unbind_to`, WDYT? Oh, that's much simpler indeed. And it seems to work just as well. Here's an updated patch (v2): --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: attachment; filename=v2-0001-Use-buffer-local-value-of-history-add-new-input-i.patch Content-Transfer-Encoding: quoted-printable >From 66fc69ff23911aa6e91e290a71568cf14d076d58 Mon Sep 17 00:00:00 2001 From: Eshel Yaron Date: Sun, 11 Feb 2024 16:18:48 +0100 Subject: [PATCH v2] 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. (Bug#69056) --- doc/lispref/minibuf.texi | 6 +++--- lisp/isearch.el | 12 +++++++----- lisp/replace.el | 36 +++++++++++++++++++----------------- lisp/simple.el | 4 ++-- src/minibuf.c | 12 ++++++------ 5 files changed, 37 insertions(+), 33 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 =20 @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 =20 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 re= gexp. 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 defau= lt)))) + 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 ?=E2=80=A6) "=E2=80=A6" "...")) @@ -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 =E2=80=98SPC=E2=80=99 to be self-inserting (use-local-map (let ((map (make-sparse-keymap))) diff --git a/src/minibuf.c b/src/minibuf.c index 7c0c9799a60..88993659b90 100644 --- a/src/minibuf.c +++ b/src/minibuf.c @@ -948,6 +948,12 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Li= sp_Object prompt, else histstring =3D Qnil; =20 + /* 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))) + call2 (Qadd_to_history, histvar, histstring); + /* The appropriate frame will get selected in set-window-configuration. */ unbind_to (count, Qnil); @@ -962,12 +968,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Li= sp_Object prompt, calling_frame)))) call2 (Qselect_frame_set_input_focus, calling_frame, Qnil); =20 - /* 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))) - call2 (Qadd_to_history, histvar, histstring); - /* If Lisp form desired instead of string, parse it. */ if (expflag) val =3D string_to_object (val, defalt); --=20 2.42.0 --=-=-=--