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)

diff --git a/etc/NEWS b/etc/NEWS
index 57d6ea24270b5090dbc1152aa69955aa69cb29d9..62175c6a82b2f45b3fab0c37051a33d0cda8d0ee 100644 (file)
--- 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.
+
 \f
 * Lisp Changes in Emacs 30.1
 

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 5f776829d7d87af955205c3a1cbfaca4337ea95b..e394295e8e30f915fcc841a6842e1da4ebdc1758 100644 (file)
--- 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 b1eef38b165b3e42af6b7f927aa084616bb7ff0d..5323943430b3fcdb459b130c5baaecf595d272d4 100644 (file)
--- 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))