From 19c8e5ffca5c56bbf81774ef31595bcaceb572fd Mon Sep 17 00:00:00 2001 From: Eshel Yaron Date: Sat, 3 Feb 2024 11:36:51 +0100 Subject: [PATCH] Cease adding non-interactive inputs to 'bookmark-history' Remove 'bookmark-maybe-historicize-string', since it's both buggy (extends 'bookmark-history' indefinitely) and unhelpful. * lisp/bookmark.el (bookmark-maybe-historicize-string): Don't. (bookmark-jump,bookmark-relocate,bookmark-insert-location) (bookmark-rename,bookmark-insert,bookmark-delete): Adjust. * test/lisp/bookmark-tests.el (bookmark-tests-maybe-historicize-string): Remove. * etc/NEWS: Announce. (Bug#12504) --- etc/NEWS | 9 +++++++++ lisp/bookmark.el | 20 ++------------------ test/lisp/bookmark-tests.el | 5 ----- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 57d6ea24270..62175c6a82b 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1599,6 +1599,15 @@ Use a float value for the first argument instead. Instead, use 'eshell-process-wait-time', which supports floating-point values. +--- +** Bookmark commands no longer extend minibuffer history when called non-interactively +Commands that read a bookmark name when called interactively, such as +'bookmark-jump', used to add their bookmark name argument to the +'bookmark-history' minibuffer history variable even when called +non-interactively. This special behavior is removed in this version +of Emacs, for consistency with the common Emacs behavior where +minibuffer history is reserved for past minibuffer inputs. + * Lisp Changes in Emacs 30.1 diff --git a/lisp/bookmark.el b/lisp/bookmark.el index 5f776829d7d..e394295e8e3 100644 --- a/lisp/bookmark.el +++ b/lisp/bookmark.el @@ -584,15 +584,6 @@ If DEFAULT is nil then return empty string for empty input." (concat "bookmark type " (prin1-to-string (substring-no-properties type)))))) -(defmacro bookmark-maybe-historicize-string (string) - "Put STRING into the bookmark prompt history, if caller non-interactive. -We need this because sometimes bookmark functions are invoked -from other commands that pass in the bookmark name, so -`completing-read' never gets a chance to set `bookmark-history'." - `(or - (called-interactively-p 'interactive) - (setq bookmark-history (cons ,string bookmark-history)))) - (defvar bookmark-make-record-function 'bookmark-make-record-default "A function that should be called to create a bookmark record. Modes may set this variable buffer-locally to enable bookmarking of @@ -1297,7 +1288,6 @@ DISPLAY-FUNC would be `switch-to-buffer-other-window'." bookmark-current-bookmark))) (unless bookmark (error "No bookmark specified")) - (bookmark-maybe-historicize-string bookmark) ;; Don't use `switch-to-buffer' because it would let the ;; window-point override the bookmark's point when ;; `switch-to-buffer-preserve-window-point' is non-nil. @@ -1415,7 +1405,6 @@ This makes an already existing bookmark point to that file, instead of the one it used to point at. Useful when a file has been renamed after a bookmark was set in it." (interactive (list (bookmark-completing-read "Bookmark to relocate"))) - (bookmark-maybe-historicize-string bookmark-name) (bookmark-maybe-load-default-file) (let ((bmrk-filename (bookmark-get-filename bookmark-name))) ;; FIXME: Make `bookmark-relocate' support bookmark Types @@ -1438,13 +1427,11 @@ after a bookmark was set in it." (bookmark-bmenu-surreptitiously-rebuild-list)))) ;;;###autoload -(defun bookmark-insert-location (bookmark-name &optional no-history) +(defun bookmark-insert-location (bookmark-name &optional _) "Insert the name of the file associated with BOOKMARK-NAME. -Optional second arg NO-HISTORY means don't record this in the -minibuffer history list `bookmark-history'." +Optional second argument is obsolete and ignored." (interactive (list (bookmark-completing-read "Insert bookmark location"))) - (or no-history (bookmark-maybe-historicize-string bookmark-name)) (insert (bookmark-location bookmark-name))) ;;;###autoload @@ -1476,7 +1463,6 @@ While you are entering the new name, consecutive \ consecutive words from the text of the buffer into the new bookmark name." (interactive (list (bookmark-completing-read "Old bookmark name"))) - (bookmark-maybe-historicize-string old-name) (bookmark-maybe-load-default-file) (setq bookmark-yank-point (point)) @@ -1511,7 +1497,6 @@ You may have a problem using this function if the value of variable bookmarks. See help on function `bookmark-load' for more about this." (interactive (list (bookmark-completing-read "Insert bookmark contents"))) - (bookmark-maybe-historicize-string bookmark-name) (bookmark-maybe-load-default-file) (let ((orig-point (point)) (str-to-insert @@ -1536,7 +1521,6 @@ probably because we were called from there." (interactive (list (bookmark-completing-read "Delete bookmark" bookmark-current-bookmark))) - (bookmark-maybe-historicize-string bookmark-name) (bookmark-maybe-load-default-file) (let ((will-go (bookmark-get-bookmark bookmark-name 'noerror))) (bookmark--remove-fringe-mark will-go) diff --git a/test/lisp/bookmark-tests.el b/test/lisp/bookmark-tests.el index b1eef38b165..5323943430b 100644 --- a/test/lisp/bookmark-tests.el +++ b/test/lisp/bookmark-tests.el @@ -192,11 +192,6 @@ the lexically-bound variable `buffer'." (bookmark-prop-set bmk 'filename "prop") (should (equal (bookmark-prop-get bmk 'filename) "prop"))))) -(ert-deftest bookmark-tests-maybe-historicize-string () - (let ((bookmark-history)) - (bookmark-maybe-historicize-string "foo") - (should (equal (car bookmark-history) "foo")))) - (defun bookmark-remove-last-modified (bmk) (assoc-delete-all 'last-modified bmk)) -- 2.39.5