unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Let bookmark handlers do everything, including display the destination
@ 2022-08-13 20:59 Drew Adams
  2022-08-18 18:14 ` Karl Fogel
  0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2022-08-13 20:59 UTC (permalink / raw)
  To: emacs-devel@gnu.org; +Cc: Karl Fogel

[-- Attachment #1: Type: text/plain, Size: 10427 bytes --]

tl;dr:

Let's change the handling of a bookmark - including
displaying the bookmark destination - to be the
responsibility of its handler.

___

Attached is a patch that does that.  It moves the displaying
of a bookmark's "destination" from `bookmark-jump' to the
bookmark itself - specifically, to its handler.

Currently `bookmark-jump', rather than a bookmark's handler,
displays the bookmark destination.  And the behavior's
hard-coded: a bookmark always displays a destination, and
that's always the last thing done before the after-jump
hook.

That's too rigid.  It means that a handler function can't do
something more after the display, unless it finagles to put
that after-display processing on `bookmark-after-jump-hook'
(temporarily).  And it means that a handler can't choose its
own display function - the DISPLAY-FUNCTION passed to
`bookmark-jump' is always used.  And it means that a handler
can't dispense with display altogether.

By moving the responsibility for display to the handler, the
patch gets rid of this rigidity.

Specifically, instead of `bookmark--jump-via' systematically
invoking its DISPLAY-FUNCTION argument, it saves the
argument value in variable `bookmark-jump-display-function'.
And then the default handler, `bookmark-default-handler'
invokes that function.

Most bookmarks don't have their own handler; they use the
default handler.  So their behavior doesn't change at all
with this approach.  Most bookmarks use `bookmark-jump's
DISPLAY FUNCTION as the last thing before running
`bookmark-after-jump-hook' (and most should do that).

The same applies to bookmarks that do have their own
handler, if the handler invokes the default handler at the
end.  For example, the Info bookmark handler,
`Info-bookmark-jump' sets up what it needs, and ends by
invoking the default handler:

  (bookmark-default-handler
   `("" (buffer . ,buf)
     . ,(bookmark-get-bookmark-record bookmark)))

What changes with the proposed approach is only what can
happen for a bookmark that has its own handler, if that
handler either (1) invokes the default handler at some point
other than at the end - so it does something after
displaying, or (2) it doesn't invoke the default handler.

If a handler doesn't invoke the default handler, it can call
whatever display function it likes, whenever it likes.  Or
it can forego display altogether.

This approach gives the handler more control - it can decide
whether, when, and how to display the destination.  The
entire behavior of a bookmark is then up to its handler.

And this allows for more kinds of bookmarks, including
bookmarks that don't display a destination.

For example, Bookmark+, which uses this approach, predefines
several kinds of bookmarks with handlers that either provide
their own kind of display or don't display anything.  This
includes bookmarks that do these things:

 . Combine a sequence of other bookmarks (composition)
 . Invoke a function
 . Open `*Bookmark List*' for a set of bookmarks, a sort
   order, etc.
 . Switch to a different bookmark file or load another
   bookmark file
 . Copy a snippet to the kill-ring
 . Switch to a desktop
 . Restore a set of keyboard macros
 . Restore a set of defvars

Other Bookmark+ predefined bookmark types have handlers that
do invoke `bookmark-default-handler' (with a definition
similar to that in the patch, invoking DISPLAY-FUNCTION),
which is the typical case.

Though Bookmark+ handling is different from that of vanilla
Emacs (it's essentially that of the patch), this difference
isn't a problem, in general - nearly all vanilla bookmarks
work with Bookmark+.

It is a problem in a few cases, however.  There's at least
one 3rd-party library, PDF Tools, whose handler doesn't make
use of the default handler, and it expects that
`bookmark--jump-via' will display the destination after
handling.  There may be a few others; dunno.

PDF Tools lives here: https://github.com/vedang/pdf-tools/.
It was formerly here: https://github.com/politza/pdf-tools/.
Both have the same code for the handler,
`pdf-view-bookmark-jump-handler'.

That does some finagling that puts a function on the
after-jump hook and then uses `set-buffer'.  It does nothing
to display the buffer - it counts on `bookmark-jump' to
display it after the handler ends.  The rest of the
handling, after display, is done on the after-jump hook.
   
The after-jump hook function depends on the buffer already
being displayed.  And it just uses the buffer of the
currently `selected-window' if the destination buffer isn't
displayed.

See example #1 in attachment `pdf-tools-handler-fix.el'.

As a result, such a bookmark doesn't work with Bookmark+.
E.g., if invoked in the window of buffer `*Bookmark List*'
it just uses that instead of the intended PDF-file buffer,
which is no good.

For it to work with Bookmark+, the handler needs to display
the intended buffer.  E.g., instead of `set-buffer' it needs
to call a function such as `pop-to-buffer-same-window'.
With such an edit the bookmark works with Bookmark+ (and
with vanilla Emacs with the patch).

See example #2 in attachment `pdf-tools-handler-fix.el'.

Another way to fix the handler is for it to call the default
handler.

See example #3 in attachment `pdf-tools-handler-fix.el'.

Yet another way to fix it is to advise the handler, to have
it invoke the default handler at the end:

  (defun my-pdf-handler-advice (bookmark)
    (bookmark-default-handler
      (bookmark-get-bookmark bookmark)))

  (advice-add 'pdf-view-bookmark-jump-handler
              :after 'my-pdf-handler-advice)

That's example #4 in attachment `pdf-tools-handler-fix.el'.

But really, the complications `pdf-view-bookmark-handler'
goes through - creating a closure on the fly and adding it
to the after-jump hook (and subsequently removing it) - are
a workaround for the fact that vanilla `bookmark.el' doesn't
let a handler display the destination when it wants to.

PDF Tools wants to set up the buffer, display it, and then
go to the right PDF page etc.  So it sets up the buffer,
ends the handler, counts on `bookmark-jump' to display the
buffer, and then has a (temporary) after-jump hook do the
rest of the handling: go to the right page etc.  (The hook
function captures some info from the bookmark, so it needs
to be a closure.)  A clever workaround.

With the approach provided by the patch, the handler can do
all of that itself: set up the buffer, display it, then go
to the page etc.  No after-jump-hook workaround.  The code
is quite a bit simpler.

Between setting up the buffer and doing the additional stuff
that was relegated to a temporary after-jump hook, the
handler can either (1) just invoke the default handler or
(2) just call a display function.  Code for each of those
handler definitions is also attached, for comparison with
the above code.

Those solutions are examples #5 and #6, respectively, in
attachment `pdf-tools-handler-fix.el'.

I mention the PDF Tools handler as an example.  I'm not
aware of other handlers that depend on `bookmark-jump'
displaying some buffer and selecting its window, and then
depend on the after-jump hook for additional handling in
that buffer, but there may be some out there.

This is a general problem for Bookmark+, even if it seems to
be run into rarely because most handlers invoke the default
handler (which in Bookmark+ displays the destination using
the DISPLAY-FUNCTION passed to `bookmark-jump').

But this is not just about getting rid of an incompatibility
with Bookmark+'s bookmark handling.  It's also about
improving vanilla `bookmark.el', by letting the handler
handle a bookmark's behavior - all of it.  Give handlers the
control over display, and have the default handler invoke
the `bookmark-jump' DISPLAY-FUNCTION arg.

That makes possible bookmarks that don't necessarily display
a destination - they perform some other action.  And it
allows bookmarks to do any kind of displaying whatsoever
(not just what the `bookmark-jump' DISPLAY-FUNCTION
provides).

Regardless of whether we adopt the proposed behavior, we
might anyway want to post a guideline to let people who
define custom bookmark handlers know that it's
common/typical for a handler to invoke the default handler -
that repositions the bookmark as needed etc.

But with the new behavior we should also make clear that for
a bookmark destination to be displayed, a custom handler
needs to invoke either `bookmark-default-handler' or a
display function.

More generally, the thing to make clear is that a handler
completely defines a bookmark's behavior.  And in
particular, it decides whether, when, and how to display a
bookmark destination.

I think the proposed approach (attached patch or similar)
would affect very few people.  It would affect only library
authors who define custom bookmark handlers (few do) that
don't invoke the default handler (fewer still), AND whose
code relegates some of the handling to the after-jump hook
and depends on `bookmark-jump' invoking the DISPLAY-FUNCTION
(very few, I expect).

The PDF Tools maintainers would need to change their handler
code, by having it invoke a display function or the default
handler.  (The code will be simpler.)

If they do that, then there's no problem for users,
regardless of what Emacs version they use.  In a version
without this change, only `bookmark--jump-via' invokes
DISPLAY-FUNCTION.  In a version with this change, only the
default handler invokes it.  The behavior is the same.

On the other hand, if they don't change their handler code
then users of vanilla `bookmark.el' will suffer the same
problem Bookmark+ users suffer now with such bookmarks -
they won't work because the destination won't be displayed.

If, for example, after this change to `bookmark.el', PDF
Tools didn't update its handler to display the destination,
then `bookmark.el' users of its PDF bookmarks would suffer
the same problem they suffer now with Bookmark+ (doesn't
work).

The patch updates the doc string of variable
`bookmark-alist', to clarify the use of alist entry
`handler'.  Please check that.

Please take a look at the patch and see what you think.

(Thanks to Roxana Dior for testing the PDF Tools fixes.)

[-- Attachment #2: bookmark-2022-08-11.patch --]
[-- Type: application/octet-stream, Size: 7384 bytes --]

diff -u bookmark.el bookmark-patched-2022-08-11.el
--- bookmark.el	2022-08-10 17:31:23.625030900 -0700
+++ bookmark-patched-2022-08-11.el	2022-08-11 13:55:59.012276700 -0700
@@ -265,34 +265,73 @@
 BOOKMARK-NAME is the name you gave to the bookmark when creating it.
 
 PARAM-ALIST is an alist of bookmark information.  The order of the
-entries in PARAM-ALIST is not important.  The default entries are
-described below.  An entry with a key but null value means the entry
-is not used.
+entries in PARAM-ALIST is not important.  An entry with a key but null
+value means the entry is not used - just as if the entry were absent.
+
+This is the set of entries available by default; their values are
+described below.
 
  (filename . FILENAME)
  (buf . BUFFER-OR-NAME)
  (position . POS)
  (front-context-string . STR-AFTER-POS)
  (rear-context-string  . STR-BEFORE-POS)
- (handler . HANDLER)
  (annotation . ANNOTATION)
+ (handler . HANDLER)
 
-FILENAME names the bookmarked file.
-BUFFER-OR-NAME is a buffer or the name of a buffer that is used
-  if FILENAME is not defined or it refers to a non-existent file.
-POS is the bookmarked buffer position.
-STR-AFTER-POS is buffer text that immediately follows POS.
-STR-BEFORE-POS is buffer text that immediately precedes POS.
-ANNOTATION is a string that describes the bookmark.
+All except `annotation' and `handler' are present by default in most
+bookmarks.  `annotation' is present only if a user gives the bookmark
+an annotation.  `handler' is typically absent, in which case the
+default handler, `bookmark-default-handler', is used.
+
+A bookmark record is typically created by the function that's
+currently the value of variable `bookmark-make-record-function'.  For
+a given type of bookmark, that function is responsible for defining
+the appropriate PARAM-ALIST.  Most `bookmark-make-record-function'
+uses don't specify a `handler'.
+
+* FILENAME names the bookmarked file.
+* BUFFER-OR-NAME is a buffer, or the name of a buffer, that's used if
+  FILENAME isn't defined or if it refers to a nonexistent file.
+* POS is the bookmarked buffer position.
+* STR-AFTER-POS is buffer text that immediately follows POS.
+* STR-BEFORE-POS is buffer text that immediately precedes POS.
+* ANNOTATION is a user-provided string associated with the bookmark.
   See options `bookmark-use-annotations' and
   `bookmark-automatically-show-annotations'.
-HANDLER is a function that provides the `bookmark-jump' behavior for a
-specific kind of bookmark instead of the default `bookmark-default-handler'.
-This is the case for Info bookmarks, for instance.  HANDLER must accept
-a bookmark as its single argument.
+* HANDLER is a function that provides the `bookmark-jump' behavior for a
+  specific kind of bookmark, replacing the default behavior provided
+  by `bookmark-default-handler'.  This is the case for Info bookmarks,
+  for instance.  HANDLER must accept a bookmark as its sole required
+  argument.
+
+  HANDLER is responsible for providing the bookmark's behavior: doing
+  everything the bookmark is meant to do.  This includes displaying
+  the bookmark destination, if that's needed (a bookmark need not
+  display anything; it can perform any action).
+
+  To display the destination, HANDLER can call the function that's the
+  value of variable `bookmark-jump-display-function', which is set by
+  `bookmark-jump' to automatically accommodate other-window etc.
+  displaying that depends on the jump command.  For example:
+
+   (funcall bookmark-jump-display-function (current-buffer))
+
+  Or HANDLER can directly call another display function.  For example:
+
+   (switch-to-buffer-other-tab BUFFER)
+
+  Or HANDLER can invoke `bookmark-default-handler'.  That displays the
+  destination.  It then moves to the recorded buffer position, POS,
+  repositioning point, if necessary, to match the recorded context
+  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, the Info
+  handler, `Info-bookmark-jump', does this at its end:
 
-A function `bookmark-make-record-function' may define additional entries
-in PARAM-LIST that can be used by HANDLER.")
+   (bookmark-default-handler
+    `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record BOOKMARK)))
+
+  HANDLER can display the destination whenever that's most appropriate
+  - it can thus perform other actions before and after displaying.")
 
 (define-obsolete-variable-alias 'bookmarks-already-loaded
   'bookmark-bookmarks-timestamp "27.1")
@@ -1194,21 +1233,22 @@
          (bookmark-load (car bookmark-bookmarks-timestamp) t t))))
 
 
+(defvar bookmark-jump-display-function nil
+ "Function used currently to display a bookmark, or nil if no function.")
 
 (defvar bookmark-after-jump-hook nil
   "Hook run after `bookmark-jump' jumps to a bookmark.
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark-name-or-record display-function)
-  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
-DISPLAY-FUNCTION is called with the current buffer as argument.
-
-After calling DISPLAY-FUNCTION, set window point to the point specified
-by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
-and then show any annotations for this bookmark."
+  "Handle BOOKMARK-NAME-OR-RECORD, providing the bookmark's behavior.
+Save DISPLAY-FUNCTION in variable `bookmark-jump-display-function'.
+Then set the window point for the current buffer, if displayed, to
+point.  Then run `bookmark-after-jump-hook'.  Finally, if option
+`bookmark-automatically-show-annotations' is non-nil and the bookmark
+has an annotation, show the annotation."
+  (setq bookmark-jump-display-function  display-function)
   (bookmark-handle-bookmark bookmark-name-or-record)
-  (save-current-buffer
-    (funcall display-function (current-buffer)))
   (let ((win (get-buffer-window (current-buffer) 0)))
     (if win (set-window-point win (point))))
   ;; FIXME: we used to only run bookmark-after-jump-hook in
@@ -1337,11 +1377,12 @@
 
 If BMK-RECORD has a property called `buffer', it should be a live
 buffer object, and this buffer will be selected."
-  (let ((file          (bookmark-get-filename bmk-record))
-	(buf           (bookmark-prop-get bmk-record 'buffer))
-        (forward-str   (bookmark-get-front-context-string bmk-record))
-        (behind-str    (bookmark-get-rear-context-string bmk-record))
-        (place         (bookmark-get-position bmk-record)))
+  (let* ((bmk          (bookmark-get-bookmark bmk))
+         (file         (bookmark-get-filename bmk))
+	 (buf          (bookmark-prop-get bmk 'buffer))
+         (forward-str  (bookmark-get-front-context-string bmk))
+         (behind-str   (bookmark-get-rear-context-string bmk))
+         (place        (bookmark-get-position bmk)))
     (set-buffer
      (cond
       ((and file (file-readable-p file) (not (buffer-live-p buf)))
@@ -1350,6 +1391,9 @@
       ((and buf (get-buffer buf)))
       (t ;; If not, raise error.
        (signal 'bookmark-error-no-filename (list 'stringp file)))))
+    (when bookmark-jump-display-function
+      (save-current-buffer (funcall bookmark-jump-display-function
+                                    (current-buffer))))
     (if place (goto-char place))
     ;; Go searching forward first.  Then, if forward-str exists and
     ;; was found in the file, we can search backward for behind-str.

[-- Attachment #3: bookmark-patched-2022-08-11.el --]
[-- Type: application/octet-stream, Size: 108043 bytes --]

;;; bookmark.el --- set bookmarks, maybe annotate them, jump to them later -*- lexical-binding: t -*-

;; Copyright (C) 1993-1997, 2001-2022 Free Software Foundation, Inc.

;; Author: Karl Fogel <kfogel@red-bean.com>
;; Created: July, 1993
;; Keywords: convenience

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:

;; This package is for setting "bookmarks" in files.  A bookmark
;; associates a string with a location in a certain file.  Thus, you
;; can navigate your way to that location by providing the string.
;;
;; Type `M-x customize-group RET bookmark RET' for user options.

\f
;;; Code:

(require 'pp)
(require 'tabulated-list)
(require 'text-property-search)
(eval-when-compile (require 'cl-lib))

;;; Misc comments:
;;
;; The bookmark list is sorted lexically by default, but you can turn
;; this off by setting bookmark-sort-flag to nil.  If it is nil, then
;; the list will be presented in the order it is recorded
;; (chronologically), which is actually fairly useful as well.

;;; User Variables

(defgroup bookmark nil
  "Setting, annotation and jumping to bookmarks."
  :group 'matching)


(defcustom bookmark-use-annotations nil
  "If non-nil, setting a bookmark queries for an annotation in a buffer."
  :type 'boolean)


(defcustom bookmark-save-flag t
  "Controls when Emacs saves bookmarks to a file.
--> nil means never save bookmarks, except when `bookmark-save' is
    explicitly called (\\[bookmark-save]).
--> t means save bookmarks when Emacs is killed.
--> Otherwise, it should be a number that is the frequency with which
    the bookmark list is saved (i.e.: the number of times which
    Emacs's bookmark list may be modified before it is automatically
    saved.).  If it is a number, Emacs will also automatically save
    bookmarks when it is killed.

Therefore, the way to get it to save every time you make or delete a
bookmark is to set this variable to 1 (or 0, which produces the same
behavior.)

To specify the file in which to save them, modify the variable
`bookmark-default-file'."
  :type '(choice (const nil) integer (other t)))


(define-obsolete-variable-alias 'bookmark-old-default-file
  'bookmark-default-file "27.1")
(define-obsolete-variable-alias 'bookmark-file 'bookmark-default-file "27.1")
(defcustom bookmark-default-file
  (locate-user-emacs-file "bookmarks" ".emacs.bmk")
  "File in which to save bookmarks by default."
  ;; The current default file is defined via the internal variable
  ;; `bookmark-bookmarks-timestamp'.  This does not affect the value
  ;; of `bookmark-default-file'.
  :type 'file)

(defcustom bookmark-watch-bookmark-file t
  "If non-nil watch the default bookmark file.
If this file has changed on disk since it was last loaded, query the user
whether to load it again.  If the value is `silent' reload without querying.
This file defaults to `bookmark-default-file'.  But during an Emacs session,
`bookmark-load' and `bookmark-save' can redefine the current default file."
  :version "27.1"
  :type 'boolean
  :group 'bookmark)

(defcustom bookmark-version-control 'nospecial
  "Whether or not to make numbered backups of the bookmark file.
It can have four values: t, nil, `never', or `nospecial'.
The first three have the same meaning that they do for the
variable `version-control'; the value `nospecial' (the default) means
just use the value of `version-control'."
  :type '(choice (const :tag "If existing" nil)
                 (const :tag "Never" never)
                 (const :tag "Use value of option `version-control'" nospecial)
                 (other :tag "Always" t)))


(defcustom bookmark-completion-ignore-case t
  "Non-nil means bookmark functions ignore case in completion."
  :type 'boolean)


(defcustom bookmark-sort-flag t
  "This controls the bookmark display sorting.
nil means they will be displayed in LIFO order (that is, most
recently created ones come first, oldest ones come last).

`last-modified' means that bookmarks will be displayed sorted
from most recently modified to least recently modified.

Other values means that bookmarks will be displayed sorted by
bookmark name."
  :type '(choice (const :tag "By name" t)
                 (const :tag "By modified time" last-modified)
                 (const :tag "By creation time" nil)))


(defcustom bookmark-menu-confirm-deletion nil
  "Non-nil means confirm before deleting bookmarks in a bookmark menu buffer.
Nil means don't prompt for confirmation."
  :version "28.1"
  :type 'boolean)

(defcustom bookmark-automatically-show-annotations t
  "Non-nil means show annotations when jumping to a bookmark."
  :type 'boolean)

(defconst bookmark-bmenu-buffer "*Bookmark List*"
  "Name of buffer used for Bookmark List.")

(defvar bookmark-bmenu-use-header-line t
  "Non-nil means to use an immovable header line.
This is as opposed to inline text at the top of the buffer.")
(make-obsolete-variable 'bookmark-bmenu-use-header-line "no longer used." "28.1")

(defconst bookmark-bmenu-inline-header-height 2
  "Number of lines used for the *Bookmark List* header.
\(This is only significant when `bookmark-bmenu-use-header-line'
is nil.)")
(make-obsolete-variable 'bookmark-bmenu-inline-header-height "no longer used." "28.1")

(defconst bookmark-bmenu-marks-width 2
  "Number of columns (chars) used for the *Bookmark List* marks column.
This includes the annotations column.")

(defcustom bookmark-bmenu-file-column 30
  "Column at which to display filenames in a buffer listing bookmarks.
You can toggle whether files are shown with \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-toggle-filenames]."
  :type 'natnum)


(defcustom bookmark-bmenu-toggle-filenames t
  "Non-nil means show filenames when listing bookmarks.
A non-nil value may result in truncated bookmark names."
  :type 'boolean)

(defface bookmark-menu-bookmark
  '((t (:weight bold)))
  "Face used to highlight bookmark names in bookmark menu buffers.")

(defcustom bookmark-menu-length 70
  "Maximum length of a bookmark name displayed on a popup menu."
  :type 'natnum)

;; FIXME: Is it really worth a customization option?
(defcustom bookmark-search-delay 0.2
  "Time before `bookmark-bmenu-search' updates the display."
  :type  'number)

(defcustom bookmark-set-fringe-mark t
  "Whether to set a fringe mark at bookmarked lines."
  :type  'boolean
  :version "28.1")

;; FIXME: No longer used.  Should be declared obsolete or removed.
(defface bookmark-menu-heading
  '((t (:inherit font-lock-type-face)))
  "Face used to highlight the heading in bookmark menu buffers."
  :version "22.1")

(defface bookmark-face
  '((((class grayscale)
      (background light))
     :foreground "DimGray")
    (((class grayscale)
      (background dark))
     :foreground "LightGray")
    (((class color)
      (background light))
     :background "White" :foreground "DarkOrange1")
    (((class color)
      (background dark))
     :background "Black" :foreground "DarkOrange1"))
  "Face used to highlight current line."
  :version "28.1")

;;; No user-serviceable parts beyond this point.

\f
;;; Keymap stuff:

;; Set up these bindings dumping time *only*;
;; if the user alters them, don't override the user when loading bookmark.el.

;;;###autoload (keymap-set ctl-x-r-map "b" #'bookmark-jump)
;;;###autoload (keymap-set ctl-x-r-map "m" #'bookmark-set)
;;;###autoload (keymap-set ctl-x-r-map "M" #'bookmark-set-no-overwrite)
;;;###autoload (keymap-set ctl-x-r-map "l" #'bookmark-bmenu-list)

;;;###autoload
(defvar-keymap bookmark-map
  :doc "Keymap containing bindings to bookmark functions.
It is not bound to any key by default: to bind it
so that you have a bookmark prefix, just use `global-set-key' and bind a
key of your choice to variable `bookmark-map'.  All interactive bookmark
functions have a binding in this keymap."
  "x" #'bookmark-set
  "m" #'bookmark-set                            ;"m"ark
  "M" #'bookmark-set-no-overwrite               ;"M"aybe mark
  "j" #'bookmark-jump
  "g" #'bookmark-jump                           ;"g"o
  "o" #'bookmark-jump-other-window
  "5" #'bookmark-jump-other-frame
  "i" #'bookmark-insert
  "e" #'edit-bookmarks
  "f" #'bookmark-insert-location                ;"f"ind
  "r" #'bookmark-rename
  "d" #'bookmark-delete
  "D" #'bookmark-delete-all
  "l" #'bookmark-load
  "w" #'bookmark-write
  "s" #'bookmark-save)

;;;###autoload (fset 'bookmark-map bookmark-map)

\f
;;; Core variables and data structures:
(defvar bookmark-alist ()
  "Association list of bookmark names and their parameters.
Bookmark functions update the value automatically.
You probably do NOT want to change the value yourself.

The value is an alist whose elements are of the form

 (BOOKMARK-NAME . PARAM-ALIST)

or the deprecated form (BOOKMARK-NAME PARAM-ALIST).  The alist is
ordered from most recently created bookmark at the front to least
recently created bookmark at the end.

BOOKMARK-NAME is the name you gave to the bookmark when creating it.

PARAM-ALIST is an alist of bookmark information.  The order of the
entries in PARAM-ALIST is not important.  An entry with a key but null
value means the entry is not used - just as if the entry were absent.

This is the set of entries available by default; their values are
described below.

 (filename . FILENAME)
 (buf . BUFFER-OR-NAME)
 (position . POS)
 (front-context-string . STR-AFTER-POS)
 (rear-context-string  . STR-BEFORE-POS)
 (annotation . ANNOTATION)
 (handler . HANDLER)

All except `annotation' and `handler' are present by default in most
bookmarks.  `annotation' is present only if a user gives the bookmark
an annotation.  `handler' is typically absent, in which case the
default handler, `bookmark-default-handler', is used.

A bookmark record is typically created by the function that's
currently the value of variable `bookmark-make-record-function'.  For
a given type of bookmark, that function is responsible for defining
the appropriate PARAM-ALIST.  Most `bookmark-make-record-function'
uses don't specify a `handler'.

* FILENAME names the bookmarked file.
* BUFFER-OR-NAME is a buffer, or the name of a buffer, that's used if
  FILENAME isn't defined or if it refers to a nonexistent file.
* POS is the bookmarked buffer position.
* STR-AFTER-POS is buffer text that immediately follows POS.
* STR-BEFORE-POS is buffer text that immediately precedes POS.
* ANNOTATION is a user-provided string associated with the bookmark.
  See options `bookmark-use-annotations' and
  `bookmark-automatically-show-annotations'.
* HANDLER is a function that provides the `bookmark-jump' behavior for a
  specific kind of bookmark, replacing the default behavior provided
  by `bookmark-default-handler'.  This is the case for Info bookmarks,
  for instance.  HANDLER must accept a bookmark as its sole required
  argument.

  HANDLER is responsible for providing the bookmark's behavior: doing
  everything the bookmark is meant to do.  This includes displaying
  the bookmark destination, if that's needed (a bookmark need not
  display anything; it can perform any action).

  To display the destination, HANDLER can call the function that's the
  value of variable `bookmark-jump-display-function', which is set by
  `bookmark-jump' to automatically accommodate other-window etc.
  displaying that depends on the jump command.  For example:

   (funcall bookmark-jump-display-function (current-buffer))

  Or HANDLER can directly call another display function.  For example:

   (switch-to-buffer-other-tab BUFFER)

  Or HANDLER can invoke `bookmark-default-handler'.  That displays the
  destination.  It then moves to the recorded buffer position, POS,
  repositioning point, if necessary, to match the recorded context
  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, the Info
  handler, `Info-bookmark-jump', does this at its end:

   (bookmark-default-handler
    `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record BOOKMARK)))

  HANDLER can display the destination whenever that's most appropriate
  - it can thus perform other actions before and after displaying.")

(define-obsolete-variable-alias 'bookmarks-already-loaded
  'bookmark-bookmarks-timestamp "27.1")
(defvar bookmark-bookmarks-timestamp nil
  "Timestamp of current default bookmark file.
The value is actually (FILE . MODTIME), where FILE is a bookmark file that
defaults to `bookmark-default-file' and MODTIME is its modification time.")

(defvar bookmark-file-coding-system nil
  "The coding-system of the last loaded or saved bookmark file.")

(defvar-local bookmark-current-bookmark nil
  "Name of bookmark most recently used in the current file.
It is buffer local, used to make moving a bookmark forward
through a file easier.")


(defvar bookmark-alist-modification-count 0
  "Number of modifications to bookmark list since it was last saved.")


(defvar bookmark-search-size 16
  "Length of the context strings recorded on either side of a bookmark.")


(defvar bookmark-current-buffer nil
  "The buffer in which a bookmark is currently being set or renamed.
Functions that insert strings into the minibuffer use this to know
the source buffer for that information; see `bookmark-yank-word'
for example.")


(defvar bookmark-yank-point 0
  "The next point from which to pull source text for `bookmark-yank-word'.
This point is in `bookmark-current-buffer'.")


(defvar bookmark-quit-flag nil
  "Non-nil means `bookmark-bmenu-search' quits immediately.")
(make-obsolete-variable 'bookmark-quit-flag "no longer used" "27.1")

\f
;; Helper functions and macros.

(defmacro with-buffer-modified-unmodified (&rest body)
  "Run BODY while preserving the buffer's `buffer-modified-p' state."
  (let ((was-modified (make-symbol "was-modified")))
    `(let ((,was-modified (buffer-modified-p)))
       (unwind-protect
           (progn ,@body)
         (set-buffer-modified-p ,was-modified)))))

;; Only functions below, in this page and the next one (file formats),
;; need to know anything about the format of bookmark-alist entries.
;; Everyone else should go through them.

(defun bookmark-name-from-full-record (bookmark-record)
  "Return the name of BOOKMARK-RECORD.
BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
  (car bookmark-record))

(defun bookmark-type-from-full-record (bookmark-record)
  "Return then type of BOOKMARK-RECORD.
BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'. It's
type is read from the symbol property named
`bookmark-handler-type' read on the record handler function."
  (let ((handler (bookmark-get-handler bookmark-record)))
    (when (autoloadp (symbol-function handler))
      (autoload-do-load (symbol-function handler)))
    (if (symbolp handler)
        (get handler 'bookmark-handler-type)
     "")))

(defun bookmark-all-names ()
  "Return a list of all current bookmark names."
  (bookmark-maybe-load-default-file)
  (mapcar 'bookmark-name-from-full-record bookmark-alist))


(defun bookmark-get-bookmark (bookmark-name-or-record &optional noerror)
  "Return the bookmark record corresponding to BOOKMARK-NAME-OR-RECORD.
If BOOKMARK-NAME-OR-RECORD is a string, look for the corresponding
bookmark record in `bookmark-alist'; return it if found, otherwise
error.  If optional argument NOERROR is non-nil, return nil
instead of signaling an error.  Else if BOOKMARK-NAME-OR-RECORD
is already a bookmark record, just return it."
  (cond
   ((consp bookmark-name-or-record) bookmark-name-or-record)
   ((stringp bookmark-name-or-record)
    (or (assoc-string bookmark-name-or-record bookmark-alist
                      bookmark-completion-ignore-case)
        (unless noerror (error "Invalid bookmark %s"
                               bookmark-name-or-record))))))


(defun bookmark-get-bookmark-record (bookmark-name-or-record)
  "Return the record portion of BOOKMARK-NAME-OR-RECORD in `bookmark-alist'.
In other words, return all information but the name."
  (let ((alist (cdr (bookmark-get-bookmark bookmark-name-or-record))))
    ;; The bookmark objects can either look like (NAME ALIST) or
    ;; (NAME . ALIST), so we have to distinguish the two here.
    (if (and (null (cdr alist)) (consp (caar alist)))
        (car alist) alist)))


(defun bookmark-set-name (bookmark-name-or-record newname)
  "Set BOOKMARK-NAME-OR-RECORD's name to NEWNAME."
  (setcar (bookmark-get-bookmark bookmark-name-or-record) newname))

(defun bookmark-prop-get (bookmark-name-or-record prop)
  "Return the property PROP of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (cdr (assq prop (bookmark-get-bookmark-record bookmark-name-or-record))))

(defun bookmark-prop-set (bookmark-name-or-record prop val)
  "Set the property PROP of BOOKMARK-NAME-OR-RECORD to VAL."
  (let ((cell (assq
               prop (bookmark-get-bookmark-record bookmark-name-or-record))))
    (if cell
        (setcdr cell val)
      (nconc (bookmark-get-bookmark-record bookmark-name-or-record)
             (list (cons prop val))))))

(defun bookmark-get-annotation (bookmark-name-or-record)
  "Return the annotation of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'annotation))

(defun bookmark-set-annotation (bookmark-name-or-record ann)
  "Set the annotation of BOOKMARK-NAME-OR-RECORD to ANN."
  (bookmark-prop-set bookmark-name-or-record 'annotation ann))


(defun bookmark-get-filename (bookmark-name-or-record)
  "Return the full filename of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'filename))


(defun bookmark-set-filename (bookmark-name-or-record filename)
  "Set the full filename of BOOKMARK-NAME-OR-RECORD to FILENAME."
  (bookmark-prop-set bookmark-name-or-record 'filename filename))


(defun bookmark-get-position (bookmark-name-or-record)
  "Return the position (i.e.: point) of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'position))


(defun bookmark-set-position (bookmark-name-or-record position)
  "Set the position (i.e.: point) of BOOKMARK-NAME-OR-RECORD to POSITION."
  (bookmark-prop-set bookmark-name-or-record 'position position))


(defun bookmark-get-front-context-string (bookmark-name-or-record)
  "Return the front-context-string of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'front-context-string))


(defun bookmark-set-front-context-string (bookmark-name-or-record string)
  "Set the front-context-string of BOOKMARK-NAME-OR-RECORD to STRING."
  (bookmark-prop-set bookmark-name-or-record 'front-context-string string))


(defun bookmark-get-rear-context-string (bookmark-name-or-record)
  "Return the rear-context-string of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'rear-context-string))


(defun bookmark-set-rear-context-string (bookmark-name-or-record string)
  "Set the rear-context-string of BOOKMARK-NAME-OR-RECORD to STRING."
  (bookmark-prop-set bookmark-name-or-record 'rear-context-string string))


(defun bookmark-get-handler (bookmark-name-or-record)
  "Return the handler function for BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'handler))


(defun bookmark-get-last-modified (bookmark-name-or-record)
  "Return the last-modified for BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'last-modified))


(defun bookmark-update-last-modified (bookmark-name-or-record)
  "Update the last-modified date of BOOKMARK-NAME-OR-RECORD to the current time."
  (bookmark-prop-set bookmark-name-or-record 'last-modified (current-time)))


(defvar bookmark-history nil
  "The history list for bookmark functions.")

(define-fringe-bitmap 'bookmark-fringe-mark
  "\x3c\x7e\xff\xff\xff\xff\x7e\x3c")

(defun bookmark--set-fringe-mark ()
  "Apply a colorized overlay to the bookmarked location.
See user option `bookmark-set-fringe-mark'."
  (let ((bm (make-overlay (point-at-bol) (1+ (point-at-bol)))))
    (overlay-put bm 'category 'bookmark)
    (overlay-put bm 'evaporate t)
    (overlay-put bm 'before-string
                 (propertize
                  "x" 'display
                  `(left-fringe bookmark-fringe-mark bookmark-face)))))

(defun bookmark--remove-fringe-mark (bm)
  "Remove a bookmark's colorized overlay.
BM is a bookmark as returned from function `bookmark-get-bookmark'.
See user option `bookmark-set-fringe'."
  (let ((filename (cdr (assq 'filename bm)))
        (pos (cdr (assq 'position bm)))
        overlays found temp)
    (when (and pos filename)
      (setq filename (expand-file-name filename))
      (dolist (buf (buffer-list))
        (with-current-buffer buf
          (when (equal filename buffer-file-name)
            (setq overlays
                  (save-excursion
                    (goto-char pos)
                    (overlays-in (point-at-bol) (1+ (point-at-bol)))))
            (while (and (not found) (setq temp (pop overlays)))
              (when (eq 'bookmark (overlay-get temp 'category))
                (delete-overlay (setq found temp))))))))))

(defun bookmark-maybe-sort-alist ()
  "Return `bookmark-alist' for display.
If `bookmark-sort-flag' is T, then return a sorted by name copy of the alist.
If `bookmark-sort-flag' is LAST-MODIFIED, then return a sorted by last modified
copy of the alist.  Otherwise, just return `bookmark-alist', which by default
is ordered from most recently created to least recently created bookmark."
  (let ((copy (copy-alist bookmark-alist)))
    (cond ((eq bookmark-sort-flag t)
           (sort copy (lambda (x y) (string-lessp (car x) (car y)))))
          ((eq bookmark-sort-flag 'last-modified)
           (sort copy (lambda (x y)
                        (let ((tx (bookmark-get-last-modified x))
                              (ty (bookmark-get-last-modified y)))
                          (cond ((null tx) nil)
                                ((null ty) t)
                                (t (time-less-p ty tx)))))))
          (t copy))))

(defun bookmark-completing-read (prompt &optional default)
  "Prompting with PROMPT, read a bookmark name in completion.
PROMPT will get a \": \" stuck on the end no matter what, so you
probably don't want to include one yourself.
Optional arg DEFAULT is a string to return if the user input is empty.
If DEFAULT is nil then return empty string for empty input."
  (bookmark-maybe-load-default-file) ; paranoia
  (if (listp last-nonmenu-event)
      (bookmark-menu-popup-paned-menu t prompt
                                      (mapcar 'bookmark-name-from-full-record
                                              (bookmark-maybe-sort-alist)))
    (let* ((completion-ignore-case bookmark-completion-ignore-case)
           (default (unless (equal "" default) default)))
      (completing-read (format-prompt prompt default)
                       (lambda (string pred action)
                         (if (eq action 'metadata)
                             '(metadata (category . bookmark))
                             (complete-with-action
                              action bookmark-alist string pred)))
                       nil 0 nil 'bookmark-history default))))


(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
locations that should be treated specially, such as Info nodes,
news posts, images, pdf documents, etc.

The function will be called with no arguments.
It should signal a user error if it is unable to construct a record for
the current location.

The returned record should be a cons cell of the form (NAME . ALIST)
where ALIST is as described in `bookmark-alist' and may typically contain
a special cons (handler . HANDLER-FUNC) which specifies the handler function
that should be used instead of `bookmark-default-handler' to open this
bookmark.  See the documentation for `bookmark-alist' for more.

NAME is a suggested name for the constructed bookmark.  It can be nil
in which case a default heuristic will be used.  The function can also
equivalently just return ALIST without NAME.")

(defun bookmark-make-record ()
  "Return a new bookmark record (NAME . ALIST) for the current location."
  (let ((record (funcall bookmark-make-record-function)))
    ;; Set up default name if the function does not provide one.
    (unless (stringp (car record))
      (if (car record) (push nil record))
      (setcar record (or bookmark-current-bookmark (bookmark-buffer-name))))
    ;; Set up defaults.
    (bookmark-prop-set
     record 'defaults
     (delq nil (delete-dups (append (bookmark-prop-get record 'defaults)
				    (list bookmark-current-bookmark
					  (car record)
                                          (bookmark-buffer-name))))))
    record))

(defun bookmark-store (name alist no-overwrite)
  "Store the bookmark NAME with data ALIST.
If NO-OVERWRITE is non-nil and another bookmark of the same name already
exists in `bookmark-alist', record the new bookmark without throwing away the
old one."
  (bookmark-maybe-load-default-file)
  (let ((stripped-name (copy-sequence name)))
    (set-text-properties 0 (length stripped-name) nil stripped-name)
    (if (and (not no-overwrite)
             (bookmark-get-bookmark stripped-name 'noerror))
        ;; Already existing bookmark under that name and
        ;; no prefix arg means just overwrite old bookmark.
        (let ((bm (bookmark-get-bookmark stripped-name)))
          ;; First clean up if previously location was fontified.
          (when bookmark-set-fringe-mark
            (bookmark--remove-fringe-mark bm))
          ;; Modify using the new (NAME . ALIST) format.
          (setcdr bm alist))

      ;; Otherwise just put it onto the front of the list.  Either the
      ;; bookmark doesn't exist already, or there is no prefix arg.
      ;; In either case, we want the new bookmark on the front of the
      ;; list, since the list is kept in reverse order of creation.
      (push (cons stripped-name alist) bookmark-alist))

    ;; Added by db
    (setq bookmark-current-bookmark stripped-name)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))

    (setq bookmark-current-bookmark stripped-name)
    (bookmark-bmenu-surreptitiously-rebuild-list)))

(defun bookmark-make-record-default (&optional no-file no-context posn)
  "Return the record describing the location of a new bookmark.
Point should be at the buffer in which the bookmark is being set,
and normally should be at the position where the bookmark is desired,
but see the optional arguments for other possibilities.

If NO-FILE is non-nil, then only return the subset of the
record that pertains to the location within the buffer, leaving off
the part that records the filename.

If NO-CONTEXT is non-nil, do not include the front- and rear-context
strings in the record -- the position is enough.

If POSN is non-nil, record POSN as the point instead of `(point)'."
  `(,@(unless no-file `((filename . ,(bookmark-buffer-file-name))))
    ,@(unless no-context `((front-context-string
                           . ,(if (>= (- (point-max) (point))
                                      bookmark-search-size)
                                  (buffer-substring-no-properties
                                   (point)
                                   (+ (point) bookmark-search-size))
                                  nil))))
    ,@(unless no-context `((rear-context-string
                           . ,(if (>= (- (point) (point-min))
                                      bookmark-search-size)
                                  (buffer-substring-no-properties
                                   (point)
                                   (- (point) bookmark-search-size))
                                  nil))))
    (position . ,(or posn (point)))
    (last-modified . ,(current-time))))

\f
;;; File format stuff

;; *IMPORTANT NOTICE* If you are thinking about modifying (redefining)
;; the bookmark file format -- please don't.  The current format
;; should be extensible enough.  If you feel the need to change it,
;; please discuss it with other Emacs developers first.
;;
;; The format of `bookmark-alist' has changed twice in its lifetime.
;; This comment describes the three formats, FIRST, SECOND, and
;; CURRENT.
;;
;; The FIRST format was used prior to Emacs 20:
;;
;;       ((BOOKMARK-NAME (FILENAME
;;                          STRING-IN-FRONT
;;                          STRING-BEHIND
;;                          POINT))
;;        ...)
;;
;; The SECOND format was introduced in Emacs 20:
;;
;;       ((BOOKMARK-NAME ((filename   . FILENAME)
;;                        (position   . POS)
;;                        (front-context-string . STR-AFTER-POS)
;;                        (rear-context-string  . STR-BEFORE-POS)
;;                        (annotation . ANNOTATION)
;;                        (whatever   . VALUE)
;;                        ...
;;                       ))
;;        ...)
;;
;; The CURRENT format was introduced in Emacs 22:
;;
;;       ((BOOKMARK-NAME (filename   . FILENAME)
;;                       (position   . POS)
;;                       (front-context-string . STR-AFTER-POS)
;;                       (rear-context-string  . STR-BEFORE-POS)
;;                       (annotation . ANNOTATION)
;;                       (whatever   . VALUE)
;;                       ...
;;                       )
;;        ...)
;;
;; Both FIRST and SECOND have the same level of nesting: the cadr of a
;; bookmark record is a list of entry information.  FIRST and SECOND
;; differ in the form of the record information: FIRST uses a list of
;; atoms, and SECOND uses an alist.  In the FIRST format, the order of
;; the list elements matters.  In the SECOND format, the order of the
;; alist elements is unimportant.  The SECOND format facilitates the
;; addition of new kinds of elements, to support new kinds of
;; bookmarks or code evolution.
;;
;; The CURRENT format removes a level of nesting wrt FIRST and SECOND,
;; saving one cons cell per bookmark: the cadr of a bookmark record is
;; no longer a cons.  Why that change was made remains a mystery --
;; just be aware of it.  (Be aware too that this explanatory comment
;; was incorrect in Emacs 22 and Emacs 23.1.)
;;
;; To deal with the change from FIRST format to SECOND, conversion
;; code was added, which is no longer used and has been declared
;; obsolete.  See `bookmark-maybe-upgrade-file-format'.
;;
;; No conversion from SECOND to CURRENT is done.  Instead, the code
;; handles both formats OK.  It must continue to do so.
;;
;; See the doc string of `bookmark-alist' for information about the
;; elements that define a bookmark (e.g. `filename').


(defconst bookmark-file-format-version 1
  "The current version of the format used by bookmark files.
You should never need to change this.")


(defconst bookmark-end-of-version-stamp-marker
  "-*- End Of Bookmark File Format Version Stamp -*-\n"
  "This string marks the end of the version stamp in a bookmark file.")


(defun bookmark-alist-from-buffer ()
  "Return a `bookmark-alist' from the current buffer.
The buffer must of course contain bookmark format information.
Does not care from where in the buffer it is called, and does not
affect point."
  (save-excursion
    (goto-char (point-min))
    (if (search-forward bookmark-end-of-version-stamp-marker nil t)
        (read (current-buffer))
      (if buffer-file-name
          (error "File not in bookmark format: %s" buffer-file-name)
        (error "Buffer not in bookmark format: %s" (buffer-name))))))

(defun bookmark-upgrade-version-0-alist (old-list)
  "Upgrade a version 0 alist OLD-LIST to the current version."
  (declare (obsolete nil "27.1"))
  (mapcar
   (lambda (bookmark)
     (let* ((name      (car bookmark))
            (record    (car (cdr bookmark)))
            (filename  (nth 0 record))
            (front-str (nth 1 record))
            (rear-str  (nth 2 record))
            (position  (nth 3 record))
            (ann       (nth 4 record)))
       (list
        name
        `((filename             .    ,filename)
          (front-context-string .    ,(or front-str ""))
          (rear-context-string  .    ,(or rear-str  ""))
          (position             .    ,position)
          (annotation           .    ,ann)))))
   old-list))


(defun bookmark-upgrade-file-format-from-0 ()
  "Upgrade a bookmark file of format 0 (the original format) to format 1.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (let* ((reporter (make-progress-reporter
                    (format "Upgrading bookmark format from 0 to %d..."
                            bookmark-file-format-version)))
         (old-list (bookmark-alist-from-buffer))
         (new-list (with-suppressed-warnings
                       ((obsolete bookmark-upgrade-version-0-alist))
                     (bookmark-upgrade-version-0-alist old-list))))
    (delete-region (point-min) (point-max))
    (bookmark-insert-file-format-version-stamp buffer-file-coding-system)
    (pp new-list (current-buffer))
    (save-buffer)
    (goto-char (point-min))
    (progress-reporter-done reporter)))


(defun bookmark-grok-file-format-version ()
  "Return an integer which is the file-format version of this bookmark file.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (if (looking-at "^;;;;")
      (save-excursion
        (save-match-data
          (re-search-forward "[0-9]")
          (forward-char -1)
          (read (current-buffer))))
    ;; Else this is format version 0, the original one, which didn't
    ;; even have version stamps.
    0))


(defun bookmark-maybe-upgrade-file-format ()
  "Check the file-format version of this bookmark file.
If the version is not up-to-date, upgrade it automatically.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (let ((version
         (with-suppressed-warnings
             ((obsolete bookmark-grok-file-format-version))
           (bookmark-grok-file-format-version))))
    (cond
     ((= version bookmark-file-format-version)
      ) ; home free -- version is current
     ((= version 0)
      (with-suppressed-warnings
          ((obsolete bookmark-upgrade-file-format-from-0))
        (bookmark-upgrade-file-format-from-0)))
     (t
      (error "Bookmark file format version strangeness")))))


(defun bookmark-insert-file-format-version-stamp (coding)
  "Insert text indicating current version of bookmark file format.
CODING is the symbol of the coding-system in which the file is encoded."
  (if (memq (coding-system-base coding) '(undecided prefer-utf-8))
      (setq coding 'utf-8-emacs))
  (insert
   (format
    ";;;; Emacs Bookmark Format Version %d\
;;;; -*- coding: %S; mode: lisp-data -*-\n"
    bookmark-file-format-version (coding-system-base coding)))
  (insert ";;; This format is meant to be slightly human-readable;\n"
          ";;; nevertheless, you probably don't want to edit it.\n"
          ";;; "
          bookmark-end-of-version-stamp-marker))


;;; end file-format stuff

\f
;;; Core code:

(define-obsolete-function-alias 'bookmark-maybe-message 'message "27.1")

(defvar-keymap bookmark-minibuffer-read-name-map
  :parent minibuffer-local-map
  "C-w" #'bookmark-yank-word)

(defun bookmark-set-internal (prompt name overwrite-or-push)
  "Set a bookmark using specified NAME or prompting with PROMPT.
The bookmark is set at the current location.

If NAME is non-nil, use it as the name of the new bookmark.  In
this case, the value of PROMPT is ignored.

Otherwise, prompt the user for the bookmark name.  Begin the
interactive prompt with PROMPT, followed by a space, a generated
default name in parentheses, a colon and a space.

OVERWRITE-OR-PUSH controls what happens if there is already a
bookmark with the same name: nil means signal an error;
`overwrite' means replace any existing bookmark; `push' means
push the new bookmark onto the bookmark alist.  The `push'
behavior means that among bookmarks with the same name, this most
recently set one becomes the one in effect, but the others are
still there, in order, if the topmost one is ever deleted."
  (unwind-protect
       (let* ((record (bookmark-make-record))
              ;; `defaults' is a transient element of the
              ;; extensible format described above in the section
              ;; `File format stuff'.  Bookmark record functions
              ;; can use it to specify a list of default values
              ;; accessible via M-n while reading a bookmark name.
              (defaults (bookmark-prop-get record 'defaults))
              (default (if (consp defaults) (car defaults) defaults)))

         (if defaults
             ;; Don't store default values in the record.
             (setq record (assq-delete-all 'defaults record))
           ;; When no defaults in the record, use its first element.
           (setq defaults (car record) default defaults))

         (bookmark-maybe-load-default-file)
         ;; Don't set `bookmark-yank-point' and `bookmark-current-buffer'
         ;; if they have been already set in another buffer. (e.g gnus-art).
         (unless (and bookmark-yank-point
                      bookmark-current-buffer)
           (setq bookmark-yank-point (point))
           (setq bookmark-current-buffer (current-buffer)))

         (let ((str
                (or name
                    (read-from-minibuffer
                     (format-prompt prompt default)
                     nil
                     bookmark-minibuffer-read-name-map
                     nil nil defaults))))
           (and (string-equal str "") (setq str default))

           (cond
            ((eq overwrite-or-push nil)
             (if (bookmark-get-bookmark str t)
                 (error "A bookmark named \"%s\" already exists" str)
               (bookmark-store str (cdr record) nil)))
            ((eq overwrite-or-push 'overwrite)
             (bookmark-store str (cdr record) nil))
            ((eq overwrite-or-push 'push)
             (bookmark-store str (cdr record) t))
            (t
             (error "Unrecognized value for `overwrite-or-push': %S"
                    overwrite-or-push)))

           ;; Ask for an annotation buffer for this bookmark
           (when bookmark-use-annotations
             (bookmark-edit-annotation str))
           (when bookmark-set-fringe-mark
             (bookmark--set-fringe-mark))))
    (setq bookmark-yank-point nil)
    (setq bookmark-current-buffer nil)))


;;;###autoload
(defun bookmark-set (&optional name no-overwrite)
  "Set a bookmark named NAME at the current location.
If NAME is nil, then prompt the user.

With a prefix arg (non-nil NO-OVERWRITE), do not overwrite any
existing bookmark that has the same name as NAME, but instead push the
new bookmark onto the bookmark alist.  The most recently set bookmark
with name NAME is thus the one in effect at any given time, but the
others are still there, should the user decide to delete the most
recent one.

To yank words from the text of the buffer and use them as part of the
bookmark name, type \\<bookmark-minibuffer-read-name-map>\
\\[bookmark-yank-word] while setting a bookmark.  Successive \
\\[bookmark-yank-word]'s
yank successive words.

Typing \\[universal-argument] inserts (at the bookmark name prompt) the name of the last
bookmark used in the document where the new bookmark is being set;
this helps you use a single bookmark name to track progress through a
large document.  If there is no prior bookmark for this document, then
\\[universal-argument] inserts an appropriate name based on the buffer or file.

Use \\[bookmark-delete] to remove bookmarks (you give it a name and
it removes only the first instance of a bookmark with that name from
the list of bookmarks.)"
  (interactive (list nil current-prefix-arg))
  (let ((prompt
         (if no-overwrite "Append bookmark named" "Set bookmark named")))
    (bookmark-set-internal prompt name (if no-overwrite 'push 'overwrite))))

;;;###autoload
(defun bookmark-set-no-overwrite (&optional name push-bookmark)
  "Set a bookmark named NAME at the current location.
If NAME is nil, then prompt the user.

If a bookmark named NAME already exists and prefix argument
PUSH-BOOKMARK is non-nil, then push the new bookmark onto the
bookmark alist.  Pushing it means that among bookmarks named
NAME, this one becomes the one in effect, but the others are
still there, in order, and become effective again if the user
ever deletes the most recent one.

Otherwise, if a bookmark named NAME already exists but PUSH-BOOKMARK
is nil, raise an error.

To yank words from the text of the buffer and use them as part of the
bookmark name, type \\<bookmark-minibuffer-read-name-map>\
\\[bookmark-yank-word] while setting a bookmark.  Successive \
\\[bookmark-yank-word]'s
yank successive words.

Typing \\[universal-argument] inserts (at the bookmark name prompt) the name of the last
bookmark used in the document where the new bookmark is being set;
this helps you use a single bookmark name to track progress through a
large document.  If there is no prior bookmark for this document, then
\\[universal-argument] inserts an appropriate name based on the buffer or file.

Use \\[bookmark-delete] to remove bookmarks (you give it a name and
it removes only the first instance of a bookmark with that name from
the list of bookmarks.)"
  (interactive (list nil current-prefix-arg))
  (bookmark-set-internal "Set bookmark" name (if push-bookmark 'push nil)))


(defun bookmark-kill-line (&optional newline-too)
  "Kill from point to end of line.
If optional arg NEWLINE-TOO is non-nil, delete the newline too.
Does not affect the kill ring."
  (let ((eol (line-end-position)))
    (delete-region (point) eol)
    (when (and newline-too (= (following-char) ?\n))
      (delete-char 1))))

(defvar-local bookmark-annotation-name nil
  "Name of bookmark under edit in `bookmark-edit-annotation-mode'.")

(defvar-local bookmark--annotation-from-bookmark-list nil
  "If non-nil, `bookmark-edit-annotation-mode' should return to bookmark list.")

(defun bookmark-default-annotation-text (bookmark-name)
  "Return default annotation text for BOOKMARK-NAME.
The default annotation text is simply some text explaining how to use
annotations."
  (concat (format-message
           "#  Type the annotation for bookmark `%s' here.\n"
           bookmark-name)
	  (format-message
           "#  All lines which start with a `#' will be deleted.\n")
          (substitute-command-keys
           (concat
            "#  Type \\[bookmark-edit-annotation-confirm] when done.  Type "
            "\\[bookmark-edit-annotation-cancel] to cancel.\n#\n"))
	  "#  Author: " (user-full-name) " <" (user-login-name) "@"
	  (system-name) ">\n"
          "#  Date:   " (current-time-string) "\n"))


(defvar bookmark-edit-annotation-text-func 'bookmark-default-annotation-text
  "Function to return default text to use for a bookmark annotation.
It takes one argument, the name of the bookmark, as a string.")

(defvar-keymap bookmark-edit-annotation-mode-map
  :doc "Keymap for editing an annotation of a bookmark."
  :parent text-mode-map
  "C-c C-c" #'bookmark-edit-annotation-confirm
  "C-c C-k" #'bookmark-edit-annotation-cancel)

(defun bookmark-insert-annotation (bookmark-name-or-record)
  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
  (when (not (bookmark-get-bookmark bookmark-name-or-record t))
    (error "Invalid bookmark: %s" bookmark-name-or-record))
  (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
  (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
    (if (and annotation (not (string-equal annotation "")))
	(insert annotation))))

(define-derived-mode bookmark-edit-annotation-mode
  text-mode "Edit Bookmark Annotation"
  "Mode for editing the annotation of bookmarks.
\\<bookmark-edit-annotation-mode-map>\
When you have finished composing, type \\[bookmark-edit-annotation-confirm] \
or \\[bookmark-edit-annotation-cancel] to cancel.

\\{bookmark-edit-annotation-mode-map}")

(defmacro bookmark-edit-annotation--maybe-display-list (&rest body)
  "Display bookmark list after editing if appropriate."
  `(let ((from-bookmark-list bookmark--annotation-from-bookmark-list)
         (old-buffer (current-buffer)))
     ,@body
     (quit-window)
     (bookmark-bmenu-surreptitiously-rebuild-list)
     (when from-bookmark-list
       (pop-to-buffer (get-buffer bookmark-bmenu-buffer))
       (goto-char (point-min))
       (bookmark-bmenu-bookmark))
     (kill-buffer old-buffer)))

(defun bookmark-edit-annotation-cancel ()
  "Cancel the current annotation edit."
  (interactive nil bookmark-edit-annotation-mode)
  (bookmark-edit-annotation--maybe-display-list
   (message "Canceled by user")))

(defun bookmark-edit-annotation-confirm ()
  "Use buffer contents as annotation for a bookmark.
Lines beginning with `#' are ignored."
  (interactive nil bookmark-edit-annotation-mode)
  (if (not (derived-mode-p 'bookmark-edit-annotation-mode))
      (error "Not in bookmark-edit-annotation-mode"))
  (goto-char (point-min))
  (while (< (point) (point-max))
    (if (= (following-char) ?#)
        (bookmark-kill-line t)
      (forward-line 1)))
  ;; Take no chances with text properties.
  (bookmark-edit-annotation--maybe-display-list
   (let ((annotation (buffer-substring-no-properties (point-min) (point-max)))
         (bookmark-name bookmark-annotation-name))
     (bookmark-set-annotation bookmark-name annotation)
     (bookmark-update-last-modified bookmark-name)
     (setq bookmark-alist-modification-count
           (1+ bookmark-alist-modification-count))
     (message "Annotation updated for \"%s\"" bookmark-name))))


(defun bookmark-edit-annotation (bookmark-name-or-record &optional from-bookmark-list)
  "Pop up a buffer for editing bookmark BOOKMARK-NAME-OR-RECORD's annotation.
If optional argument FROM-BOOKMARK-LIST is non-nil, return to the
bookmark list when editing is done."
  (pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
  (bookmark-edit-annotation-mode)
  (bookmark-insert-annotation bookmark-name-or-record)
  (setq bookmark--annotation-from-bookmark-list from-bookmark-list)
  (setq bookmark-annotation-name bookmark-name-or-record))


(defun bookmark-buffer-name ()
  "Return the name of the current buffer in a form usable as a bookmark name.
If the buffer is associated with a file or directory, use that name."
  (cond
   ;; Or are we a file?
   (buffer-file-name (file-name-nondirectory buffer-file-name))
   ;; Or are we a directory?
   ((and (boundp 'dired-directory) dired-directory)
    (let* ((dirname (if (stringp dired-directory)
                        dired-directory
                      (car dired-directory)))
           (idx (1- (length dirname))))
      ;; Strip the trailing slash.
      (if (= ?/ (aref dirname idx))
          (file-name-nondirectory (substring dirname 0 idx))
        ;; Else return the current-buffer
        (buffer-name (current-buffer)))))
   ;; If all else fails, use the buffer's name.
   (t
    (buffer-name (current-buffer)))))


(defun bookmark-yank-word ()
  "Get the next word from buffer `bookmark-current-buffer' and append
it to the name of the bookmark currently being set, advancing
`bookmark-yank-point' by one word."
  (interactive)
  (let ((string (with-current-buffer bookmark-current-buffer
                  (goto-char bookmark-yank-point)
                  (buffer-substring-no-properties
                   (point)
                   (progn
                     (forward-word 1)
                     (setq bookmark-yank-point (point)))))))
    (insert string)))

(defun bookmark-buffer-file-name ()
  "Return the current buffer's file in a way useful for bookmarks."
  ;; Abbreviate the path, both so it's shorter and so it's more
  ;; portable.  E.g., the user's home dir might be a different
  ;; path on different machines, but "~/" will still reach it.
  (abbreviate-file-name
   (cond
    (buffer-file-name buffer-file-name)
    ((and (boundp 'dired-directory) dired-directory)
     (if (stringp dired-directory)
         dired-directory
       (car dired-directory)))
    (t (error "Buffer not visiting a file or directory")))))

(defvar bookmark--watch-already-asked-mtime nil
  "Mtime for which we already queried about reloading.")

(defun bookmark--watch-file-already-queried-p (new-mtime)
  ;; Don't ask repeatedly if user already said "no" to reloading a
  ;; file with this mtime:
  (prog1 (time-equal-p new-mtime bookmark--watch-already-asked-mtime)
    (setq bookmark--watch-already-asked-mtime new-mtime)))

(defun bookmark-maybe-load-default-file ()
  "If bookmarks have not been loaded from the default place, load them."
  (cond ((and (not bookmark-bookmarks-timestamp)
              (null bookmark-alist)
              (file-readable-p bookmark-default-file)
              (bookmark-load bookmark-default-file t t)))
        ((and bookmark-watch-bookmark-file
              (let ((new-mtime (nth 5 (file-attributes
                                       (car bookmark-bookmarks-timestamp))))
                    (old-mtime (cdr bookmark-bookmarks-timestamp)))
		(and (not (time-equal-p new-mtime old-mtime))
                     (not (bookmark--watch-file-already-queried-p new-mtime))
                     (or (eq 'silent bookmark-watch-bookmark-file)
                         (yes-or-no-p
                          (format "Bookmarks %s changed on disk.  Reload? "
                                  (car bookmark-bookmarks-timestamp)))))))
         (bookmark-load (car bookmark-bookmarks-timestamp) t t))))


(defvar bookmark-jump-display-function nil
 "Function used currently to display a bookmark, or nil if no function.")

(defvar bookmark-after-jump-hook nil
  "Hook run after `bookmark-jump' jumps to a bookmark.
Useful for example to unhide text in `outline-mode'.")

(defun bookmark--jump-via (bookmark-name-or-record display-function)
  "Handle BOOKMARK-NAME-OR-RECORD, providing the bookmark's behavior.
Save DISPLAY-FUNCTION in variable `bookmark-jump-display-function'.
Then set the window point for the current buffer, if displayed, to
point.  Then run `bookmark-after-jump-hook'.  Finally, if option
`bookmark-automatically-show-annotations' is non-nil and the bookmark
has an annotation, show the annotation."
  (setq bookmark-jump-display-function  display-function)
  (bookmark-handle-bookmark bookmark-name-or-record)
  (let ((win (get-buffer-window (current-buffer) 0)))
    (if win (set-window-point win (point))))
  ;; FIXME: we used to only run bookmark-after-jump-hook in
  ;; `bookmark-jump' itself, but in none of the other commands.
  (when bookmark-set-fringe-mark
    (let ((overlays (overlays-in (point-at-bol) (1+ (point-at-bol))))
          temp found)
      (while (and (not found) (setq temp (pop overlays)))
        (when (eq 'bookmark (overlay-get temp 'category))
          (setq found t)))
      (unless found
        (bookmark--set-fringe-mark))))
  (run-hooks 'bookmark-after-jump-hook)
  (if bookmark-automatically-show-annotations
      ;; if there is an annotation for this bookmark,
      ;; show it in a buffer.
      (bookmark-show-annotation bookmark-name-or-record)))


;;;###autoload
(defun bookmark-jump (bookmark &optional display-func)
  "Jump to bookmark BOOKMARK (a point in some file).
You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
bookmarks.  See help on function `bookmark-load' for more about
this.

If the file pointed to by BOOKMARK no longer exists, you will be asked
if you wish to give the bookmark a new location, and `bookmark-jump'
will then jump to the new location, as well as recording it in place
of the old one in the permanent bookmark record.

BOOKMARK is usually a bookmark name (a string).  It can also be a
bookmark record, but this is usually only done by programmatic callers.

If DISPLAY-FUNC is non-nil, it is a function to invoke to display the
bookmark.  It defaults to `pop-to-buffer-same-window'.  A typical value for
DISPLAY-FUNC would be `switch-to-buffer-other-window'."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark"
				   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.
  (bookmark--jump-via bookmark (or display-func 'pop-to-buffer-same-window)))


;;;###autoload
(defun bookmark-jump-other-window (bookmark)
  "Jump to BOOKMARK in another window.  See `bookmark-jump' for more."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark (in another window)"
                                   bookmark-current-bookmark)))
  (bookmark-jump bookmark 'switch-to-buffer-other-window))

;;;###autoload
(defun bookmark-jump-other-frame (bookmark)
  "Jump to BOOKMARK in another frame.  See `bookmark-jump' for more."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark (in another frame)"
                                   bookmark-current-bookmark)))
  (let ((pop-up-frames t))
    (bookmark-jump-other-window bookmark)))

(defun bookmark-handle-bookmark (bookmark-name-or-record)
  "Call BOOKMARK-NAME-OR-RECORD's handler or `bookmark-default-handler'
if it has none.  This changes current buffer and point and returns nil,
or signals a `file-error'.

If BOOKMARK-NAME-OR-RECORD has no file, this is a no-op.  If
BOOKMARK-NAME-OR-RECORD has a file, but that file no longer exists,
then offer interactively to relocate BOOKMARK-NAME-OR-RECORD."
  (condition-case err
      (funcall (or (bookmark-get-handler bookmark-name-or-record)
                   'bookmark-default-handler)
               (bookmark-get-bookmark bookmark-name-or-record))
    (bookmark-error-no-filename         ;file-error
     ;; We were unable to find the marked file, so ask if user wants to
     ;; relocate the bookmark, else remind them to consider deletion.
     (when (stringp bookmark-name-or-record)
       ;; `bookmark-name-or-record' can be either a bookmark name
       ;; (from `bookmark-alist')  or a bookmark object.  If it's an
       ;; object, we assume it's a bookmark used internally by some
       ;; other package.
       (let ((file (bookmark-get-filename bookmark-name-or-record)))
         (when file        ;Don't know how to relocate if there's no `file'.
           ;; If file is not a dir, directory-file-name just returns file.
           (let ((display-name (directory-file-name file)))
             (ding)
             ;; Dialog boxes can accept a file target, but usually don't
             ;; know how to accept a directory target (at least, this
             ;; is true in Gnome on GNU/Linux, and Bug#4230 says it's
             ;; true on Windows as well).  So we suppress file dialogs
             ;; when relocating.
             (let ((use-dialog-box nil)
                   (use-file-dialog nil))
               (if (y-or-n-p (concat display-name " nonexistent.  Relocate \""
                                     bookmark-name-or-record "\"? "))
                   (progn
                     (bookmark-relocate bookmark-name-or-record)
                     ;; Try again.
                     (funcall (or (bookmark-get-handler bookmark-name-or-record)
                                  'bookmark-default-handler)
                              (bookmark-get-bookmark bookmark-name-or-record)))
                 (message
                  "Bookmark not relocated; consider removing it (%s)."
                  bookmark-name-or-record)
                 (signal (car err) (cdr err))))))))))
  ;; Added by db.
  (when (stringp bookmark-name-or-record)
    (setq bookmark-current-bookmark bookmark-name-or-record))
  nil)

(define-error 'bookmark-errors
  "Bookmark error")
(define-error 'bookmark-error-no-filename
  "Bookmark has no associated file (or directory)" 'bookmark-errors)

(defun bookmark-default-handler (bmk-record)
  "Default handler to jump to a particular bookmark location.
BMK-RECORD is a bookmark record, not a bookmark name (i.e., not a string).
Changes current buffer and point and returns nil, or signals a `file-error'.

If BMK-RECORD has a property called `buffer', it should be a live
buffer object, and this buffer will be selected."
  (let* ((bmk          (bookmark-get-bookmark bmk))
         (file         (bookmark-get-filename bmk))
	 (buf          (bookmark-prop-get bmk 'buffer))
         (forward-str  (bookmark-get-front-context-string bmk))
         (behind-str   (bookmark-get-rear-context-string bmk))
         (place        (bookmark-get-position bmk)))
    (set-buffer
     (cond
      ((and file (file-readable-p file) (not (buffer-live-p buf)))
       (find-file-noselect file))
      ;; No file found.  See if buffer BUF has been created.
      ((and buf (get-buffer buf)))
      (t ;; If not, raise error.
       (signal 'bookmark-error-no-filename (list 'stringp file)))))
    (when bookmark-jump-display-function
      (save-current-buffer (funcall bookmark-jump-display-function
                                    (current-buffer))))
    (if place (goto-char place))
    ;; Go searching forward first.  Then, if forward-str exists and
    ;; was found in the file, we can search backward for behind-str.
    ;; Rationale is that if text was inserted between the two in the
    ;; file, it's better to be put before it so you can read it,
    ;; rather than after and remain perhaps unaware of the changes.
    (when (and forward-str (search-forward forward-str (point-max) t))
      (goto-char (match-beginning 0)))
    (when (and behind-str (search-backward behind-str (point-min) t))
      (goto-char (match-end 0)))
    nil))

;;;###autoload
(defun bookmark-relocate (bookmark-name)
  "Relocate BOOKMARK-NAME to another file, reading file name with minibuffer.

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))
         (newloc (abbreviate-file-name
                  (expand-file-name
                   (read-file-name
                    (format "Relocate %s to: " bookmark-name)
                    (file-name-directory bmrk-filename))))))
    (bookmark-set-filename bookmark-name newloc)
    (bookmark-update-last-modified bookmark-name)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))
    (bookmark-bmenu-surreptitiously-rebuild-list)))


;;;###autoload
(defun bookmark-insert-location (bookmark-name &optional no-history)
  "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'."
  (interactive (list (bookmark-completing-read "Insert bookmark location")))
  (or no-history (bookmark-maybe-historicize-string bookmark-name))
  (insert (bookmark-location bookmark-name)))

;;;###autoload
(defalias 'bookmark-locate 'bookmark-insert-location)

(defun bookmark-location (bookmark-name-or-record)
  "Return a description of the location of BOOKMARK-NAME-OR-RECORD."
  (bookmark-maybe-load-default-file)
  ;; We could call the `handler' and ask for it to construct a description
  ;; dynamically: it would open up several new possibilities, but it
  ;; would have the major disadvantage of forcing to load each and
  ;; every handler when the user calls bookmark-menu.
  (or (bookmark-prop-get bookmark-name-or-record 'location)
      (bookmark-get-filename bookmark-name-or-record)
      "-- Unknown location --"))

;;;###autoload
(defun bookmark-rename (old-name &optional new-name)
  "Change the name of OLD-NAME bookmark to NEW-NAME name.
If called from keyboard, prompt for OLD-NAME and NEW-NAME.
If called from menubar, select OLD-NAME from a menu and prompt for NEW-NAME.

If called from Lisp, prompt for NEW-NAME if only OLD-NAME was passed
as an argument.  If called with two strings, then no prompting is done.
You must pass at least OLD-NAME when calling from Lisp.

While you are entering the new name, consecutive \
\\<bookmark-minibuffer-read-name-map>\\[bookmark-yank-word]'s insert
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))
  (setq bookmark-current-buffer (current-buffer))
  (let ((final-new-name
         (or new-name   ; use second arg, if non-nil
             (read-from-minibuffer
              "New name: "
              nil
              (define-keymap
                :parent minibuffer-local-map
                "C-w" #'bookmark-yank-word)
              nil
              'bookmark-history))))
    (bookmark-set-name old-name final-new-name)
    (bookmark-update-last-modified final-new-name)
    (setq bookmark-current-bookmark final-new-name)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))))


;;;###autoload
(defun bookmark-insert (bookmark-name)
  "Insert the text of the file pointed to by bookmark BOOKMARK-NAME.
BOOKMARK-NAME is a bookmark name (a string), not a bookmark record.

You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
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
	 (save-current-buffer
           (bookmark-handle-bookmark bookmark-name)
	   (buffer-string))))
    (insert str-to-insert)
    (push-mark)
    (goto-char orig-point)))


;;;###autoload
(defun bookmark-delete (bookmark-name &optional batch)
  "Delete BOOKMARK-NAME from the bookmark list.

Removes only the first instance of a bookmark with that name.  If
there are one or more other bookmarks with the same name, they will
not be deleted.  Defaults to the \"current\" bookmark (that is, the
one most recently used in this file, if any).
Optional second arg BATCH means don't update the bookmark list buffer,
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)
    (setq bookmark-alist (delq will-go bookmark-alist))
    ;; Added by db, nil bookmark-current-bookmark if the last
    ;; occurrence has been deleted
    (or (bookmark-get-bookmark bookmark-current-bookmark 'noerror)
        (setq bookmark-current-bookmark nil)))
  (unless batch
    (bookmark-bmenu-surreptitiously-rebuild-list))
  (setq bookmark-alist-modification-count
        (1+ bookmark-alist-modification-count))
  (when (bookmark-time-to-save-p)
    (bookmark-save)))


;;;###autoload
(defun bookmark-delete-all (&optional no-confirm)
  "Permanently delete all bookmarks.
If optional argument NO-CONFIRM is non-nil, don't ask for
confirmation."
  (interactive "P")
  ;; We don't use `bookmark-menu-confirm-deletion' here because that
  ;; variable is specifically to control confirmation prompting in a
  ;; bookmark menu buffer, where the user has the marked-for-deletion
  ;; bookmarks arrayed in front of them and might have accidentally
  ;; hit the key that executes the deletions.  The UI situation here
  ;; is quite different, by contrast: the user got to this point by a
  ;; sequence of keystrokes unlikely to be typed by chance.
  (when (or no-confirm
            (yes-or-no-p "Permanently delete all bookmarks? "))
    (bookmark-maybe-load-default-file)
    (setq bookmark-alist-modification-count
          (+ bookmark-alist-modification-count (length bookmark-alist)))
    (setq bookmark-alist nil)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (when (bookmark-time-to-save-p)
      (bookmark-save))))


(defun bookmark-time-to-save-p (&optional final-time)
  "Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
is being killed, so save even if `bookmark-save-flag' is a number and
is greater than `bookmark-alist-modification-count'."
  ;; By Gregory M. Saunders <saunders{_AT_}cis.ohio-state.edu>
  (cond (final-time
	 (and (> bookmark-alist-modification-count 0)
	      bookmark-save-flag))
	((numberp bookmark-save-flag)
	 (>= bookmark-alist-modification-count bookmark-save-flag))
	(t
	 nil)))


;;;###autoload
(defun bookmark-write ()
  "Write bookmarks to a file (reading the file name with the minibuffer)."
  (declare (interactive-only bookmark-save))
  (interactive)
  (bookmark-maybe-load-default-file)
  (bookmark-save t))


;;;###autoload
(defun bookmark-save (&optional parg file make-default)
  "Save currently defined bookmarks in FILE.
FILE defaults to `bookmark-default-file'.
With prefix PARG, query user for a file to save in.
If MAKE-DEFAULT is non-nil (interactively with prefix \\[universal-argument] \\[universal-argument])
the file we save in becomes the new default in the current Emacs
session (without affecting the value of `bookmark-default-file'.).

When you want to load in the bookmarks from a file, use
`bookmark-load', \\[bookmark-load].  That function will prompt you
for a file, defaulting to the file defined by variable
`bookmark-default-file'."
  (interactive
   (list current-prefix-arg nil (equal '(16) current-prefix-arg)))
  (bookmark-maybe-load-default-file)
  (unless file
    (setq file
          (let ((default (or (car bookmark-bookmarks-timestamp)
                             bookmark-default-file)))
            (if parg
                ;; This should be part of the `interactive' spec.
                (read-file-name (format "File to save bookmarks in: (%s) "
                                        default)
                                (file-name-directory default) default)
              default))))
  (bookmark-write-file file)
  ;; Signal that we have synced the bookmark file by setting this to 0.
  ;; If there was an error at any point before, it will not get set,
  ;; which is what we want.
  (setq bookmark-alist-modification-count 0)
  (if make-default
      (let ((default (expand-file-name file)))
        (setq bookmark-bookmarks-timestamp
              (cons default (nth 5 (file-attributes default)))))
    (let ((default (car bookmark-bookmarks-timestamp)))
      (if (string= default (expand-file-name file))
          (setq bookmark-bookmarks-timestamp
                (cons default (nth 5 (file-attributes default))))))))

\f
(defun bookmark-write-file (file)
  "Write `bookmark-alist' to FILE."
  (let ((reporter (make-progress-reporter
		   (format "Saving bookmarks to file %s..." file))))
    (with-current-buffer (get-buffer-create " *Bookmarks*")
      (goto-char (point-min))
      (delete-region (point-min) (point-max))
      (let ((coding-system-for-write
	     (or coding-system-for-write
		 bookmark-file-coding-system 'utf-8-emacs))
	    (print-length nil)
	    (print-level nil)
	    ;; See bug #12503 for why we bind `print-circle'.  Users
	    ;; can define their own bookmark types, which can result in
	    ;; arbitrary Lisp objects being stored in bookmark records,
	    ;; and some users create objects containing circularities.
	    (print-circle t))
	(insert "(")
	;; Rather than a single call to `pp' we make one per bookmark.
	;; Apparently `pp' has a poor algorithmic complexity, so this
	;; scales a lot better.  bug#4485.
	(dolist (i bookmark-alist) (pp i (current-buffer)))
	(insert ")\n")
	;; Make sure the specified encoding can safely encode the
	;; bookmarks.  If it cannot, suggest utf-8-emacs as default.
	(with-coding-priority '(utf-8-emacs)
	  (setq coding-system-for-write
		(select-safe-coding-system (point-min) (point-max)
					   (list t coding-system-for-write))))
	(goto-char (point-min))
	(bookmark-insert-file-format-version-stamp coding-system-for-write)
	(let ((version-control
	       (cond
		((null bookmark-version-control) nil)
		((eq 'never bookmark-version-control) 'never)
		((eq 'nospecial bookmark-version-control) version-control)
		(t t))))
	  (condition-case nil
              ;; There was a stretch of time (about 15 years) when we
              ;; used `write-region' below instead of `write-file',
              ;; before going back to `write-file' again.  So if you're
              ;; considering changing it to `write-region', please see
              ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=12507.
              ;; That bug tells the story of how we first started using
              ;; `write-region' in 2005...
              ;;
              ;;   commit a506054af7cd86a63fda996056c09310966f32ef
              ;;   Author:     Karl Fogel <kfogel@red-bean.com>
              ;;   AuthorDate: Sat Nov 12 20:30:22 2005 +0000
              ;;
              ;;       (bookmark-write-file): Don't visit the
              ;;       destination file, just write the data to it
              ;;       using write-region.  This is similar to
              ;;       2005-05-29T08:36:26Z!rms@gnu.org of saveplace.el,
              ;;       but with an additional change to avoid visiting
              ;;       the  file in the first place.
              ;;
              ;; ...and of how further inquiry led us to investigate (in
              ;; 2012 and then again in 2020) and eventually decide that
              ;; matching the saveplace.el change doesn't make sense for
              ;; bookmark.el.  Therefore we reverted to `write-file',
              ;; which means numbered backups may now be created,
              ;; depending on `bookmark-version-control' as per above.
	      (write-file file)
	    (file-error (message "Can't write %s" file)))
	  (setq bookmark-file-coding-system coding-system-for-write)
	  (kill-buffer (current-buffer))
	  (progress-reporter-done reporter))))))


(defun bookmark-import-new-list (new-list)
  "Add NEW-LIST of bookmarks to `bookmark-alist'.
Rename new bookmarks as needed using suffix \"<N>\" (N=1,2,3...), when
they conflict with existing bookmark names."
  (let ((names (bookmark-all-names)))
    (dolist (full-record new-list)
      (bookmark-maybe-rename full-record names)
      (setq bookmark-alist (nconc bookmark-alist (list full-record)))
      (push (bookmark-name-from-full-record full-record) names))))


(defun bookmark-maybe-rename (full-record names)
  "Rename bookmark FULL-RECORD if its current name is already used.
This is a helper for `bookmark-import-new-list'."
  (let ((found-name (bookmark-name-from-full-record full-record)))
    (if (member found-name names)
        ;; We've got a conflict, so generate a new name
        (let ((count 2)
              (new-name found-name))
          (while (member new-name names)
            (setq new-name (concat found-name (format "<%d>" count)))
            (setq count (1+ count)))
          (bookmark-set-name full-record new-name)))))


;;;###autoload
(defun bookmark-load (file &optional overwrite no-msg default)
  "Load bookmarks from FILE (which must be in bookmark format).
Appends loaded bookmarks to the front of the list of bookmarks.
If argument OVERWRITE is non-nil, existing bookmarks are destroyed.
Optional third arg NO-MSG means don't display any messages while loading.
If DEFAULT is non-nil make FILE the new bookmark file to watch.
Interactively, a prefix arg makes OVERWRITE and DEFAULT non-nil.

If you load a file that doesn't contain a proper bookmark alist, you
will corrupt Emacs's bookmark list.  Generally, you should only load
in files that were created with the bookmark functions in the first
place.  Your own personal bookmark file, specified by the variable
`bookmark-default-file', is maintained automatically by Emacs; you
shouldn't need to load it explicitly.

If you load a file containing bookmarks with the same names as
bookmarks already present in your Emacs, the new bookmarks will get
unique numeric suffixes \"<2>\", \"<3>\", etc."
  (interactive
   (let ((default (abbreviate-file-name
		   (or (car bookmark-bookmarks-timestamp)
		       (expand-file-name bookmark-default-file))))
	 (prefix current-prefix-arg))
     (list (read-file-name (format "Load bookmarks from: (%s) " default)
			   (file-name-directory default) default 'confirm)
	   prefix nil prefix)))
  (let* ((file (expand-file-name file))
	 (afile (abbreviate-file-name file)))
    (unless (file-readable-p file)
      (user-error "Cannot read bookmark file %s" afile))
    (let ((reporter
	   (unless no-msg
	     (make-progress-reporter
	      (format "Loading bookmarks from %s..." file)))))
      (with-current-buffer (let (enable-local-variables)
			     (find-file-noselect file))
	(goto-char (point-min))
	(let ((blist (bookmark-alist-from-buffer)))
	  (unless (listp blist)
	    (error "Invalid bookmark list in %s" file))
	  ;; RW: Upon loading the bookmarks, we could add to each bookmark
	  ;; in `bookmark-alist' an extra key `bookmark-file', so that
	  ;; upon reloading the bookmarks with OVERWRITE non-nil,
	  ;; we overwrite only those bookmarks for which the key `bookmark-file'
	  ;; matches FILE.  `bookmark-save' can ignore this key.
	  ;; Would this be a useful option?
	  (if overwrite
	      (setq bookmark-alist blist
		    bookmark-alist-modification-count 0)
	    (bookmark-import-new-list blist)
	    (setq bookmark-alist-modification-count
		  (1+ bookmark-alist-modification-count)))
	  (if (or default
		  (string= file (or (car bookmark-bookmarks-timestamp)
				    (expand-file-name bookmark-default-file))))
	      (setq bookmark-bookmarks-timestamp
		    (cons file (nth 5 (file-attributes file)))))
	  (bookmark-bmenu-surreptitiously-rebuild-list)
	  (setq bookmark-file-coding-system buffer-file-coding-system))
	(kill-buffer (current-buffer)))
      (unless no-msg
	(progress-reporter-done reporter)))))

\f
;;; Code supporting the dired-like bookmark list.
;; Prefix is "bookmark-bmenu" for "buffer-menu":

(defvar bookmark-bmenu-hidden-bookmarks ())

(defvar-keymap bookmark-bmenu-mode-map
  :doc "Keymap for `bookmark-bmenu-mode'."
  :parent tabulated-list-mode-map
  "v" #'bookmark-bmenu-select
  "w" #'bookmark-bmenu-locate
  "5" #'bookmark-bmenu-other-frame
  "2" #'bookmark-bmenu-2-window
  "1" #'bookmark-bmenu-1-window
  "j" #'bookmark-bmenu-this-window
  "C-c C-c" #'bookmark-bmenu-this-window
  "f" #'bookmark-bmenu-this-window
  "C-m" #'bookmark-bmenu-this-window
  "o" #'bookmark-bmenu-other-window
  "C-o" #'bookmark-bmenu-switch-other-window
  "s" #'bookmark-bmenu-save
  "C-x C-s" #'bookmark-bmenu-save
  "k" #'bookmark-bmenu-delete
  "C-d" #'bookmark-bmenu-delete-backwards
  "x" #'bookmark-bmenu-execute-deletions
  "d" #'bookmark-bmenu-delete
  "D" #'bookmark-bmenu-delete-all
  "S-SPC" #'previous-line
  "SPC" #'next-line
  "DEL" #'bookmark-bmenu-backup-unmark
  "u" #'bookmark-bmenu-unmark
  "U" #'bookmark-bmenu-unmark-all
  "m" #'bookmark-bmenu-mark
  "M" #'bookmark-bmenu-mark-all
  "l" #'bookmark-bmenu-load
  "r" #'bookmark-bmenu-rename
  "R" #'bookmark-bmenu-relocate
  "t" #'bookmark-bmenu-toggle-filenames
  "a" #'bookmark-bmenu-show-annotation
  "A" #'bookmark-bmenu-show-all-annotations
  "e" #'bookmark-bmenu-edit-annotation
  "/" #'bookmark-bmenu-search
  "<mouse-2>" #'bookmark-bmenu-other-window-with-mouse)

(easy-menu-define bookmark-menu bookmark-bmenu-mode-map
  "Menu for `bookmark-bmenu'."
  '("Bookmark"
    ["Select Bookmark in This Window" bookmark-bmenu-this-window  t]
    ["Select Bookmark in Full-Frame Window" bookmark-bmenu-1-window  t]
    ["Select Bookmark in Other Window" bookmark-bmenu-other-window  t]
    ["Select Bookmark in Other Frame" bookmark-bmenu-other-frame  t]
    ["Select Marked Bookmarks" bookmark-bmenu-select t]
    "---"
    ["Mark Bookmark" bookmark-bmenu-mark t]
    ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
    ["Unmark Bookmark" bookmark-bmenu-unmark  t]
    ["Unmark Backwards" bookmark-bmenu-backup-unmark  t]
    ["Unmark all Bookmarks" bookmark-bmenu-unmark-all  t]
    ["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames  t]
    ["Display Location of Bookmark" bookmark-bmenu-locate  t]
    "---"
    ("Edit Bookmarks"
     ["Rename Bookmark" bookmark-bmenu-rename  t]
     ["Relocate Bookmark's File" bookmark-bmenu-relocate  t]
     ["Mark Bookmark for Deletion" bookmark-bmenu-delete  t]
     ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all  t]
     ["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions  t])
    ("Annotations"
     ["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation  t]
     ["Show Annotations for All Bookmarks" bookmark-bmenu-show-all-annotations  t]
     ["Edit Annotation for Current Bookmark."  bookmark-bmenu-edit-annotation  t])
    "---"
    ["Save Bookmarks" bookmark-bmenu-save  t]
    ["Load Bookmarks" bookmark-bmenu-load  t]))

;; Bookmark Buffer Menu mode is suitable only for specially formatted
;; data.
(put 'bookmark-bmenu-mode 'mode-class 'special)


;; todo: need to display whether or not bookmark exists as a buffer in
;; flag column.

;; Format:
;; FLAGS  BOOKMARK [ LOCATION ]


(defun bookmark-bmenu-surreptitiously-rebuild-list ()
  "Rebuild the Bookmark List if it exists.
Don't affect the buffer ring order."
  (if (get-buffer bookmark-bmenu-buffer)
      (save-excursion
        (save-window-excursion
          (bookmark-bmenu-list)))))

(defun bookmark-bmenu--revert ()
  "Re-populate `tabulated-list-entries'."
  (let (entries)
    (dolist (full-record (bookmark-maybe-sort-alist))
      (let* ((name       (bookmark-name-from-full-record full-record))
             (type       (bookmark-type-from-full-record full-record))
             (annotation (bookmark-get-annotation full-record))
             (location   (bookmark-location full-record)))
        (push (list
               full-record
               `[,(if (and annotation (not (string-equal annotation "")))
                      "*" "")
                 ,(if (display-mouse-p)
                      (propertize name
                                  'font-lock-face 'bookmark-menu-bookmark
                                  'mouse-face 'highlight
                                  'follow-link t
                                  'help-echo "mouse-2: go to this bookmark in other window")
                    name)
                 ,(or type "")
                 ,@(if bookmark-bmenu-toggle-filenames
                       (list location))])
              entries)))
    ;; The value of `bookmark-sort-flag' might have changed since the
    ;; last time the buffer contents were generated, so re-check it.
    (cond ((eq bookmark-sort-flag t)
           (setq tabulated-list-sort-key '("Bookmark Name" . nil)
                 tabulated-list-entries entries))
          ((or (null bookmark-sort-flag)
               (eq bookmark-sort-flag 'last-modified))
           (setq tabulated-list-sort-key nil)
           ;; And since we're not sorting by bookmark name, show bookmarks
           ;; according to order of creation, with the most recently
           ;; created bookmarks at the top and the least recently created
           ;; at the bottom.
           ;;
           ;; Note that clicking the column sort toggle for the bookmark
           ;; name column will invoke the `tabulated-list-mode' sort, which
           ;; uses `bookmark-bmenu--name-predicate' to sort lexically by
           ;; bookmark name instead of by (reverse) creation order.
           ;; Clicking the toggle again will reverse the lexical sort, but
           ;; the sort will still be lexical not creation-order.  However,
           ;; if the user reverts the buffer, then the above check of
           ;; `bookmark-sort-flag' will happen again and the buffer will
           ;; go back to a creation-order sort.  This is all expected
           ;; behavior, as documented in `bookmark-bmenu-mode'.
           (setq tabulated-list-entries (reverse entries))))
    ;; Generate the header only after `tabulated-list-sort-key' is
    ;; settled, because if that's non-nil then the sort-direction
    ;; indicator will be shown in the named column, but if it's
    ;; nil then the indicator will not be shown.
    (tabulated-list-init-header))
  (tabulated-list-print t))

;;;###autoload
(defun bookmark-bmenu-get-buffer ()
  "Return the Bookmark List, building it if it doesn't exists.
Don't affect the buffer ring order."
  (or (get-buffer bookmark-bmenu-buffer)
      (save-excursion
	(save-window-excursion
	  (bookmark-bmenu-list)
	  (get-buffer bookmark-bmenu-buffer)))))

(custom-add-choice 'tab-bar-new-tab-choice
                   '(const :tag "Bookmark List" bookmark-bmenu-get-buffer))

;;;###autoload
(defun bookmark-bmenu-list ()
  "Display a list of existing bookmarks.
The list is displayed in a buffer named `*Bookmark List*'.
The leftmost column displays a D if the bookmark is flagged for
deletion, or > if it is flagged for displaying."
  (interactive)
  (bookmark-maybe-load-default-file)
  (let ((buf (get-buffer-create bookmark-bmenu-buffer)))
    (if (called-interactively-p 'interactive)
        (switch-to-buffer buf)
      (set-buffer buf)))
  (bookmark-bmenu-mode)
  (bookmark-bmenu--revert))

;;;###autoload
(defalias 'list-bookmarks 'bookmark-bmenu-list)
;;;###autoload
(defalias 'edit-bookmarks 'bookmark-bmenu-list)

(define-obsolete-function-alias 'bookmark-bmenu-set-header
  #'tabulated-list-init-header "28.1")

(define-derived-mode bookmark-bmenu-mode tabulated-list-mode "Bookmark Menu"
  "Major mode for editing a list of bookmarks.
Each line describes one of the bookmarks in Emacs.
Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.

If `bookmark-sort-flag' is non-nil, then sort the list by
bookmark name (case-insensitively, in collation order); the
direction of that sort can be reversed by using the column sort
toggle for the bookmark name column.

If `bookmark-sort-flag' is nil, then sort the list by bookmark
creation order, with most recently created bookmarks on top.
However, the column sort toggle will still activate (and
thereafter toggle the direction of) lexical sorting by bookmark name.
At any time you may use \\[revert-buffer] to go back to sorting by creation order.

\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
  Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
\\[bookmark-bmenu-locate] -- display (in minibuffer) location of this bookmark.
\\[bookmark-bmenu-1-window] -- select this bookmark in full-frame window.
\\[bookmark-bmenu-2-window] -- select this bookmark in one window,
  together with bookmark selected before this one in another window.
\\[bookmark-bmenu-this-window] -- select this bookmark in place of the bookmark menu buffer.
\\[bookmark-bmenu-other-window] -- select this bookmark in another window,
  so the bookmark menu bookmark remains visible in its window.
\\[bookmark-bmenu-other-frame] -- select this bookmark in another frame.
\\[bookmark-bmenu-switch-other-window] -- switch the other window to this bookmark.
\\[bookmark-bmenu-rename] -- rename this bookmark (prompts for new name).
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
  With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
  With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
  in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
\\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current bookmark.
\\[bookmark-bmenu-search] -- incrementally search for bookmarks.
\\[revert-buffer] -- refresh the buffer, and thus refresh the sort order (useful
  if `bookmark-sort-flag' is nil)."
  (setq truncate-lines t)
  (setq buffer-read-only t)
  ;; FIXME: The header could also display the current default bookmark file
  ;; according to `bookmark-bookmarks-timestamp'.
  (setq tabulated-list-format
        `[("" 1) ;; Space to add "*" for bookmark with annotation
          ("Bookmark Name"
           ,bookmark-bmenu-file-column bookmark-bmenu--name-predicate)
          ("Type" 8 bookmark-bmenu--type-predicate)
          ,@(if bookmark-bmenu-toggle-filenames
                '(("File" 0 bookmark-bmenu--file-predicate)))])
  (setq tabulated-list-padding bookmark-bmenu-marks-width)
  (when (and bookmark-sort-flag
             (not (eq bookmark-sort-flag 'last-modified)))
    (setq tabulated-list-sort-key '("Bookmark Name" . nil)))
  (add-hook 'tabulated-list-revert-hook #'bookmark-bmenu--revert nil t)'
  (setq revert-buffer-function 'bookmark-bmenu--revert)
  (tabulated-list-init-header))


(defun bookmark-bmenu--name-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the name column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (caar a) (caar b) nil t))

(defun bookmark-bmenu--type-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the type column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (elt (cadr a) 2) (elt (cadr b) 2) nil t))

(defun bookmark-bmenu--file-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the file column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (bookmark-location (car a))
                        (bookmark-location (car b))
                        nil t))


(defun bookmark-bmenu-toggle-filenames (&optional show)
  "Toggle whether filenames are shown in the bookmark list.
Optional argument SHOW means show them unconditionally."
  (interactive nil bookmark-bmenu-mode)
  (cond
   (show
    (setq bookmark-bmenu-toggle-filenames t))
   (bookmark-bmenu-toggle-filenames
    (setq bookmark-bmenu-toggle-filenames nil))
   (t
    (setq bookmark-bmenu-toggle-filenames t)))
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-show-filenames (&optional _)
  "In an interactive bookmark list, show filenames along with bookmarks."
  (setq bookmark-bmenu-toggle-filenames t)
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-hide-filenames (&optional _)
  "In an interactive bookmark list, hide the filenames of the bookmarks."
  (setq bookmark-bmenu-toggle-filenames nil)
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-ensure-position ()
  "If point is not on a bookmark line, move it to one.
If after the last full line, move to the last full line.  The
return value is undefined."
  (cond ((and (bolp) (eobp))
         (beginning-of-line 0))))


(defun bookmark-bmenu-bookmark ()
  "Return the bookmark for this line in an interactive bookmark list buffer."
  (bookmark-bmenu-ensure-position)
  (let* ((id (tabulated-list-get-id))
         (entry (and id (assoc id tabulated-list-entries))))
    (if entry
        (caar entry)
      "")))


(defun bookmark-show-annotation (bookmark-name-or-record)
  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
If the annotation does not exist, do nothing."
  (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
    (when (and annotation (not (string-equal annotation "")))
      (save-excursion
        (let ((old-buf (current-buffer)))
          (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
          (let (buffer-read-only)
            (erase-buffer)
            (insert annotation)
            (goto-char (point-min))
            (set-buffer-modified-p nil))
          (setq buffer-read-only t)
          (switch-to-buffer-other-window old-buf))))))


(defun bookmark-show-all-annotations ()
  "Display the annotations for all bookmarks in a buffer."
  (save-selected-window
    (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
    (let (buffer-read-only)
      (erase-buffer)
      (dolist (full-record (bookmark-maybe-sort-alist))
        (let* ((name (bookmark-name-from-full-record full-record))
               (ann  (bookmark-get-annotation full-record)))
          (insert (concat name ":\n"))
          (if (and ann (not (string-equal ann "")))
              ;; insert the annotation, indented by 4 spaces.
              (progn
                (save-excursion (insert ann) (unless (bolp)
                                               (insert "\n")))
                (while (< (point) (point-max))
                  (beginning-of-line)     ; paranoia
                  (insert "    ")
                  (forward-line)
                  (end-of-line))))))
      (goto-char (point-min))
      (set-buffer-modified-p nil))
    (setq buffer-read-only t)))


(defun bookmark-bmenu-mark ()
  "Mark bookmark on this line to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag ">" t))


(defun bookmark-bmenu-mark-all ()
  "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag ">" t))))


(defun bookmark-bmenu-select ()
  "Select this line's bookmark; also display bookmarks marked with `>'.
You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (menu (current-buffer))
        (others ())
        tem)
    (goto-char (point-min))
    (while (re-search-forward "^>" nil t)
      (setq tem (bookmark-bmenu-bookmark))
      (let ((inhibit-read-only t))
        (delete-char -1)
        (insert ?\s))
      (or (string-equal tem bmrk)
          (member tem others)
          (setq others (cons tem others))))
    (setq others (nreverse others)
          tem (/ (1- (frame-height)) (1+ (length others))))
    (delete-other-windows)
    (bookmark-jump bmrk)
    (bury-buffer menu)
    (if others
        (while others
          (split-window nil tem)
          (other-window 1)
          (bookmark-jump (car others))
          (setq others (cdr others)))
      (other-window 1))))


(defun bookmark-bmenu-any-marks ()
  "Return non-nil if any bookmarks are marked in the marks column."
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (catch 'found-mark
      (while (not (eobp))
        (beginning-of-line)
        (if (looking-at "^\\S-")
            (throw 'found-mark t)
          (forward-line 1)))
      nil)))


(defun bookmark-bmenu-save ()
  "Save the current list into a bookmark file.
With a prefix arg, prompt for a file to save them in.

See also the related behaviors of `bookmark-load' and
`bookmark-bmenu-load'."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (save-window-excursion
      (call-interactively 'bookmark-save)
      (set-buffer-modified-p nil))))


(defun bookmark-bmenu-load ()
  "Load bookmarks from a file and rebuild the bookmark menu-buffer.
Prompt for a file, with the default choice being the value of
`bookmark-default-file'.

With a prefix argument, replace the current ambient bookmarks
(i.e., the ones in `bookmark-alist') with the ones from the selected
file and make that file be the new value of `bookmark-default-file'.
In other words, a prefix argument means \"switch over to the bookmark
universe defined in the loaded file\".  Without a prefix argument,
just add the loaded bookmarks into the current ambient set.

See the documentation for `bookmark-load' for more details; see also
the related behaviors of `bookmark-save' and `bookmark-bmenu-save'."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (save-excursion
    (save-window-excursion
      ;; This will call `bookmark-bmenu-list'
      (call-interactively 'bookmark-load))))


(defun bookmark-bmenu-1-window ()
  "Select this line's bookmark, alone, in full frame."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-jump (bookmark-bmenu-bookmark))
  (bury-buffer (other-buffer))
  (delete-other-windows))


(defun bookmark-bmenu-2-window ()
  "Select this line's bookmark, with previous buffer in second window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (menu (current-buffer))
        (pop-up-windows t))
    (delete-other-windows)
    (switch-to-buffer (other-buffer) nil t)
    (bookmark--jump-via bmrk 'pop-to-buffer)
    (bury-buffer menu)))


(defun bookmark-bmenu-this-window ()
  "Select this line's bookmark in this window."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-jump (bookmark-bmenu-bookmark)))


(defun bookmark-bmenu-other-window ()
  "Select this line's bookmark in other window, leaving bookmark menu visible."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))


(defun bookmark-bmenu-other-frame ()
  "Select this line's bookmark in other frame."
  (interactive nil bookmark-bmenu-mode)
  (let  ((bookmark (bookmark-bmenu-bookmark))
         (pop-up-frames t))
    (bookmark-jump-other-window bookmark)))

(defun bookmark-bmenu-switch-other-window ()
  "Make the other window select this line's bookmark.
The current window remains selected."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark))
	(fun (lambda (b) (display-buffer b t))))
    (bookmark--jump-via bookmark fun)))

(defun bookmark-bmenu-other-window-with-mouse (event)
  "Jump to bookmark at mouse EVENT position in other window.
Move point in menu buffer to the position of EVENT and leave
bookmark menu visible."
  (interactive "e" bookmark-bmenu-mode)
  (with-current-buffer (window-buffer (posn-window (event-end event)))
    (save-excursion
      (goto-char (posn-point (event-end event)))
      (bookmark-bmenu-other-window))))


(defun bookmark-bmenu-show-annotation ()
  "Show the annotation for the current bookmark in another window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark-show-annotation bookmark)))


(defun bookmark-bmenu-show-all-annotations ()
  "Show the annotation for all bookmarks in another window."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-show-all-annotations))


(defun bookmark-bmenu-edit-annotation ()
  "Edit the annotation for the current bookmark in another window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark-edit-annotation bookmark t)))


(defun bookmark-bmenu-unmark (&optional backup)
  "Cancel all requested operations on bookmark on this line and move down.
Optional BACKUP means move up."
  (interactive "P" bookmark-bmenu-mode)
  ;; any flags to reset according to circumstances?  How about a
  ;; flag indicating whether this bookmark is being visited?
  ;; well, we don't have this now, so maybe later.
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag " ")
  (forward-line (if backup -1 1)))


(defun bookmark-bmenu-backup-unmark ()
  "Move up and cancel all requested operations on bookmark on line above."
  (interactive nil bookmark-bmenu-mode)
  (forward-line -1)
  (bookmark-bmenu-ensure-position)
  (bookmark-bmenu-unmark)
  (forward-line -1)
  (bookmark-bmenu-ensure-position))


(defun bookmark-bmenu-unmark-all ()
  "Cancel all requested operations on all listed bookmarks."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag " " t))))


(defun bookmark-bmenu-delete ()
  "Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag "D" t))


(defun bookmark-bmenu-delete-backwards ()
  "Mark bookmark on this line to be deleted, then move up one line.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-delete)
  (forward-line -2))


(defun bookmark-bmenu-delete-all ()
  "Mark all listed bookmarks as to be deleted.
To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag "D" t))))


(defun bookmark-bmenu-execute-deletions ()
  "Delete bookmarks flagged `D'.
If `bookmark-menu-confirm-deletion' is non-nil, prompt for
confirmation first."
  (interactive nil bookmark-bmenu-mode)
  (if (and bookmark-menu-confirm-deletion
           (not (yes-or-no-p "Delete selected bookmarks? ")))
      (message "Bookmarks not deleted.")
    (let ((reporter (make-progress-reporter "Deleting bookmarks..."))
          (o-point  (point))
          (o-str    (save-excursion
                      (beginning-of-line)
                      (unless (= (following-char) ?D)
                        (buffer-substring
                         (point)
                         (progn (end-of-line) (point))))))
          (o-col     (current-column)))
      (goto-char (point-min))
      (while (re-search-forward "^D" (point-max) t)
        (bookmark-delete (bookmark-bmenu-bookmark) t)) ; pass BATCH arg
      (bookmark-bmenu-list)
      (if o-str
          (progn
            (goto-char (point-min))
            (search-forward o-str)
            (beginning-of-line)
            (forward-char o-col))
        (goto-char o-point))
      (beginning-of-line)
      (progress-reporter-done reporter))))


(defun bookmark-bmenu-rename ()
  "Rename bookmark on current line.  Prompts for a new name."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (thispoint (point)))
    (bookmark-rename bmrk)
    (goto-char thispoint)))


(defun bookmark-bmenu-locate ()
  "Display location of this bookmark.  Displays in the minibuffer."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark)))
    (message "%s" (bookmark-location bmrk))))

(defun bookmark-bmenu-relocate ()
  "Change the absolute file name of the bookmark on the current line.
Prompt with completion for the new path."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (thispoint (point)))
    (bookmark-relocate bmrk)
    (goto-char thispoint)))

;;; Bookmark-bmenu search

(defun bookmark-bmenu-filter-alist-by-regexp (regexp)
  "Filter `bookmark-alist' with bookmarks matching REGEXP and rebuild list."
  (let ((bookmark-alist
         (cl-loop for i in bookmark-alist
                  when (string-match regexp (car i)) collect i into new
                  finally return new)))
    (bookmark-bmenu-list)))


;;;###autoload
(defun bookmark-bmenu-search ()
  "Incremental search of bookmarks, hiding the non-matches as we go."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmk (bookmark-bmenu-bookmark))
        (timer nil))
    (unwind-protect
        (minibuffer-with-setup-hook
            (lambda ()
              (setq timer (run-with-idle-timer
                           bookmark-search-delay 'repeat
                           (lambda (buf)
                             (with-current-buffer buf
                               (bookmark-bmenu-filter-alist-by-regexp
                                (minibuffer-contents))))
                           (current-buffer))))
          (read-string "Pattern: ")
          (when timer (cancel-timer timer) (setq timer nil)))
      (when timer ;; Signaled an error or a `quit'.
        (cancel-timer timer)
        (bookmark-bmenu-list)
        (bookmark-bmenu-goto-bookmark bmk)))))

(defun bookmark-bmenu-goto-bookmark (name)
  "Move point to bookmark with name NAME."
  (goto-char (point-min))
  (while (not (or (equal name (bookmark-bmenu-bookmark))
                  (eobp)))
    (forward-line 1))
  (forward-line 0))


\f
;;; Menu bar stuff.  Prefix is "bookmark-menu".

(defun bookmark-menu-popup-paned-menu (event name entries)
  "Pop up multi-paned menu at EVENT, return string chosen from ENTRIES.
That is, ENTRIES is a list of strings which appear as the choices
in the menu.
The number of panes depends on the number of entries.
The visible entries are truncated to `bookmark-menu-length', but the
strings returned are not."
  (let ((f-height (/ (frame-height) 2))
	(pane-list nil)
	(iter 0))
    (while entries
      (let (lst
	    (count 0))
	(while (and (< count f-height) entries)
	  (let ((str (car entries)))
	    (push (cons
		   (if (> (length str) bookmark-menu-length)
		       (substring str 0 bookmark-menu-length)
		     str)
		   str)
		  lst)
	    (setq entries (cdr entries))
	    (setq count (1+ count))))
	(setq iter (1+ iter))
	(push (cons
	       (format "-*- %s (%d) -*-" name iter)
	       (nreverse lst))
	      pane-list)))

    ;; Popup the menu and return the string.
    (x-popup-menu event (cons (concat "-*- " name " -*-")
			      (nreverse pane-list)))))


;; Thanks to Roland McGrath for fixing menubar.el so that the
;; following works, and for explaining what to do to make it work.

;; We MUST autoload EACH form used to set up this variable's value, so
;; that the whole job is done in loaddefs.el.

;; Emacs menubar stuff.

;;;###autoload
(defvar menu-bar-bookmark-map
  (let ((map (make-sparse-keymap "Bookmark functions")))
    (bindings--define-key map [load]
      '(menu-item "Load a Bookmark File..." bookmark-load
		  :help "Load bookmarks from a bookmark file)"))
    (bindings--define-key map [write]
      '(menu-item "Save Bookmarks As..." bookmark-write
		  :help "Write bookmarks to a file (reading the file name with the minibuffer)"))
    (bindings--define-key map [save]
      '(menu-item "Save Bookmarks" bookmark-save
		  :help "Save currently defined bookmarks"))
    (bindings--define-key map [edit]
      '(menu-item "Edit Bookmark List" bookmark-bmenu-list
		  :help "Display a list of existing bookmarks"))
    (bindings--define-key map [delete]
      '(menu-item "Delete Bookmark..." bookmark-delete
		  :help "Delete a bookmark from the bookmark list"))
    (bindings--define-key map [delete-all]
      '(menu-item "Delete all Bookmarks..." bookmark-delete-all
		  :help "Delete all bookmarks from the bookmark list"))
    (bindings--define-key map [rename]
      '(menu-item "Rename Bookmark..." bookmark-rename
		  :help "Change the name of a bookmark"))
    (bindings--define-key map [locate]
      '(menu-item "Insert Location..." bookmark-locate
		  :help "Insert the name of the file associated with a bookmark"))
    (bindings--define-key map [insert]
      '(menu-item "Insert Contents..." bookmark-insert
		  :help "Insert the text of the file pointed to by a bookmark"))
    (bindings--define-key map [set]
      '(menu-item "Set Bookmark..." bookmark-set
		  :help "Set a bookmark named inside a file."))
    (bindings--define-key map [jump]
      '(menu-item "Jump to Bookmark..." bookmark-jump
		  :help "Jump to a bookmark (a point in some file)"))
    map))

;;;###autoload
(defalias 'menu-bar-bookmark-map menu-bar-bookmark-map)

;; make bookmarks appear toward the right side of the menu.
(if (boundp 'menu-bar-final-items)
    (if menu-bar-final-items
        (push 'bookmark menu-bar-final-items))
  (setq menu-bar-final-items '(bookmark)))

;;;; end bookmark menu stuff ;;;;

\f
;; Load Hook
(defvar bookmark-load-hook nil
  "Hook run at the end of loading library `bookmark.el'.")
(make-obsolete-variable 'bookmark-load-hook
                        "use `with-eval-after-load' instead." "28.1")

;; Exit Hook, called from kill-emacs-hook
(defvar bookmark-exit-hook nil
  "Hook run when Emacs exits.")

(defun bookmark-exit-hook-internal ()
  "Save bookmark state, if necessary, at Emacs exit time.
This also runs `bookmark-exit-hook'."
  (run-hooks 'bookmark-exit-hook)
  (and (bookmark-time-to-save-p t)
       (bookmark-save)))

(unless noninteractive
  (add-hook 'kill-emacs-hook 'bookmark-exit-hook-internal))

(defun bookmark-unload-function ()
  "Unload the Bookmark library."
  (when bookmark-save-flag (bookmark-save))
  ;; continue standard unloading
  nil)


(run-hooks 'bookmark-load-hook)

\f
;;; Obsolete:

(define-obsolete-function-alias 'bookmark-send-edited-annotation
  #'bookmark-edit-annotation-confirm "29.1")

(provide 'bookmark)

;;; bookmark.el ends here

[-- Attachment #4: bookmark-2022-08-11.el --]
[-- Type: application/octet-stream, Size: 105851 bytes --]

;;; bookmark.el --- set bookmarks, maybe annotate them, jump to them later -*- lexical-binding: t -*-

;; Copyright (C) 1993-1997, 2001-2022 Free Software Foundation, Inc.

;; Author: Karl Fogel <kfogel@red-bean.com>
;; Created: July, 1993
;; Keywords: convenience

;; This file is part of GNU Emacs.

;; GNU Emacs is free software: you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; GNU Emacs is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:

;; This package is for setting "bookmarks" in files.  A bookmark
;; associates a string with a location in a certain file.  Thus, you
;; can navigate your way to that location by providing the string.
;;
;; Type `M-x customize-group RET bookmark RET' for user options.

\f
;;; Code:

(require 'pp)
(require 'tabulated-list)
(require 'text-property-search)
(eval-when-compile (require 'cl-lib))

;;; Misc comments:
;;
;; The bookmark list is sorted lexically by default, but you can turn
;; this off by setting bookmark-sort-flag to nil.  If it is nil, then
;; the list will be presented in the order it is recorded
;; (chronologically), which is actually fairly useful as well.

;;; User Variables

(defgroup bookmark nil
  "Setting, annotation and jumping to bookmarks."
  :group 'matching)


(defcustom bookmark-use-annotations nil
  "If non-nil, setting a bookmark queries for an annotation in a buffer."
  :type 'boolean)


(defcustom bookmark-save-flag t
  "Controls when Emacs saves bookmarks to a file.
--> nil means never save bookmarks, except when `bookmark-save' is
    explicitly called (\\[bookmark-save]).
--> t means save bookmarks when Emacs is killed.
--> Otherwise, it should be a number that is the frequency with which
    the bookmark list is saved (i.e.: the number of times which
    Emacs's bookmark list may be modified before it is automatically
    saved.).  If it is a number, Emacs will also automatically save
    bookmarks when it is killed.

Therefore, the way to get it to save every time you make or delete a
bookmark is to set this variable to 1 (or 0, which produces the same
behavior.)

To specify the file in which to save them, modify the variable
`bookmark-default-file'."
  :type '(choice (const nil) integer (other t)))


(define-obsolete-variable-alias 'bookmark-old-default-file
  'bookmark-default-file "27.1")
(define-obsolete-variable-alias 'bookmark-file 'bookmark-default-file "27.1")
(defcustom bookmark-default-file
  (locate-user-emacs-file "bookmarks" ".emacs.bmk")
  "File in which to save bookmarks by default."
  ;; The current default file is defined via the internal variable
  ;; `bookmark-bookmarks-timestamp'.  This does not affect the value
  ;; of `bookmark-default-file'.
  :type 'file)

(defcustom bookmark-watch-bookmark-file t
  "If non-nil watch the default bookmark file.
If this file has changed on disk since it was last loaded, query the user
whether to load it again.  If the value is `silent' reload without querying.
This file defaults to `bookmark-default-file'.  But during an Emacs session,
`bookmark-load' and `bookmark-save' can redefine the current default file."
  :version "27.1"
  :type 'boolean
  :group 'bookmark)

(defcustom bookmark-version-control 'nospecial
  "Whether or not to make numbered backups of the bookmark file.
It can have four values: t, nil, `never', or `nospecial'.
The first three have the same meaning that they do for the
variable `version-control'; the value `nospecial' (the default) means
just use the value of `version-control'."
  :type '(choice (const :tag "If existing" nil)
                 (const :tag "Never" never)
                 (const :tag "Use value of option `version-control'" nospecial)
                 (other :tag "Always" t)))


(defcustom bookmark-completion-ignore-case t
  "Non-nil means bookmark functions ignore case in completion."
  :type 'boolean)


(defcustom bookmark-sort-flag t
  "This controls the bookmark display sorting.
nil means they will be displayed in LIFO order (that is, most
recently created ones come first, oldest ones come last).

`last-modified' means that bookmarks will be displayed sorted
from most recently modified to least recently modified.

Other values means that bookmarks will be displayed sorted by
bookmark name."
  :type '(choice (const :tag "By name" t)
                 (const :tag "By modified time" last-modified)
                 (const :tag "By creation time" nil)))


(defcustom bookmark-menu-confirm-deletion nil
  "Non-nil means confirm before deleting bookmarks in a bookmark menu buffer.
Nil means don't prompt for confirmation."
  :version "28.1"
  :type 'boolean)

(defcustom bookmark-automatically-show-annotations t
  "Non-nil means show annotations when jumping to a bookmark."
  :type 'boolean)

(defconst bookmark-bmenu-buffer "*Bookmark List*"
  "Name of buffer used for Bookmark List.")

(defvar bookmark-bmenu-use-header-line t
  "Non-nil means to use an immovable header line.
This is as opposed to inline text at the top of the buffer.")
(make-obsolete-variable 'bookmark-bmenu-use-header-line "no longer used." "28.1")

(defconst bookmark-bmenu-inline-header-height 2
  "Number of lines used for the *Bookmark List* header.
\(This is only significant when `bookmark-bmenu-use-header-line'
is nil.)")
(make-obsolete-variable 'bookmark-bmenu-inline-header-height "no longer used." "28.1")

(defconst bookmark-bmenu-marks-width 2
  "Number of columns (chars) used for the *Bookmark List* marks column.
This includes the annotations column.")

(defcustom bookmark-bmenu-file-column 30
  "Column at which to display filenames in a buffer listing bookmarks.
You can toggle whether files are shown with \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-toggle-filenames]."
  :type 'natnum)


(defcustom bookmark-bmenu-toggle-filenames t
  "Non-nil means show filenames when listing bookmarks.
A non-nil value may result in truncated bookmark names."
  :type 'boolean)

(defface bookmark-menu-bookmark
  '((t (:weight bold)))
  "Face used to highlight bookmark names in bookmark menu buffers.")

(defcustom bookmark-menu-length 70
  "Maximum length of a bookmark name displayed on a popup menu."
  :type 'natnum)

;; FIXME: Is it really worth a customization option?
(defcustom bookmark-search-delay 0.2
  "Time before `bookmark-bmenu-search' updates the display."
  :type  'number)

(defcustom bookmark-set-fringe-mark t
  "Whether to set a fringe mark at bookmarked lines."
  :type  'boolean
  :version "28.1")

;; FIXME: No longer used.  Should be declared obsolete or removed.
(defface bookmark-menu-heading
  '((t (:inherit font-lock-type-face)))
  "Face used to highlight the heading in bookmark menu buffers."
  :version "22.1")

(defface bookmark-face
  '((((class grayscale)
      (background light))
     :foreground "DimGray")
    (((class grayscale)
      (background dark))
     :foreground "LightGray")
    (((class color)
      (background light))
     :background "White" :foreground "DarkOrange1")
    (((class color)
      (background dark))
     :background "Black" :foreground "DarkOrange1"))
  "Face used to highlight current line."
  :version "28.1")

;;; No user-serviceable parts beyond this point.

\f
;;; Keymap stuff:

;; Set up these bindings dumping time *only*;
;; if the user alters them, don't override the user when loading bookmark.el.

;;;###autoload (keymap-set ctl-x-r-map "b" #'bookmark-jump)
;;;###autoload (keymap-set ctl-x-r-map "m" #'bookmark-set)
;;;###autoload (keymap-set ctl-x-r-map "M" #'bookmark-set-no-overwrite)
;;;###autoload (keymap-set ctl-x-r-map "l" #'bookmark-bmenu-list)

;;;###autoload
(defvar-keymap bookmark-map
  :doc "Keymap containing bindings to bookmark functions.
It is not bound to any key by default: to bind it
so that you have a bookmark prefix, just use `global-set-key' and bind a
key of your choice to variable `bookmark-map'.  All interactive bookmark
functions have a binding in this keymap."
  "x" #'bookmark-set
  "m" #'bookmark-set                            ;"m"ark
  "M" #'bookmark-set-no-overwrite               ;"M"aybe mark
  "j" #'bookmark-jump
  "g" #'bookmark-jump                           ;"g"o
  "o" #'bookmark-jump-other-window
  "5" #'bookmark-jump-other-frame
  "i" #'bookmark-insert
  "e" #'edit-bookmarks
  "f" #'bookmark-insert-location                ;"f"ind
  "r" #'bookmark-rename
  "d" #'bookmark-delete
  "D" #'bookmark-delete-all
  "l" #'bookmark-load
  "w" #'bookmark-write
  "s" #'bookmark-save)

;;;###autoload (fset 'bookmark-map bookmark-map)

\f
;;; Core variables and data structures:
(defvar bookmark-alist ()
  "Association list of bookmark names and their parameters.
Bookmark functions update the value automatically.
You probably do NOT want to change the value yourself.

The value is an alist whose elements are of the form

 (BOOKMARK-NAME . PARAM-ALIST)

or the deprecated form (BOOKMARK-NAME PARAM-ALIST).  The alist is
ordered from most recently created bookmark at the front to least
recently created bookmark at the end.

BOOKMARK-NAME is the name you gave to the bookmark when creating it.

PARAM-ALIST is an alist of bookmark information.  The order of the
entries in PARAM-ALIST is not important.  The default entries are
described below.  An entry with a key but null value means the entry
is not used.

 (filename . FILENAME)
 (buf . BUFFER-OR-NAME)
 (position . POS)
 (front-context-string . STR-AFTER-POS)
 (rear-context-string  . STR-BEFORE-POS)
 (handler . HANDLER)
 (annotation . ANNOTATION)

FILENAME names the bookmarked file.
BUFFER-OR-NAME is a buffer or the name of a buffer that is used
  if FILENAME is not defined or it refers to a non-existent file.
POS is the bookmarked buffer position.
STR-AFTER-POS is buffer text that immediately follows POS.
STR-BEFORE-POS is buffer text that immediately precedes POS.
ANNOTATION is a string that describes the bookmark.
  See options `bookmark-use-annotations' and
  `bookmark-automatically-show-annotations'.
HANDLER is a function that provides the `bookmark-jump' behavior for a
specific kind of bookmark instead of the default `bookmark-default-handler'.
This is the case for Info bookmarks, for instance.  HANDLER must accept
a bookmark as its single argument.

A function `bookmark-make-record-function' may define additional entries
in PARAM-LIST that can be used by HANDLER.")

(define-obsolete-variable-alias 'bookmarks-already-loaded
  'bookmark-bookmarks-timestamp "27.1")
(defvar bookmark-bookmarks-timestamp nil
  "Timestamp of current default bookmark file.
The value is actually (FILE . MODTIME), where FILE is a bookmark file that
defaults to `bookmark-default-file' and MODTIME is its modification time.")

(defvar bookmark-file-coding-system nil
  "The coding-system of the last loaded or saved bookmark file.")

(defvar-local bookmark-current-bookmark nil
  "Name of bookmark most recently used in the current file.
It is buffer local, used to make moving a bookmark forward
through a file easier.")


(defvar bookmark-alist-modification-count 0
  "Number of modifications to bookmark list since it was last saved.")


(defvar bookmark-search-size 16
  "Length of the context strings recorded on either side of a bookmark.")


(defvar bookmark-current-buffer nil
  "The buffer in which a bookmark is currently being set or renamed.
Functions that insert strings into the minibuffer use this to know
the source buffer for that information; see `bookmark-yank-word'
for example.")


(defvar bookmark-yank-point 0
  "The next point from which to pull source text for `bookmark-yank-word'.
This point is in `bookmark-current-buffer'.")


(defvar bookmark-quit-flag nil
  "Non-nil means `bookmark-bmenu-search' quits immediately.")
(make-obsolete-variable 'bookmark-quit-flag "no longer used" "27.1")

\f
;; Helper functions and macros.

(defmacro with-buffer-modified-unmodified (&rest body)
  "Run BODY while preserving the buffer's `buffer-modified-p' state."
  (let ((was-modified (make-symbol "was-modified")))
    `(let ((,was-modified (buffer-modified-p)))
       (unwind-protect
           (progn ,@body)
         (set-buffer-modified-p ,was-modified)))))

;; Only functions below, in this page and the next one (file formats),
;; need to know anything about the format of bookmark-alist entries.
;; Everyone else should go through them.

(defun bookmark-name-from-full-record (bookmark-record)
  "Return the name of BOOKMARK-RECORD.
BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'."
  (car bookmark-record))

(defun bookmark-type-from-full-record (bookmark-record)
  "Return then type of BOOKMARK-RECORD.
BOOKMARK-RECORD is, e.g., one element from `bookmark-alist'. It's
type is read from the symbol property named
`bookmark-handler-type' read on the record handler function."
  (let ((handler (bookmark-get-handler bookmark-record)))
    (when (autoloadp (symbol-function handler))
      (autoload-do-load (symbol-function handler)))
    (if (symbolp handler)
        (get handler 'bookmark-handler-type)
     "")))

(defun bookmark-all-names ()
  "Return a list of all current bookmark names."
  (bookmark-maybe-load-default-file)
  (mapcar 'bookmark-name-from-full-record bookmark-alist))


(defun bookmark-get-bookmark (bookmark-name-or-record &optional noerror)
  "Return the bookmark record corresponding to BOOKMARK-NAME-OR-RECORD.
If BOOKMARK-NAME-OR-RECORD is a string, look for the corresponding
bookmark record in `bookmark-alist'; return it if found, otherwise
error.  If optional argument NOERROR is non-nil, return nil
instead of signaling an error.  Else if BOOKMARK-NAME-OR-RECORD
is already a bookmark record, just return it."
  (cond
   ((consp bookmark-name-or-record) bookmark-name-or-record)
   ((stringp bookmark-name-or-record)
    (or (assoc-string bookmark-name-or-record bookmark-alist
                      bookmark-completion-ignore-case)
        (unless noerror (error "Invalid bookmark %s"
                               bookmark-name-or-record))))))


(defun bookmark-get-bookmark-record (bookmark-name-or-record)
  "Return the record portion of BOOKMARK-NAME-OR-RECORD in `bookmark-alist'.
In other words, return all information but the name."
  (let ((alist (cdr (bookmark-get-bookmark bookmark-name-or-record))))
    ;; The bookmark objects can either look like (NAME ALIST) or
    ;; (NAME . ALIST), so we have to distinguish the two here.
    (if (and (null (cdr alist)) (consp (caar alist)))
        (car alist) alist)))


(defun bookmark-set-name (bookmark-name-or-record newname)
  "Set BOOKMARK-NAME-OR-RECORD's name to NEWNAME."
  (setcar (bookmark-get-bookmark bookmark-name-or-record) newname))

(defun bookmark-prop-get (bookmark-name-or-record prop)
  "Return the property PROP of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (cdr (assq prop (bookmark-get-bookmark-record bookmark-name-or-record))))

(defun bookmark-prop-set (bookmark-name-or-record prop val)
  "Set the property PROP of BOOKMARK-NAME-OR-RECORD to VAL."
  (let ((cell (assq
               prop (bookmark-get-bookmark-record bookmark-name-or-record))))
    (if cell
        (setcdr cell val)
      (nconc (bookmark-get-bookmark-record bookmark-name-or-record)
             (list (cons prop val))))))

(defun bookmark-get-annotation (bookmark-name-or-record)
  "Return the annotation of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'annotation))

(defun bookmark-set-annotation (bookmark-name-or-record ann)
  "Set the annotation of BOOKMARK-NAME-OR-RECORD to ANN."
  (bookmark-prop-set bookmark-name-or-record 'annotation ann))


(defun bookmark-get-filename (bookmark-name-or-record)
  "Return the full filename of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'filename))


(defun bookmark-set-filename (bookmark-name-or-record filename)
  "Set the full filename of BOOKMARK-NAME-OR-RECORD to FILENAME."
  (bookmark-prop-set bookmark-name-or-record 'filename filename))


(defun bookmark-get-position (bookmark-name-or-record)
  "Return the position (i.e.: point) of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'position))


(defun bookmark-set-position (bookmark-name-or-record position)
  "Set the position (i.e.: point) of BOOKMARK-NAME-OR-RECORD to POSITION."
  (bookmark-prop-set bookmark-name-or-record 'position position))


(defun bookmark-get-front-context-string (bookmark-name-or-record)
  "Return the front-context-string of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'front-context-string))


(defun bookmark-set-front-context-string (bookmark-name-or-record string)
  "Set the front-context-string of BOOKMARK-NAME-OR-RECORD to STRING."
  (bookmark-prop-set bookmark-name-or-record 'front-context-string string))


(defun bookmark-get-rear-context-string (bookmark-name-or-record)
  "Return the rear-context-string of BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'rear-context-string))


(defun bookmark-set-rear-context-string (bookmark-name-or-record string)
  "Set the rear-context-string of BOOKMARK-NAME-OR-RECORD to STRING."
  (bookmark-prop-set bookmark-name-or-record 'rear-context-string string))


(defun bookmark-get-handler (bookmark-name-or-record)
  "Return the handler function for BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'handler))


(defun bookmark-get-last-modified (bookmark-name-or-record)
  "Return the last-modified for BOOKMARK-NAME-OR-RECORD, or nil if none."
  (bookmark-prop-get bookmark-name-or-record 'last-modified))


(defun bookmark-update-last-modified (bookmark-name-or-record)
  "Update the last-modified date of BOOKMARK-NAME-OR-RECORD to the current time."
  (bookmark-prop-set bookmark-name-or-record 'last-modified (current-time)))


(defvar bookmark-history nil
  "The history list for bookmark functions.")

(define-fringe-bitmap 'bookmark-fringe-mark
  "\x3c\x7e\xff\xff\xff\xff\x7e\x3c")

(defun bookmark--set-fringe-mark ()
  "Apply a colorized overlay to the bookmarked location.
See user option `bookmark-set-fringe-mark'."
  (let ((bm (make-overlay (point-at-bol) (1+ (point-at-bol)))))
    (overlay-put bm 'category 'bookmark)
    (overlay-put bm 'evaporate t)
    (overlay-put bm 'before-string
                 (propertize
                  "x" 'display
                  `(left-fringe bookmark-fringe-mark bookmark-face)))))

(defun bookmark--remove-fringe-mark (bm)
  "Remove a bookmark's colorized overlay.
BM is a bookmark as returned from function `bookmark-get-bookmark'.
See user option `bookmark-set-fringe'."
  (let ((filename (cdr (assq 'filename bm)))
        (pos (cdr (assq 'position bm)))
        overlays found temp)
    (when (and pos filename)
      (setq filename (expand-file-name filename))
      (dolist (buf (buffer-list))
        (with-current-buffer buf
          (when (equal filename buffer-file-name)
            (setq overlays
                  (save-excursion
                    (goto-char pos)
                    (overlays-in (point-at-bol) (1+ (point-at-bol)))))
            (while (and (not found) (setq temp (pop overlays)))
              (when (eq 'bookmark (overlay-get temp 'category))
                (delete-overlay (setq found temp))))))))))

(defun bookmark-maybe-sort-alist ()
  "Return `bookmark-alist' for display.
If `bookmark-sort-flag' is T, then return a sorted by name copy of the alist.
If `bookmark-sort-flag' is LAST-MODIFIED, then return a sorted by last modified
copy of the alist.  Otherwise, just return `bookmark-alist', which by default
is ordered from most recently created to least recently created bookmark."
  (let ((copy (copy-alist bookmark-alist)))
    (cond ((eq bookmark-sort-flag t)
           (sort copy (lambda (x y) (string-lessp (car x) (car y)))))
          ((eq bookmark-sort-flag 'last-modified)
           (sort copy (lambda (x y)
                        (let ((tx (bookmark-get-last-modified x))
                              (ty (bookmark-get-last-modified y)))
                          (cond ((null tx) nil)
                                ((null ty) t)
                                (t (time-less-p ty tx)))))))
          (t copy))))

(defun bookmark-completing-read (prompt &optional default)
  "Prompting with PROMPT, read a bookmark name in completion.
PROMPT will get a \": \" stuck on the end no matter what, so you
probably don't want to include one yourself.
Optional arg DEFAULT is a string to return if the user input is empty.
If DEFAULT is nil then return empty string for empty input."
  (bookmark-maybe-load-default-file) ; paranoia
  (if (listp last-nonmenu-event)
      (bookmark-menu-popup-paned-menu t prompt
                                      (mapcar 'bookmark-name-from-full-record
                                              (bookmark-maybe-sort-alist)))
    (let* ((completion-ignore-case bookmark-completion-ignore-case)
           (default (unless (equal "" default) default)))
      (completing-read (format-prompt prompt default)
                       (lambda (string pred action)
                         (if (eq action 'metadata)
                             '(metadata (category . bookmark))
                             (complete-with-action
                              action bookmark-alist string pred)))
                       nil 0 nil 'bookmark-history default))))


(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
locations that should be treated specially, such as Info nodes,
news posts, images, pdf documents, etc.

The function will be called with no arguments.
It should signal a user error if it is unable to construct a record for
the current location.

The returned record should be a cons cell of the form (NAME . ALIST)
where ALIST is as described in `bookmark-alist' and may typically contain
a special cons (handler . HANDLER-FUNC) which specifies the handler function
that should be used instead of `bookmark-default-handler' to open this
bookmark.  See the documentation for `bookmark-alist' for more.

NAME is a suggested name for the constructed bookmark.  It can be nil
in which case a default heuristic will be used.  The function can also
equivalently just return ALIST without NAME.")

(defun bookmark-make-record ()
  "Return a new bookmark record (NAME . ALIST) for the current location."
  (let ((record (funcall bookmark-make-record-function)))
    ;; Set up default name if the function does not provide one.
    (unless (stringp (car record))
      (if (car record) (push nil record))
      (setcar record (or bookmark-current-bookmark (bookmark-buffer-name))))
    ;; Set up defaults.
    (bookmark-prop-set
     record 'defaults
     (delq nil (delete-dups (append (bookmark-prop-get record 'defaults)
				    (list bookmark-current-bookmark
					  (car record)
                                          (bookmark-buffer-name))))))
    record))

(defun bookmark-store (name alist no-overwrite)
  "Store the bookmark NAME with data ALIST.
If NO-OVERWRITE is non-nil and another bookmark of the same name already
exists in `bookmark-alist', record the new bookmark without throwing away the
old one."
  (bookmark-maybe-load-default-file)
  (let ((stripped-name (copy-sequence name)))
    (set-text-properties 0 (length stripped-name) nil stripped-name)
    (if (and (not no-overwrite)
             (bookmark-get-bookmark stripped-name 'noerror))
        ;; Already existing bookmark under that name and
        ;; no prefix arg means just overwrite old bookmark.
        (let ((bm (bookmark-get-bookmark stripped-name)))
          ;; First clean up if previously location was fontified.
          (when bookmark-set-fringe-mark
            (bookmark--remove-fringe-mark bm))
          ;; Modify using the new (NAME . ALIST) format.
          (setcdr bm alist))

      ;; Otherwise just put it onto the front of the list.  Either the
      ;; bookmark doesn't exist already, or there is no prefix arg.
      ;; In either case, we want the new bookmark on the front of the
      ;; list, since the list is kept in reverse order of creation.
      (push (cons stripped-name alist) bookmark-alist))

    ;; Added by db
    (setq bookmark-current-bookmark stripped-name)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))

    (setq bookmark-current-bookmark stripped-name)
    (bookmark-bmenu-surreptitiously-rebuild-list)))

(defun bookmark-make-record-default (&optional no-file no-context posn)
  "Return the record describing the location of a new bookmark.
Point should be at the buffer in which the bookmark is being set,
and normally should be at the position where the bookmark is desired,
but see the optional arguments for other possibilities.

If NO-FILE is non-nil, then only return the subset of the
record that pertains to the location within the buffer, leaving off
the part that records the filename.

If NO-CONTEXT is non-nil, do not include the front- and rear-context
strings in the record -- the position is enough.

If POSN is non-nil, record POSN as the point instead of `(point)'."
  `(,@(unless no-file `((filename . ,(bookmark-buffer-file-name))))
    ,@(unless no-context `((front-context-string
                           . ,(if (>= (- (point-max) (point))
                                      bookmark-search-size)
                                  (buffer-substring-no-properties
                                   (point)
                                   (+ (point) bookmark-search-size))
                                  nil))))
    ,@(unless no-context `((rear-context-string
                           . ,(if (>= (- (point) (point-min))
                                      bookmark-search-size)
                                  (buffer-substring-no-properties
                                   (point)
                                   (- (point) bookmark-search-size))
                                  nil))))
    (position . ,(or posn (point)))
    (last-modified . ,(current-time))))

\f
;;; File format stuff

;; *IMPORTANT NOTICE* If you are thinking about modifying (redefining)
;; the bookmark file format -- please don't.  The current format
;; should be extensible enough.  If you feel the need to change it,
;; please discuss it with other Emacs developers first.
;;
;; The format of `bookmark-alist' has changed twice in its lifetime.
;; This comment describes the three formats, FIRST, SECOND, and
;; CURRENT.
;;
;; The FIRST format was used prior to Emacs 20:
;;
;;       ((BOOKMARK-NAME (FILENAME
;;                          STRING-IN-FRONT
;;                          STRING-BEHIND
;;                          POINT))
;;        ...)
;;
;; The SECOND format was introduced in Emacs 20:
;;
;;       ((BOOKMARK-NAME ((filename   . FILENAME)
;;                        (position   . POS)
;;                        (front-context-string . STR-AFTER-POS)
;;                        (rear-context-string  . STR-BEFORE-POS)
;;                        (annotation . ANNOTATION)
;;                        (whatever   . VALUE)
;;                        ...
;;                       ))
;;        ...)
;;
;; The CURRENT format was introduced in Emacs 22:
;;
;;       ((BOOKMARK-NAME (filename   . FILENAME)
;;                       (position   . POS)
;;                       (front-context-string . STR-AFTER-POS)
;;                       (rear-context-string  . STR-BEFORE-POS)
;;                       (annotation . ANNOTATION)
;;                       (whatever   . VALUE)
;;                       ...
;;                       )
;;        ...)
;;
;; Both FIRST and SECOND have the same level of nesting: the cadr of a
;; bookmark record is a list of entry information.  FIRST and SECOND
;; differ in the form of the record information: FIRST uses a list of
;; atoms, and SECOND uses an alist.  In the FIRST format, the order of
;; the list elements matters.  In the SECOND format, the order of the
;; alist elements is unimportant.  The SECOND format facilitates the
;; addition of new kinds of elements, to support new kinds of
;; bookmarks or code evolution.
;;
;; The CURRENT format removes a level of nesting wrt FIRST and SECOND,
;; saving one cons cell per bookmark: the cadr of a bookmark record is
;; no longer a cons.  Why that change was made remains a mystery --
;; just be aware of it.  (Be aware too that this explanatory comment
;; was incorrect in Emacs 22 and Emacs 23.1.)
;;
;; To deal with the change from FIRST format to SECOND, conversion
;; code was added, which is no longer used and has been declared
;; obsolete.  See `bookmark-maybe-upgrade-file-format'.
;;
;; No conversion from SECOND to CURRENT is done.  Instead, the code
;; handles both formats OK.  It must continue to do so.
;;
;; See the doc string of `bookmark-alist' for information about the
;; elements that define a bookmark (e.g. `filename').


(defconst bookmark-file-format-version 1
  "The current version of the format used by bookmark files.
You should never need to change this.")


(defconst bookmark-end-of-version-stamp-marker
  "-*- End Of Bookmark File Format Version Stamp -*-\n"
  "This string marks the end of the version stamp in a bookmark file.")


(defun bookmark-alist-from-buffer ()
  "Return a `bookmark-alist' from the current buffer.
The buffer must of course contain bookmark format information.
Does not care from where in the buffer it is called, and does not
affect point."
  (save-excursion
    (goto-char (point-min))
    (if (search-forward bookmark-end-of-version-stamp-marker nil t)
        (read (current-buffer))
      (if buffer-file-name
          (error "File not in bookmark format: %s" buffer-file-name)
        (error "Buffer not in bookmark format: %s" (buffer-name))))))

(defun bookmark-upgrade-version-0-alist (old-list)
  "Upgrade a version 0 alist OLD-LIST to the current version."
  (declare (obsolete nil "27.1"))
  (mapcar
   (lambda (bookmark)
     (let* ((name      (car bookmark))
            (record    (car (cdr bookmark)))
            (filename  (nth 0 record))
            (front-str (nth 1 record))
            (rear-str  (nth 2 record))
            (position  (nth 3 record))
            (ann       (nth 4 record)))
       (list
        name
        `((filename             .    ,filename)
          (front-context-string .    ,(or front-str ""))
          (rear-context-string  .    ,(or rear-str  ""))
          (position             .    ,position)
          (annotation           .    ,ann)))))
   old-list))


(defun bookmark-upgrade-file-format-from-0 ()
  "Upgrade a bookmark file of format 0 (the original format) to format 1.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (let* ((reporter (make-progress-reporter
                    (format "Upgrading bookmark format from 0 to %d..."
                            bookmark-file-format-version)))
         (old-list (bookmark-alist-from-buffer))
         (new-list (with-suppressed-warnings
                       ((obsolete bookmark-upgrade-version-0-alist))
                     (bookmark-upgrade-version-0-alist old-list))))
    (delete-region (point-min) (point-max))
    (bookmark-insert-file-format-version-stamp buffer-file-coding-system)
    (pp new-list (current-buffer))
    (save-buffer)
    (goto-char (point-min))
    (progress-reporter-done reporter)))


(defun bookmark-grok-file-format-version ()
  "Return an integer which is the file-format version of this bookmark file.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (if (looking-at "^;;;;")
      (save-excursion
        (save-match-data
          (re-search-forward "[0-9]")
          (forward-char -1)
          (read (current-buffer))))
    ;; Else this is format version 0, the original one, which didn't
    ;; even have version stamps.
    0))


(defun bookmark-maybe-upgrade-file-format ()
  "Check the file-format version of this bookmark file.
If the version is not up-to-date, upgrade it automatically.
This expects to be called from `point-min' in a bookmark file."
  (declare (obsolete nil "27.1"))
  (let ((version
         (with-suppressed-warnings
             ((obsolete bookmark-grok-file-format-version))
           (bookmark-grok-file-format-version))))
    (cond
     ((= version bookmark-file-format-version)
      ) ; home free -- version is current
     ((= version 0)
      (with-suppressed-warnings
          ((obsolete bookmark-upgrade-file-format-from-0))
        (bookmark-upgrade-file-format-from-0)))
     (t
      (error "Bookmark file format version strangeness")))))


(defun bookmark-insert-file-format-version-stamp (coding)
  "Insert text indicating current version of bookmark file format.
CODING is the symbol of the coding-system in which the file is encoded."
  (if (memq (coding-system-base coding) '(undecided prefer-utf-8))
      (setq coding 'utf-8-emacs))
  (insert
   (format
    ";;;; Emacs Bookmark Format Version %d\
;;;; -*- coding: %S; mode: lisp-data -*-\n"
    bookmark-file-format-version (coding-system-base coding)))
  (insert ";;; This format is meant to be slightly human-readable;\n"
          ";;; nevertheless, you probably don't want to edit it.\n"
          ";;; "
          bookmark-end-of-version-stamp-marker))


;;; end file-format stuff

\f
;;; Core code:

(define-obsolete-function-alias 'bookmark-maybe-message 'message "27.1")

(defvar-keymap bookmark-minibuffer-read-name-map
  :parent minibuffer-local-map
  "C-w" #'bookmark-yank-word)

(defun bookmark-set-internal (prompt name overwrite-or-push)
  "Set a bookmark using specified NAME or prompting with PROMPT.
The bookmark is set at the current location.

If NAME is non-nil, use it as the name of the new bookmark.  In
this case, the value of PROMPT is ignored.

Otherwise, prompt the user for the bookmark name.  Begin the
interactive prompt with PROMPT, followed by a space, a generated
default name in parentheses, a colon and a space.

OVERWRITE-OR-PUSH controls what happens if there is already a
bookmark with the same name: nil means signal an error;
`overwrite' means replace any existing bookmark; `push' means
push the new bookmark onto the bookmark alist.  The `push'
behavior means that among bookmarks with the same name, this most
recently set one becomes the one in effect, but the others are
still there, in order, if the topmost one is ever deleted."
  (unwind-protect
       (let* ((record (bookmark-make-record))
              ;; `defaults' is a transient element of the
              ;; extensible format described above in the section
              ;; `File format stuff'.  Bookmark record functions
              ;; can use it to specify a list of default values
              ;; accessible via M-n while reading a bookmark name.
              (defaults (bookmark-prop-get record 'defaults))
              (default (if (consp defaults) (car defaults) defaults)))

         (if defaults
             ;; Don't store default values in the record.
             (setq record (assq-delete-all 'defaults record))
           ;; When no defaults in the record, use its first element.
           (setq defaults (car record) default defaults))

         (bookmark-maybe-load-default-file)
         ;; Don't set `bookmark-yank-point' and `bookmark-current-buffer'
         ;; if they have been already set in another buffer. (e.g gnus-art).
         (unless (and bookmark-yank-point
                      bookmark-current-buffer)
           (setq bookmark-yank-point (point))
           (setq bookmark-current-buffer (current-buffer)))

         (let ((str
                (or name
                    (read-from-minibuffer
                     (format-prompt prompt default)
                     nil
                     bookmark-minibuffer-read-name-map
                     nil nil defaults))))
           (and (string-equal str "") (setq str default))

           (cond
            ((eq overwrite-or-push nil)
             (if (bookmark-get-bookmark str t)
                 (error "A bookmark named \"%s\" already exists" str)
               (bookmark-store str (cdr record) nil)))
            ((eq overwrite-or-push 'overwrite)
             (bookmark-store str (cdr record) nil))
            ((eq overwrite-or-push 'push)
             (bookmark-store str (cdr record) t))
            (t
             (error "Unrecognized value for `overwrite-or-push': %S"
                    overwrite-or-push)))

           ;; Ask for an annotation buffer for this bookmark
           (when bookmark-use-annotations
             (bookmark-edit-annotation str))
           (when bookmark-set-fringe-mark
             (bookmark--set-fringe-mark))))
    (setq bookmark-yank-point nil)
    (setq bookmark-current-buffer nil)))


;;;###autoload
(defun bookmark-set (&optional name no-overwrite)
  "Set a bookmark named NAME at the current location.
If NAME is nil, then prompt the user.

With a prefix arg (non-nil NO-OVERWRITE), do not overwrite any
existing bookmark that has the same name as NAME, but instead push the
new bookmark onto the bookmark alist.  The most recently set bookmark
with name NAME is thus the one in effect at any given time, but the
others are still there, should the user decide to delete the most
recent one.

To yank words from the text of the buffer and use them as part of the
bookmark name, type \\<bookmark-minibuffer-read-name-map>\
\\[bookmark-yank-word] while setting a bookmark.  Successive \
\\[bookmark-yank-word]'s
yank successive words.

Typing \\[universal-argument] inserts (at the bookmark name prompt) the name of the last
bookmark used in the document where the new bookmark is being set;
this helps you use a single bookmark name to track progress through a
large document.  If there is no prior bookmark for this document, then
\\[universal-argument] inserts an appropriate name based on the buffer or file.

Use \\[bookmark-delete] to remove bookmarks (you give it a name and
it removes only the first instance of a bookmark with that name from
the list of bookmarks.)"
  (interactive (list nil current-prefix-arg))
  (let ((prompt
         (if no-overwrite "Append bookmark named" "Set bookmark named")))
    (bookmark-set-internal prompt name (if no-overwrite 'push 'overwrite))))

;;;###autoload
(defun bookmark-set-no-overwrite (&optional name push-bookmark)
  "Set a bookmark named NAME at the current location.
If NAME is nil, then prompt the user.

If a bookmark named NAME already exists and prefix argument
PUSH-BOOKMARK is non-nil, then push the new bookmark onto the
bookmark alist.  Pushing it means that among bookmarks named
NAME, this one becomes the one in effect, but the others are
still there, in order, and become effective again if the user
ever deletes the most recent one.

Otherwise, if a bookmark named NAME already exists but PUSH-BOOKMARK
is nil, raise an error.

To yank words from the text of the buffer and use them as part of the
bookmark name, type \\<bookmark-minibuffer-read-name-map>\
\\[bookmark-yank-word] while setting a bookmark.  Successive \
\\[bookmark-yank-word]'s
yank successive words.

Typing \\[universal-argument] inserts (at the bookmark name prompt) the name of the last
bookmark used in the document where the new bookmark is being set;
this helps you use a single bookmark name to track progress through a
large document.  If there is no prior bookmark for this document, then
\\[universal-argument] inserts an appropriate name based on the buffer or file.

Use \\[bookmark-delete] to remove bookmarks (you give it a name and
it removes only the first instance of a bookmark with that name from
the list of bookmarks.)"
  (interactive (list nil current-prefix-arg))
  (bookmark-set-internal "Set bookmark" name (if push-bookmark 'push nil)))


(defun bookmark-kill-line (&optional newline-too)
  "Kill from point to end of line.
If optional arg NEWLINE-TOO is non-nil, delete the newline too.
Does not affect the kill ring."
  (let ((eol (line-end-position)))
    (delete-region (point) eol)
    (when (and newline-too (= (following-char) ?\n))
      (delete-char 1))))

(defvar-local bookmark-annotation-name nil
  "Name of bookmark under edit in `bookmark-edit-annotation-mode'.")

(defvar-local bookmark--annotation-from-bookmark-list nil
  "If non-nil, `bookmark-edit-annotation-mode' should return to bookmark list.")

(defun bookmark-default-annotation-text (bookmark-name)
  "Return default annotation text for BOOKMARK-NAME.
The default annotation text is simply some text explaining how to use
annotations."
  (concat (format-message
           "#  Type the annotation for bookmark `%s' here.\n"
           bookmark-name)
	  (format-message
           "#  All lines which start with a `#' will be deleted.\n")
          (substitute-command-keys
           (concat
            "#  Type \\[bookmark-edit-annotation-confirm] when done.  Type "
            "\\[bookmark-edit-annotation-cancel] to cancel.\n#\n"))
	  "#  Author: " (user-full-name) " <" (user-login-name) "@"
	  (system-name) ">\n"
          "#  Date:   " (current-time-string) "\n"))


(defvar bookmark-edit-annotation-text-func 'bookmark-default-annotation-text
  "Function to return default text to use for a bookmark annotation.
It takes one argument, the name of the bookmark, as a string.")

(defvar-keymap bookmark-edit-annotation-mode-map
  :doc "Keymap for editing an annotation of a bookmark."
  :parent text-mode-map
  "C-c C-c" #'bookmark-edit-annotation-confirm
  "C-c C-k" #'bookmark-edit-annotation-cancel)

(defun bookmark-insert-annotation (bookmark-name-or-record)
  "Insert annotation for BOOKMARK-NAME-OR-RECORD at point."
  (when (not (bookmark-get-bookmark bookmark-name-or-record t))
    (error "Invalid bookmark: %s" bookmark-name-or-record))
  (insert (funcall bookmark-edit-annotation-text-func bookmark-name-or-record))
  (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
    (if (and annotation (not (string-equal annotation "")))
	(insert annotation))))

(define-derived-mode bookmark-edit-annotation-mode
  text-mode "Edit Bookmark Annotation"
  "Mode for editing the annotation of bookmarks.
\\<bookmark-edit-annotation-mode-map>\
When you have finished composing, type \\[bookmark-edit-annotation-confirm] \
or \\[bookmark-edit-annotation-cancel] to cancel.

\\{bookmark-edit-annotation-mode-map}")

(defmacro bookmark-edit-annotation--maybe-display-list (&rest body)
  "Display bookmark list after editing if appropriate."
  `(let ((from-bookmark-list bookmark--annotation-from-bookmark-list)
         (old-buffer (current-buffer)))
     ,@body
     (quit-window)
     (bookmark-bmenu-surreptitiously-rebuild-list)
     (when from-bookmark-list
       (pop-to-buffer (get-buffer bookmark-bmenu-buffer))
       (goto-char (point-min))
       (bookmark-bmenu-bookmark))
     (kill-buffer old-buffer)))

(defun bookmark-edit-annotation-cancel ()
  "Cancel the current annotation edit."
  (interactive nil bookmark-edit-annotation-mode)
  (bookmark-edit-annotation--maybe-display-list
   (message "Canceled by user")))

(defun bookmark-edit-annotation-confirm ()
  "Use buffer contents as annotation for a bookmark.
Lines beginning with `#' are ignored."
  (interactive nil bookmark-edit-annotation-mode)
  (if (not (derived-mode-p 'bookmark-edit-annotation-mode))
      (error "Not in bookmark-edit-annotation-mode"))
  (goto-char (point-min))
  (while (< (point) (point-max))
    (if (= (following-char) ?#)
        (bookmark-kill-line t)
      (forward-line 1)))
  ;; Take no chances with text properties.
  (bookmark-edit-annotation--maybe-display-list
   (let ((annotation (buffer-substring-no-properties (point-min) (point-max)))
         (bookmark-name bookmark-annotation-name))
     (bookmark-set-annotation bookmark-name annotation)
     (bookmark-update-last-modified bookmark-name)
     (setq bookmark-alist-modification-count
           (1+ bookmark-alist-modification-count))
     (message "Annotation updated for \"%s\"" bookmark-name))))


(defun bookmark-edit-annotation (bookmark-name-or-record &optional from-bookmark-list)
  "Pop up a buffer for editing bookmark BOOKMARK-NAME-OR-RECORD's annotation.
If optional argument FROM-BOOKMARK-LIST is non-nil, return to the
bookmark list when editing is done."
  (pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
  (bookmark-edit-annotation-mode)
  (bookmark-insert-annotation bookmark-name-or-record)
  (setq bookmark--annotation-from-bookmark-list from-bookmark-list)
  (setq bookmark-annotation-name bookmark-name-or-record))


(defun bookmark-buffer-name ()
  "Return the name of the current buffer in a form usable as a bookmark name.
If the buffer is associated with a file or directory, use that name."
  (cond
   ;; Or are we a file?
   (buffer-file-name (file-name-nondirectory buffer-file-name))
   ;; Or are we a directory?
   ((and (boundp 'dired-directory) dired-directory)
    (let* ((dirname (if (stringp dired-directory)
                        dired-directory
                      (car dired-directory)))
           (idx (1- (length dirname))))
      ;; Strip the trailing slash.
      (if (= ?/ (aref dirname idx))
          (file-name-nondirectory (substring dirname 0 idx))
        ;; Else return the current-buffer
        (buffer-name (current-buffer)))))
   ;; If all else fails, use the buffer's name.
   (t
    (buffer-name (current-buffer)))))


(defun bookmark-yank-word ()
  "Get the next word from buffer `bookmark-current-buffer' and append
it to the name of the bookmark currently being set, advancing
`bookmark-yank-point' by one word."
  (interactive)
  (let ((string (with-current-buffer bookmark-current-buffer
                  (goto-char bookmark-yank-point)
                  (buffer-substring-no-properties
                   (point)
                   (progn
                     (forward-word 1)
                     (setq bookmark-yank-point (point)))))))
    (insert string)))

(defun bookmark-buffer-file-name ()
  "Return the current buffer's file in a way useful for bookmarks."
  ;; Abbreviate the path, both so it's shorter and so it's more
  ;; portable.  E.g., the user's home dir might be a different
  ;; path on different machines, but "~/" will still reach it.
  (abbreviate-file-name
   (cond
    (buffer-file-name buffer-file-name)
    ((and (boundp 'dired-directory) dired-directory)
     (if (stringp dired-directory)
         dired-directory
       (car dired-directory)))
    (t (error "Buffer not visiting a file or directory")))))

(defvar bookmark--watch-already-asked-mtime nil
  "Mtime for which we already queried about reloading.")

(defun bookmark--watch-file-already-queried-p (new-mtime)
  ;; Don't ask repeatedly if user already said "no" to reloading a
  ;; file with this mtime:
  (prog1 (time-equal-p new-mtime bookmark--watch-already-asked-mtime)
    (setq bookmark--watch-already-asked-mtime new-mtime)))

(defun bookmark-maybe-load-default-file ()
  "If bookmarks have not been loaded from the default place, load them."
  (cond ((and (not bookmark-bookmarks-timestamp)
              (null bookmark-alist)
              (file-readable-p bookmark-default-file)
              (bookmark-load bookmark-default-file t t)))
        ((and bookmark-watch-bookmark-file
              (let ((new-mtime (nth 5 (file-attributes
                                       (car bookmark-bookmarks-timestamp))))
                    (old-mtime (cdr bookmark-bookmarks-timestamp)))
		(and (not (time-equal-p new-mtime old-mtime))
                     (not (bookmark--watch-file-already-queried-p new-mtime))
                     (or (eq 'silent bookmark-watch-bookmark-file)
                         (yes-or-no-p
                          (format "Bookmarks %s changed on disk.  Reload? "
                                  (car bookmark-bookmarks-timestamp)))))))
         (bookmark-load (car bookmark-bookmarks-timestamp) t t))))



(defvar bookmark-after-jump-hook nil
  "Hook run after `bookmark-jump' jumps to a bookmark.
Useful for example to unhide text in `outline-mode'.")

(defun bookmark--jump-via (bookmark-name-or-record display-function)
  "Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
DISPLAY-FUNCTION is called with the current buffer as argument.

After calling DISPLAY-FUNCTION, set window point to the point specified
by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
and then show any annotations for this bookmark."
  (bookmark-handle-bookmark bookmark-name-or-record)
  (save-current-buffer
    (funcall display-function (current-buffer)))
  (let ((win (get-buffer-window (current-buffer) 0)))
    (if win (set-window-point win (point))))
  ;; FIXME: we used to only run bookmark-after-jump-hook in
  ;; `bookmark-jump' itself, but in none of the other commands.
  (when bookmark-set-fringe-mark
    (let ((overlays (overlays-in (point-at-bol) (1+ (point-at-bol))))
          temp found)
      (while (and (not found) (setq temp (pop overlays)))
        (when (eq 'bookmark (overlay-get temp 'category))
          (setq found t)))
      (unless found
        (bookmark--set-fringe-mark))))
  (run-hooks 'bookmark-after-jump-hook)
  (if bookmark-automatically-show-annotations
      ;; if there is an annotation for this bookmark,
      ;; show it in a buffer.
      (bookmark-show-annotation bookmark-name-or-record)))


;;;###autoload
(defun bookmark-jump (bookmark &optional display-func)
  "Jump to bookmark BOOKMARK (a point in some file).
You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
bookmarks.  See help on function `bookmark-load' for more about
this.

If the file pointed to by BOOKMARK no longer exists, you will be asked
if you wish to give the bookmark a new location, and `bookmark-jump'
will then jump to the new location, as well as recording it in place
of the old one in the permanent bookmark record.

BOOKMARK is usually a bookmark name (a string).  It can also be a
bookmark record, but this is usually only done by programmatic callers.

If DISPLAY-FUNC is non-nil, it is a function to invoke to display the
bookmark.  It defaults to `pop-to-buffer-same-window'.  A typical value for
DISPLAY-FUNC would be `switch-to-buffer-other-window'."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark"
				   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.
  (bookmark--jump-via bookmark (or display-func 'pop-to-buffer-same-window)))


;;;###autoload
(defun bookmark-jump-other-window (bookmark)
  "Jump to BOOKMARK in another window.  See `bookmark-jump' for more."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark (in another window)"
                                   bookmark-current-bookmark)))
  (bookmark-jump bookmark 'switch-to-buffer-other-window))

;;;###autoload
(defun bookmark-jump-other-frame (bookmark)
  "Jump to BOOKMARK in another frame.  See `bookmark-jump' for more."
  (interactive
   (list (bookmark-completing-read "Jump to bookmark (in another frame)"
                                   bookmark-current-bookmark)))
  (let ((pop-up-frames t))
    (bookmark-jump-other-window bookmark)))

(defun bookmark-handle-bookmark (bookmark-name-or-record)
  "Call BOOKMARK-NAME-OR-RECORD's handler or `bookmark-default-handler'
if it has none.  This changes current buffer and point and returns nil,
or signals a `file-error'.

If BOOKMARK-NAME-OR-RECORD has no file, this is a no-op.  If
BOOKMARK-NAME-OR-RECORD has a file, but that file no longer exists,
then offer interactively to relocate BOOKMARK-NAME-OR-RECORD."
  (condition-case err
      (funcall (or (bookmark-get-handler bookmark-name-or-record)
                   'bookmark-default-handler)
               (bookmark-get-bookmark bookmark-name-or-record))
    (bookmark-error-no-filename         ;file-error
     ;; We were unable to find the marked file, so ask if user wants to
     ;; relocate the bookmark, else remind them to consider deletion.
     (when (stringp bookmark-name-or-record)
       ;; `bookmark-name-or-record' can be either a bookmark name
       ;; (from `bookmark-alist')  or a bookmark object.  If it's an
       ;; object, we assume it's a bookmark used internally by some
       ;; other package.
       (let ((file (bookmark-get-filename bookmark-name-or-record)))
         (when file        ;Don't know how to relocate if there's no `file'.
           ;; If file is not a dir, directory-file-name just returns file.
           (let ((display-name (directory-file-name file)))
             (ding)
             ;; Dialog boxes can accept a file target, but usually don't
             ;; know how to accept a directory target (at least, this
             ;; is true in Gnome on GNU/Linux, and Bug#4230 says it's
             ;; true on Windows as well).  So we suppress file dialogs
             ;; when relocating.
             (let ((use-dialog-box nil)
                   (use-file-dialog nil))
               (if (y-or-n-p (concat display-name " nonexistent.  Relocate \""
                                     bookmark-name-or-record "\"? "))
                   (progn
                     (bookmark-relocate bookmark-name-or-record)
                     ;; Try again.
                     (funcall (or (bookmark-get-handler bookmark-name-or-record)
                                  'bookmark-default-handler)
                              (bookmark-get-bookmark bookmark-name-or-record)))
                 (message
                  "Bookmark not relocated; consider removing it (%s)."
                  bookmark-name-or-record)
                 (signal (car err) (cdr err))))))))))
  ;; Added by db.
  (when (stringp bookmark-name-or-record)
    (setq bookmark-current-bookmark bookmark-name-or-record))
  nil)

(define-error 'bookmark-errors
  "Bookmark error")
(define-error 'bookmark-error-no-filename
  "Bookmark has no associated file (or directory)" 'bookmark-errors)

(defun bookmark-default-handler (bmk-record)
  "Default handler to jump to a particular bookmark location.
BMK-RECORD is a bookmark record, not a bookmark name (i.e., not a string).
Changes current buffer and point and returns nil, or signals a `file-error'.

If BMK-RECORD has a property called `buffer', it should be a live
buffer object, and this buffer will be selected."
  (let ((file          (bookmark-get-filename bmk-record))
	(buf           (bookmark-prop-get bmk-record 'buffer))
        (forward-str   (bookmark-get-front-context-string bmk-record))
        (behind-str    (bookmark-get-rear-context-string bmk-record))
        (place         (bookmark-get-position bmk-record)))
    (set-buffer
     (cond
      ((and file (file-readable-p file) (not (buffer-live-p buf)))
       (find-file-noselect file))
      ;; No file found.  See if buffer BUF has been created.
      ((and buf (get-buffer buf)))
      (t ;; If not, raise error.
       (signal 'bookmark-error-no-filename (list 'stringp file)))))
    (if place (goto-char place))
    ;; Go searching forward first.  Then, if forward-str exists and
    ;; was found in the file, we can search backward for behind-str.
    ;; Rationale is that if text was inserted between the two in the
    ;; file, it's better to be put before it so you can read it,
    ;; rather than after and remain perhaps unaware of the changes.
    (when (and forward-str (search-forward forward-str (point-max) t))
      (goto-char (match-beginning 0)))
    (when (and behind-str (search-backward behind-str (point-min) t))
      (goto-char (match-end 0)))
    nil))

;;;###autoload
(defun bookmark-relocate (bookmark-name)
  "Relocate BOOKMARK-NAME to another file, reading file name with minibuffer.

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))
         (newloc (abbreviate-file-name
                  (expand-file-name
                   (read-file-name
                    (format "Relocate %s to: " bookmark-name)
                    (file-name-directory bmrk-filename))))))
    (bookmark-set-filename bookmark-name newloc)
    (bookmark-update-last-modified bookmark-name)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))
    (bookmark-bmenu-surreptitiously-rebuild-list)))


;;;###autoload
(defun bookmark-insert-location (bookmark-name &optional no-history)
  "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'."
  (interactive (list (bookmark-completing-read "Insert bookmark location")))
  (or no-history (bookmark-maybe-historicize-string bookmark-name))
  (insert (bookmark-location bookmark-name)))

;;;###autoload
(defalias 'bookmark-locate 'bookmark-insert-location)

(defun bookmark-location (bookmark-name-or-record)
  "Return a description of the location of BOOKMARK-NAME-OR-RECORD."
  (bookmark-maybe-load-default-file)
  ;; We could call the `handler' and ask for it to construct a description
  ;; dynamically: it would open up several new possibilities, but it
  ;; would have the major disadvantage of forcing to load each and
  ;; every handler when the user calls bookmark-menu.
  (or (bookmark-prop-get bookmark-name-or-record 'location)
      (bookmark-get-filename bookmark-name-or-record)
      "-- Unknown location --"))

;;;###autoload
(defun bookmark-rename (old-name &optional new-name)
  "Change the name of OLD-NAME bookmark to NEW-NAME name.
If called from keyboard, prompt for OLD-NAME and NEW-NAME.
If called from menubar, select OLD-NAME from a menu and prompt for NEW-NAME.

If called from Lisp, prompt for NEW-NAME if only OLD-NAME was passed
as an argument.  If called with two strings, then no prompting is done.
You must pass at least OLD-NAME when calling from Lisp.

While you are entering the new name, consecutive \
\\<bookmark-minibuffer-read-name-map>\\[bookmark-yank-word]'s insert
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))
  (setq bookmark-current-buffer (current-buffer))
  (let ((final-new-name
         (or new-name   ; use second arg, if non-nil
             (read-from-minibuffer
              "New name: "
              nil
              (define-keymap
                :parent minibuffer-local-map
                "C-w" #'bookmark-yank-word)
              nil
              'bookmark-history))))
    (bookmark-set-name old-name final-new-name)
    (bookmark-update-last-modified final-new-name)
    (setq bookmark-current-bookmark final-new-name)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (setq bookmark-alist-modification-count
          (1+ bookmark-alist-modification-count))
    (if (bookmark-time-to-save-p)
        (bookmark-save))))


;;;###autoload
(defun bookmark-insert (bookmark-name)
  "Insert the text of the file pointed to by bookmark BOOKMARK-NAME.
BOOKMARK-NAME is a bookmark name (a string), not a bookmark record.

You may have a problem using this function if the value of variable
`bookmark-alist' is nil.  If that happens, you need to load in some
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
	 (save-current-buffer
           (bookmark-handle-bookmark bookmark-name)
	   (buffer-string))))
    (insert str-to-insert)
    (push-mark)
    (goto-char orig-point)))


;;;###autoload
(defun bookmark-delete (bookmark-name &optional batch)
  "Delete BOOKMARK-NAME from the bookmark list.

Removes only the first instance of a bookmark with that name.  If
there are one or more other bookmarks with the same name, they will
not be deleted.  Defaults to the \"current\" bookmark (that is, the
one most recently used in this file, if any).
Optional second arg BATCH means don't update the bookmark list buffer,
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)
    (setq bookmark-alist (delq will-go bookmark-alist))
    ;; Added by db, nil bookmark-current-bookmark if the last
    ;; occurrence has been deleted
    (or (bookmark-get-bookmark bookmark-current-bookmark 'noerror)
        (setq bookmark-current-bookmark nil)))
  (unless batch
    (bookmark-bmenu-surreptitiously-rebuild-list))
  (setq bookmark-alist-modification-count
        (1+ bookmark-alist-modification-count))
  (when (bookmark-time-to-save-p)
    (bookmark-save)))


;;;###autoload
(defun bookmark-delete-all (&optional no-confirm)
  "Permanently delete all bookmarks.
If optional argument NO-CONFIRM is non-nil, don't ask for
confirmation."
  (interactive "P")
  ;; We don't use `bookmark-menu-confirm-deletion' here because that
  ;; variable is specifically to control confirmation prompting in a
  ;; bookmark menu buffer, where the user has the marked-for-deletion
  ;; bookmarks arrayed in front of them and might have accidentally
  ;; hit the key that executes the deletions.  The UI situation here
  ;; is quite different, by contrast: the user got to this point by a
  ;; sequence of keystrokes unlikely to be typed by chance.
  (when (or no-confirm
            (yes-or-no-p "Permanently delete all bookmarks? "))
    (bookmark-maybe-load-default-file)
    (setq bookmark-alist-modification-count
          (+ bookmark-alist-modification-count (length bookmark-alist)))
    (setq bookmark-alist nil)
    (bookmark-bmenu-surreptitiously-rebuild-list)
    (when (bookmark-time-to-save-p)
      (bookmark-save))))


(defun bookmark-time-to-save-p (&optional final-time)
  "Return t if it is time to save bookmarks to disk, nil otherwise.
Optional argument FINAL-TIME means this is being called when Emacs
is being killed, so save even if `bookmark-save-flag' is a number and
is greater than `bookmark-alist-modification-count'."
  ;; By Gregory M. Saunders <saunders{_AT_}cis.ohio-state.edu>
  (cond (final-time
	 (and (> bookmark-alist-modification-count 0)
	      bookmark-save-flag))
	((numberp bookmark-save-flag)
	 (>= bookmark-alist-modification-count bookmark-save-flag))
	(t
	 nil)))


;;;###autoload
(defun bookmark-write ()
  "Write bookmarks to a file (reading the file name with the minibuffer)."
  (declare (interactive-only bookmark-save))
  (interactive)
  (bookmark-maybe-load-default-file)
  (bookmark-save t))


;;;###autoload
(defun bookmark-save (&optional parg file make-default)
  "Save currently defined bookmarks in FILE.
FILE defaults to `bookmark-default-file'.
With prefix PARG, query user for a file to save in.
If MAKE-DEFAULT is non-nil (interactively with prefix \\[universal-argument] \\[universal-argument])
the file we save in becomes the new default in the current Emacs
session (without affecting the value of `bookmark-default-file'.).

When you want to load in the bookmarks from a file, use
`bookmark-load', \\[bookmark-load].  That function will prompt you
for a file, defaulting to the file defined by variable
`bookmark-default-file'."
  (interactive
   (list current-prefix-arg nil (equal '(16) current-prefix-arg)))
  (bookmark-maybe-load-default-file)
  (unless file
    (setq file
          (let ((default (or (car bookmark-bookmarks-timestamp)
                             bookmark-default-file)))
            (if parg
                ;; This should be part of the `interactive' spec.
                (read-file-name (format "File to save bookmarks in: (%s) "
                                        default)
                                (file-name-directory default) default)
              default))))
  (bookmark-write-file file)
  ;; Signal that we have synced the bookmark file by setting this to 0.
  ;; If there was an error at any point before, it will not get set,
  ;; which is what we want.
  (setq bookmark-alist-modification-count 0)
  (if make-default
      (let ((default (expand-file-name file)))
        (setq bookmark-bookmarks-timestamp
              (cons default (nth 5 (file-attributes default)))))
    (let ((default (car bookmark-bookmarks-timestamp)))
      (if (string= default (expand-file-name file))
          (setq bookmark-bookmarks-timestamp
                (cons default (nth 5 (file-attributes default))))))))

\f
(defun bookmark-write-file (file)
  "Write `bookmark-alist' to FILE."
  (let ((reporter (make-progress-reporter
		   (format "Saving bookmarks to file %s..." file))))
    (with-current-buffer (get-buffer-create " *Bookmarks*")
      (goto-char (point-min))
      (delete-region (point-min) (point-max))
      (let ((coding-system-for-write
	     (or coding-system-for-write
		 bookmark-file-coding-system 'utf-8-emacs))
	    (print-length nil)
	    (print-level nil)
	    ;; See bug #12503 for why we bind `print-circle'.  Users
	    ;; can define their own bookmark types, which can result in
	    ;; arbitrary Lisp objects being stored in bookmark records,
	    ;; and some users create objects containing circularities.
	    (print-circle t))
	(insert "(")
	;; Rather than a single call to `pp' we make one per bookmark.
	;; Apparently `pp' has a poor algorithmic complexity, so this
	;; scales a lot better.  bug#4485.
	(dolist (i bookmark-alist) (pp i (current-buffer)))
	(insert ")\n")
	;; Make sure the specified encoding can safely encode the
	;; bookmarks.  If it cannot, suggest utf-8-emacs as default.
	(with-coding-priority '(utf-8-emacs)
	  (setq coding-system-for-write
		(select-safe-coding-system (point-min) (point-max)
					   (list t coding-system-for-write))))
	(goto-char (point-min))
	(bookmark-insert-file-format-version-stamp coding-system-for-write)
	(let ((version-control
	       (cond
		((null bookmark-version-control) nil)
		((eq 'never bookmark-version-control) 'never)
		((eq 'nospecial bookmark-version-control) version-control)
		(t t))))
	  (condition-case nil
              ;; There was a stretch of time (about 15 years) when we
              ;; used `write-region' below instead of `write-file',
              ;; before going back to `write-file' again.  So if you're
              ;; considering changing it to `write-region', please see
              ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=12507.
              ;; That bug tells the story of how we first started using
              ;; `write-region' in 2005...
              ;;
              ;;   commit a506054af7cd86a63fda996056c09310966f32ef
              ;;   Author:     Karl Fogel <kfogel@red-bean.com>
              ;;   AuthorDate: Sat Nov 12 20:30:22 2005 +0000
              ;;
              ;;       (bookmark-write-file): Don't visit the
              ;;       destination file, just write the data to it
              ;;       using write-region.  This is similar to
              ;;       2005-05-29T08:36:26Z!rms@gnu.org of saveplace.el,
              ;;       but with an additional change to avoid visiting
              ;;       the  file in the first place.
              ;;
              ;; ...and of how further inquiry led us to investigate (in
              ;; 2012 and then again in 2020) and eventually decide that
              ;; matching the saveplace.el change doesn't make sense for
              ;; bookmark.el.  Therefore we reverted to `write-file',
              ;; which means numbered backups may now be created,
              ;; depending on `bookmark-version-control' as per above.
	      (write-file file)
	    (file-error (message "Can't write %s" file)))
	  (setq bookmark-file-coding-system coding-system-for-write)
	  (kill-buffer (current-buffer))
	  (progress-reporter-done reporter))))))


(defun bookmark-import-new-list (new-list)
  "Add NEW-LIST of bookmarks to `bookmark-alist'.
Rename new bookmarks as needed using suffix \"<N>\" (N=1,2,3...), when
they conflict with existing bookmark names."
  (let ((names (bookmark-all-names)))
    (dolist (full-record new-list)
      (bookmark-maybe-rename full-record names)
      (setq bookmark-alist (nconc bookmark-alist (list full-record)))
      (push (bookmark-name-from-full-record full-record) names))))


(defun bookmark-maybe-rename (full-record names)
  "Rename bookmark FULL-RECORD if its current name is already used.
This is a helper for `bookmark-import-new-list'."
  (let ((found-name (bookmark-name-from-full-record full-record)))
    (if (member found-name names)
        ;; We've got a conflict, so generate a new name
        (let ((count 2)
              (new-name found-name))
          (while (member new-name names)
            (setq new-name (concat found-name (format "<%d>" count)))
            (setq count (1+ count)))
          (bookmark-set-name full-record new-name)))))


;;;###autoload
(defun bookmark-load (file &optional overwrite no-msg default)
  "Load bookmarks from FILE (which must be in bookmark format).
Appends loaded bookmarks to the front of the list of bookmarks.
If argument OVERWRITE is non-nil, existing bookmarks are destroyed.
Optional third arg NO-MSG means don't display any messages while loading.
If DEFAULT is non-nil make FILE the new bookmark file to watch.
Interactively, a prefix arg makes OVERWRITE and DEFAULT non-nil.

If you load a file that doesn't contain a proper bookmark alist, you
will corrupt Emacs's bookmark list.  Generally, you should only load
in files that were created with the bookmark functions in the first
place.  Your own personal bookmark file, specified by the variable
`bookmark-default-file', is maintained automatically by Emacs; you
shouldn't need to load it explicitly.

If you load a file containing bookmarks with the same names as
bookmarks already present in your Emacs, the new bookmarks will get
unique numeric suffixes \"<2>\", \"<3>\", etc."
  (interactive
   (let ((default (abbreviate-file-name
		   (or (car bookmark-bookmarks-timestamp)
		       (expand-file-name bookmark-default-file))))
	 (prefix current-prefix-arg))
     (list (read-file-name (format "Load bookmarks from: (%s) " default)
			   (file-name-directory default) default 'confirm)
	   prefix nil prefix)))
  (let* ((file (expand-file-name file))
	 (afile (abbreviate-file-name file)))
    (unless (file-readable-p file)
      (user-error "Cannot read bookmark file %s" afile))
    (let ((reporter
	   (unless no-msg
	     (make-progress-reporter
	      (format "Loading bookmarks from %s..." file)))))
      (with-current-buffer (let (enable-local-variables)
			     (find-file-noselect file))
	(goto-char (point-min))
	(let ((blist (bookmark-alist-from-buffer)))
	  (unless (listp blist)
	    (error "Invalid bookmark list in %s" file))
	  ;; RW: Upon loading the bookmarks, we could add to each bookmark
	  ;; in `bookmark-alist' an extra key `bookmark-file', so that
	  ;; upon reloading the bookmarks with OVERWRITE non-nil,
	  ;; we overwrite only those bookmarks for which the key `bookmark-file'
	  ;; matches FILE.  `bookmark-save' can ignore this key.
	  ;; Would this be a useful option?
	  (if overwrite
	      (setq bookmark-alist blist
		    bookmark-alist-modification-count 0)
	    (bookmark-import-new-list blist)
	    (setq bookmark-alist-modification-count
		  (1+ bookmark-alist-modification-count)))
	  (if (or default
		  (string= file (or (car bookmark-bookmarks-timestamp)
				    (expand-file-name bookmark-default-file))))
	      (setq bookmark-bookmarks-timestamp
		    (cons file (nth 5 (file-attributes file)))))
	  (bookmark-bmenu-surreptitiously-rebuild-list)
	  (setq bookmark-file-coding-system buffer-file-coding-system))
	(kill-buffer (current-buffer)))
      (unless no-msg
	(progress-reporter-done reporter)))))

\f
;;; Code supporting the dired-like bookmark list.
;; Prefix is "bookmark-bmenu" for "buffer-menu":

(defvar bookmark-bmenu-hidden-bookmarks ())

(defvar-keymap bookmark-bmenu-mode-map
  :doc "Keymap for `bookmark-bmenu-mode'."
  :parent tabulated-list-mode-map
  "v" #'bookmark-bmenu-select
  "w" #'bookmark-bmenu-locate
  "5" #'bookmark-bmenu-other-frame
  "2" #'bookmark-bmenu-2-window
  "1" #'bookmark-bmenu-1-window
  "j" #'bookmark-bmenu-this-window
  "C-c C-c" #'bookmark-bmenu-this-window
  "f" #'bookmark-bmenu-this-window
  "C-m" #'bookmark-bmenu-this-window
  "o" #'bookmark-bmenu-other-window
  "C-o" #'bookmark-bmenu-switch-other-window
  "s" #'bookmark-bmenu-save
  "C-x C-s" #'bookmark-bmenu-save
  "k" #'bookmark-bmenu-delete
  "C-d" #'bookmark-bmenu-delete-backwards
  "x" #'bookmark-bmenu-execute-deletions
  "d" #'bookmark-bmenu-delete
  "D" #'bookmark-bmenu-delete-all
  "S-SPC" #'previous-line
  "SPC" #'next-line
  "DEL" #'bookmark-bmenu-backup-unmark
  "u" #'bookmark-bmenu-unmark
  "U" #'bookmark-bmenu-unmark-all
  "m" #'bookmark-bmenu-mark
  "M" #'bookmark-bmenu-mark-all
  "l" #'bookmark-bmenu-load
  "r" #'bookmark-bmenu-rename
  "R" #'bookmark-bmenu-relocate
  "t" #'bookmark-bmenu-toggle-filenames
  "a" #'bookmark-bmenu-show-annotation
  "A" #'bookmark-bmenu-show-all-annotations
  "e" #'bookmark-bmenu-edit-annotation
  "/" #'bookmark-bmenu-search
  "<mouse-2>" #'bookmark-bmenu-other-window-with-mouse)

(easy-menu-define bookmark-menu bookmark-bmenu-mode-map
  "Menu for `bookmark-bmenu'."
  '("Bookmark"
    ["Select Bookmark in This Window" bookmark-bmenu-this-window  t]
    ["Select Bookmark in Full-Frame Window" bookmark-bmenu-1-window  t]
    ["Select Bookmark in Other Window" bookmark-bmenu-other-window  t]
    ["Select Bookmark in Other Frame" bookmark-bmenu-other-frame  t]
    ["Select Marked Bookmarks" bookmark-bmenu-select t]
    "---"
    ["Mark Bookmark" bookmark-bmenu-mark t]
    ["Mark all Bookmarks" bookmark-bmenu-mark-all t]
    ["Unmark Bookmark" bookmark-bmenu-unmark  t]
    ["Unmark Backwards" bookmark-bmenu-backup-unmark  t]
    ["Unmark all Bookmarks" bookmark-bmenu-unmark-all  t]
    ["Toggle Display of Filenames" bookmark-bmenu-toggle-filenames  t]
    ["Display Location of Bookmark" bookmark-bmenu-locate  t]
    "---"
    ("Edit Bookmarks"
     ["Rename Bookmark" bookmark-bmenu-rename  t]
     ["Relocate Bookmark's File" bookmark-bmenu-relocate  t]
     ["Mark Bookmark for Deletion" bookmark-bmenu-delete  t]
     ["Mark all Bookmarks for Deletion" bookmark-bmenu-delete-all  t]
     ["Delete Marked Bookmarks" bookmark-bmenu-execute-deletions  t])
    ("Annotations"
     ["Show Annotation for Current Bookmark" bookmark-bmenu-show-annotation  t]
     ["Show Annotations for All Bookmarks" bookmark-bmenu-show-all-annotations  t]
     ["Edit Annotation for Current Bookmark."  bookmark-bmenu-edit-annotation  t])
    "---"
    ["Save Bookmarks" bookmark-bmenu-save  t]
    ["Load Bookmarks" bookmark-bmenu-load  t]))

;; Bookmark Buffer Menu mode is suitable only for specially formatted
;; data.
(put 'bookmark-bmenu-mode 'mode-class 'special)


;; todo: need to display whether or not bookmark exists as a buffer in
;; flag column.

;; Format:
;; FLAGS  BOOKMARK [ LOCATION ]


(defun bookmark-bmenu-surreptitiously-rebuild-list ()
  "Rebuild the Bookmark List if it exists.
Don't affect the buffer ring order."
  (if (get-buffer bookmark-bmenu-buffer)
      (save-excursion
        (save-window-excursion
          (bookmark-bmenu-list)))))

(defun bookmark-bmenu--revert ()
  "Re-populate `tabulated-list-entries'."
  (let (entries)
    (dolist (full-record (bookmark-maybe-sort-alist))
      (let* ((name       (bookmark-name-from-full-record full-record))
             (type       (bookmark-type-from-full-record full-record))
             (annotation (bookmark-get-annotation full-record))
             (location   (bookmark-location full-record)))
        (push (list
               full-record
               `[,(if (and annotation (not (string-equal annotation "")))
                      "*" "")
                 ,(if (display-mouse-p)
                      (propertize name
                                  'font-lock-face 'bookmark-menu-bookmark
                                  'mouse-face 'highlight
                                  'follow-link t
                                  'help-echo "mouse-2: go to this bookmark in other window")
                    name)
                 ,(or type "")
                 ,@(if bookmark-bmenu-toggle-filenames
                       (list location))])
              entries)))
    ;; The value of `bookmark-sort-flag' might have changed since the
    ;; last time the buffer contents were generated, so re-check it.
    (cond ((eq bookmark-sort-flag t)
           (setq tabulated-list-sort-key '("Bookmark Name" . nil)
                 tabulated-list-entries entries))
          ((or (null bookmark-sort-flag)
               (eq bookmark-sort-flag 'last-modified))
           (setq tabulated-list-sort-key nil)
           ;; And since we're not sorting by bookmark name, show bookmarks
           ;; according to order of creation, with the most recently
           ;; created bookmarks at the top and the least recently created
           ;; at the bottom.
           ;;
           ;; Note that clicking the column sort toggle for the bookmark
           ;; name column will invoke the `tabulated-list-mode' sort, which
           ;; uses `bookmark-bmenu--name-predicate' to sort lexically by
           ;; bookmark name instead of by (reverse) creation order.
           ;; Clicking the toggle again will reverse the lexical sort, but
           ;; the sort will still be lexical not creation-order.  However,
           ;; if the user reverts the buffer, then the above check of
           ;; `bookmark-sort-flag' will happen again and the buffer will
           ;; go back to a creation-order sort.  This is all expected
           ;; behavior, as documented in `bookmark-bmenu-mode'.
           (setq tabulated-list-entries (reverse entries))))
    ;; Generate the header only after `tabulated-list-sort-key' is
    ;; settled, because if that's non-nil then the sort-direction
    ;; indicator will be shown in the named column, but if it's
    ;; nil then the indicator will not be shown.
    (tabulated-list-init-header))
  (tabulated-list-print t))

;;;###autoload
(defun bookmark-bmenu-get-buffer ()
  "Return the Bookmark List, building it if it doesn't exists.
Don't affect the buffer ring order."
  (or (get-buffer bookmark-bmenu-buffer)
      (save-excursion
	(save-window-excursion
	  (bookmark-bmenu-list)
	  (get-buffer bookmark-bmenu-buffer)))))

(custom-add-choice 'tab-bar-new-tab-choice
                   '(const :tag "Bookmark List" bookmark-bmenu-get-buffer))

;;;###autoload
(defun bookmark-bmenu-list ()
  "Display a list of existing bookmarks.
The list is displayed in a buffer named `*Bookmark List*'.
The leftmost column displays a D if the bookmark is flagged for
deletion, or > if it is flagged for displaying."
  (interactive)
  (bookmark-maybe-load-default-file)
  (let ((buf (get-buffer-create bookmark-bmenu-buffer)))
    (if (called-interactively-p 'interactive)
        (switch-to-buffer buf)
      (set-buffer buf)))
  (bookmark-bmenu-mode)
  (bookmark-bmenu--revert))

;;;###autoload
(defalias 'list-bookmarks 'bookmark-bmenu-list)
;;;###autoload
(defalias 'edit-bookmarks 'bookmark-bmenu-list)

(define-obsolete-function-alias 'bookmark-bmenu-set-header
  #'tabulated-list-init-header "28.1")

(define-derived-mode bookmark-bmenu-mode tabulated-list-mode "Bookmark Menu"
  "Major mode for editing a list of bookmarks.
Each line describes one of the bookmarks in Emacs.
Letters do not insert themselves; instead, they are commands.
Bookmark names preceded by a \"*\" have annotations.

If `bookmark-sort-flag' is non-nil, then sort the list by
bookmark name (case-insensitively, in collation order); the
direction of that sort can be reversed by using the column sort
toggle for the bookmark name column.

If `bookmark-sort-flag' is nil, then sort the list by bookmark
creation order, with most recently created bookmarks on top.
However, the column sort toggle will still activate (and
thereafter toggle the direction of) lexical sorting by bookmark name.
At any time you may use \\[revert-buffer] to go back to sorting by creation order.

\\<bookmark-bmenu-mode-map>
\\[bookmark-bmenu-mark] -- mark bookmark to be displayed.
\\[bookmark-bmenu-mark-all] -- mark all listed bookmarks to be displayed.
\\[bookmark-bmenu-select] -- select bookmark of line point is on.
  Also show bookmarks marked using m in other windows.
\\[bookmark-bmenu-toggle-filenames] -- toggle displaying of filenames (they may obscure long bookmark names).
\\[bookmark-bmenu-locate] -- display (in minibuffer) location of this bookmark.
\\[bookmark-bmenu-1-window] -- select this bookmark in full-frame window.
\\[bookmark-bmenu-2-window] -- select this bookmark in one window,
  together with bookmark selected before this one in another window.
\\[bookmark-bmenu-this-window] -- select this bookmark in place of the bookmark menu buffer.
\\[bookmark-bmenu-other-window] -- select this bookmark in another window,
  so the bookmark menu bookmark remains visible in its window.
\\[bookmark-bmenu-other-frame] -- select this bookmark in another frame.
\\[bookmark-bmenu-switch-other-window] -- switch the other window to this bookmark.
\\[bookmark-bmenu-rename] -- rename this bookmark (prompts for new name).
\\[bookmark-bmenu-relocate] -- relocate this bookmark's file (prompts for new file).
\\[bookmark-bmenu-delete] -- mark this bookmark to be deleted, and move down.
\\[bookmark-bmenu-delete-backwards] -- mark this bookmark to be deleted, and move up.
\\[bookmark-bmenu-delete-all] -- mark all listed bookmarks as to be deleted.
\\[bookmark-bmenu-execute-deletions] -- delete bookmarks marked with `\\[bookmark-bmenu-delete]' or `\\[bookmark-bmenu-delete-all]'.
\\[bookmark-bmenu-save] -- save the current bookmark list in the default file.
  With a prefix arg, prompts for a file to save in.
\\[bookmark-bmenu-load] -- load in a file of bookmarks (prompts for file.)
\\[bookmark-bmenu-unmark] -- remove all kinds of marks from current line.
  With prefix argument, also move up one line.
\\[bookmark-bmenu-backup-unmark] -- back up a line and remove marks.
\\[bookmark-bmenu-unmark-all] -- remove all kinds of marks from all listed bookmarks.
\\[bookmark-bmenu-show-annotation] -- show the annotation, if it exists, for the current bookmark
  in another buffer.
\\[bookmark-bmenu-show-all-annotations] -- show the annotations of all bookmarks in another buffer.
\\[bookmark-bmenu-edit-annotation] -- edit the annotation for the current bookmark.
\\[bookmark-bmenu-search] -- incrementally search for bookmarks.
\\[revert-buffer] -- refresh the buffer, and thus refresh the sort order (useful
  if `bookmark-sort-flag' is nil)."
  (setq truncate-lines t)
  (setq buffer-read-only t)
  ;; FIXME: The header could also display the current default bookmark file
  ;; according to `bookmark-bookmarks-timestamp'.
  (setq tabulated-list-format
        `[("" 1) ;; Space to add "*" for bookmark with annotation
          ("Bookmark Name"
           ,bookmark-bmenu-file-column bookmark-bmenu--name-predicate)
          ("Type" 8 bookmark-bmenu--type-predicate)
          ,@(if bookmark-bmenu-toggle-filenames
                '(("File" 0 bookmark-bmenu--file-predicate)))])
  (setq tabulated-list-padding bookmark-bmenu-marks-width)
  (when (and bookmark-sort-flag
             (not (eq bookmark-sort-flag 'last-modified)))
    (setq tabulated-list-sort-key '("Bookmark Name" . nil)))
  (add-hook 'tabulated-list-revert-hook #'bookmark-bmenu--revert nil t)'
  (setq revert-buffer-function 'bookmark-bmenu--revert)
  (tabulated-list-init-header))


(defun bookmark-bmenu--name-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the name column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (caar a) (caar b) nil t))

(defun bookmark-bmenu--type-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the type column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (elt (cadr a) 2) (elt (cadr b) 2) nil t))

(defun bookmark-bmenu--file-predicate (a b)
  "Predicate to sort \"*Bookmark List*\" buffer by the file column.
This is used for `tabulated-list-format' in `bookmark-bmenu-mode'."
  (string-collate-lessp (bookmark-location (car a))
                        (bookmark-location (car b))
                        nil t))


(defun bookmark-bmenu-toggle-filenames (&optional show)
  "Toggle whether filenames are shown in the bookmark list.
Optional argument SHOW means show them unconditionally."
  (interactive nil bookmark-bmenu-mode)
  (cond
   (show
    (setq bookmark-bmenu-toggle-filenames t))
   (bookmark-bmenu-toggle-filenames
    (setq bookmark-bmenu-toggle-filenames nil))
   (t
    (setq bookmark-bmenu-toggle-filenames t)))
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-show-filenames (&optional _)
  "In an interactive bookmark list, show filenames along with bookmarks."
  (setq bookmark-bmenu-toggle-filenames t)
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-hide-filenames (&optional _)
  "In an interactive bookmark list, hide the filenames of the bookmarks."
  (setq bookmark-bmenu-toggle-filenames nil)
  (bookmark-bmenu-surreptitiously-rebuild-list))


(defun bookmark-bmenu-ensure-position ()
  "If point is not on a bookmark line, move it to one.
If after the last full line, move to the last full line.  The
return value is undefined."
  (cond ((and (bolp) (eobp))
         (beginning-of-line 0))))


(defun bookmark-bmenu-bookmark ()
  "Return the bookmark for this line in an interactive bookmark list buffer."
  (bookmark-bmenu-ensure-position)
  (let* ((id (tabulated-list-get-id))
         (entry (and id (assoc id tabulated-list-entries))))
    (if entry
        (caar entry)
      "")))


(defun bookmark-show-annotation (bookmark-name-or-record)
  "Display the annotation for BOOKMARK-NAME-OR-RECORD in a buffer.
If the annotation does not exist, do nothing."
  (let ((annotation (bookmark-get-annotation bookmark-name-or-record)))
    (when (and annotation (not (string-equal annotation "")))
      (save-excursion
        (let ((old-buf (current-buffer)))
          (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
          (let (buffer-read-only)
            (erase-buffer)
            (insert annotation)
            (goto-char (point-min))
            (set-buffer-modified-p nil))
          (setq buffer-read-only t)
          (switch-to-buffer-other-window old-buf))))))


(defun bookmark-show-all-annotations ()
  "Display the annotations for all bookmarks in a buffer."
  (save-selected-window
    (pop-to-buffer (get-buffer-create "*Bookmark Annotation*") t)
    (let (buffer-read-only)
      (erase-buffer)
      (dolist (full-record (bookmark-maybe-sort-alist))
        (let* ((name (bookmark-name-from-full-record full-record))
               (ann  (bookmark-get-annotation full-record)))
          (insert (concat name ":\n"))
          (if (and ann (not (string-equal ann "")))
              ;; insert the annotation, indented by 4 spaces.
              (progn
                (save-excursion (insert ann) (unless (bolp)
                                               (insert "\n")))
                (while (< (point) (point-max))
                  (beginning-of-line)     ; paranoia
                  (insert "    ")
                  (forward-line)
                  (end-of-line))))))
      (goto-char (point-min))
      (set-buffer-modified-p nil))
    (setq buffer-read-only t)))


(defun bookmark-bmenu-mark ()
  "Mark bookmark on this line to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag ">" t))


(defun bookmark-bmenu-mark-all ()
  "Mark all listed bookmarks to be displayed by \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-select]."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag ">" t))))


(defun bookmark-bmenu-select ()
  "Select this line's bookmark; also display bookmarks marked with `>'.
You can mark bookmarks with the \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark] or \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-mark-all] commands."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (menu (current-buffer))
        (others ())
        tem)
    (goto-char (point-min))
    (while (re-search-forward "^>" nil t)
      (setq tem (bookmark-bmenu-bookmark))
      (let ((inhibit-read-only t))
        (delete-char -1)
        (insert ?\s))
      (or (string-equal tem bmrk)
          (member tem others)
          (setq others (cons tem others))))
    (setq others (nreverse others)
          tem (/ (1- (frame-height)) (1+ (length others))))
    (delete-other-windows)
    (bookmark-jump bmrk)
    (bury-buffer menu)
    (if others
        (while others
          (split-window nil tem)
          (other-window 1)
          (bookmark-jump (car others))
          (setq others (cdr others)))
      (other-window 1))))


(defun bookmark-bmenu-any-marks ()
  "Return non-nil if any bookmarks are marked in the marks column."
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (catch 'found-mark
      (while (not (eobp))
        (beginning-of-line)
        (if (looking-at "^\\S-")
            (throw 'found-mark t)
          (forward-line 1)))
      nil)))


(defun bookmark-bmenu-save ()
  "Save the current list into a bookmark file.
With a prefix arg, prompt for a file to save them in.

See also the related behaviors of `bookmark-load' and
`bookmark-bmenu-load'."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (save-window-excursion
      (call-interactively 'bookmark-save)
      (set-buffer-modified-p nil))))


(defun bookmark-bmenu-load ()
  "Load bookmarks from a file and rebuild the bookmark menu-buffer.
Prompt for a file, with the default choice being the value of
`bookmark-default-file'.

With a prefix argument, replace the current ambient bookmarks
(i.e., the ones in `bookmark-alist') with the ones from the selected
file and make that file be the new value of `bookmark-default-file'.
In other words, a prefix argument means \"switch over to the bookmark
universe defined in the loaded file\".  Without a prefix argument,
just add the loaded bookmarks into the current ambient set.

See the documentation for `bookmark-load' for more details; see also
the related behaviors of `bookmark-save' and `bookmark-bmenu-save'."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (save-excursion
    (save-window-excursion
      ;; This will call `bookmark-bmenu-list'
      (call-interactively 'bookmark-load))))


(defun bookmark-bmenu-1-window ()
  "Select this line's bookmark, alone, in full frame."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-jump (bookmark-bmenu-bookmark))
  (bury-buffer (other-buffer))
  (delete-other-windows))


(defun bookmark-bmenu-2-window ()
  "Select this line's bookmark, with previous buffer in second window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (menu (current-buffer))
        (pop-up-windows t))
    (delete-other-windows)
    (switch-to-buffer (other-buffer) nil t)
    (bookmark--jump-via bmrk 'pop-to-buffer)
    (bury-buffer menu)))


(defun bookmark-bmenu-this-window ()
  "Select this line's bookmark in this window."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-jump (bookmark-bmenu-bookmark)))


(defun bookmark-bmenu-other-window ()
  "Select this line's bookmark in other window, leaving bookmark menu visible."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))


(defun bookmark-bmenu-other-frame ()
  "Select this line's bookmark in other frame."
  (interactive nil bookmark-bmenu-mode)
  (let  ((bookmark (bookmark-bmenu-bookmark))
         (pop-up-frames t))
    (bookmark-jump-other-window bookmark)))

(defun bookmark-bmenu-switch-other-window ()
  "Make the other window select this line's bookmark.
The current window remains selected."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark))
	(fun (lambda (b) (display-buffer b t))))
    (bookmark--jump-via bookmark fun)))

(defun bookmark-bmenu-other-window-with-mouse (event)
  "Jump to bookmark at mouse EVENT position in other window.
Move point in menu buffer to the position of EVENT and leave
bookmark menu visible."
  (interactive "e" bookmark-bmenu-mode)
  (with-current-buffer (window-buffer (posn-window (event-end event)))
    (save-excursion
      (goto-char (posn-point (event-end event)))
      (bookmark-bmenu-other-window))))


(defun bookmark-bmenu-show-annotation ()
  "Show the annotation for the current bookmark in another window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark-show-annotation bookmark)))


(defun bookmark-bmenu-show-all-annotations ()
  "Show the annotation for all bookmarks in another window."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-show-all-annotations))


(defun bookmark-bmenu-edit-annotation ()
  "Edit the annotation for the current bookmark in another window."
  (interactive nil bookmark-bmenu-mode)
  (let ((bookmark (bookmark-bmenu-bookmark)))
    (bookmark-edit-annotation bookmark t)))


(defun bookmark-bmenu-unmark (&optional backup)
  "Cancel all requested operations on bookmark on this line and move down.
Optional BACKUP means move up."
  (interactive "P" bookmark-bmenu-mode)
  ;; any flags to reset according to circumstances?  How about a
  ;; flag indicating whether this bookmark is being visited?
  ;; well, we don't have this now, so maybe later.
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag " ")
  (forward-line (if backup -1 1)))


(defun bookmark-bmenu-backup-unmark ()
  "Move up and cancel all requested operations on bookmark on line above."
  (interactive nil bookmark-bmenu-mode)
  (forward-line -1)
  (bookmark-bmenu-ensure-position)
  (bookmark-bmenu-unmark)
  (forward-line -1)
  (bookmark-bmenu-ensure-position))


(defun bookmark-bmenu-unmark-all ()
  "Cancel all requested operations on all listed bookmarks."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag " " t))))


(defun bookmark-bmenu-delete ()
  "Mark bookmark on this line to be deleted.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-ensure-position)
  (tabulated-list-put-tag "D" t))


(defun bookmark-bmenu-delete-backwards ()
  "Mark bookmark on this line to be deleted, then move up one line.
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (bookmark-bmenu-delete)
  (forward-line -2))


(defun bookmark-bmenu-delete-all ()
  "Mark all listed bookmarks as to be deleted.
To remove all deletion marks, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-unmark-all].
To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\[bookmark-bmenu-execute-deletions]."
  (interactive nil bookmark-bmenu-mode)
  (save-excursion
    (goto-char (point-min))
    (bookmark-bmenu-ensure-position)
    (while (not (eobp))
      (tabulated-list-put-tag "D" t))))


(defun bookmark-bmenu-execute-deletions ()
  "Delete bookmarks flagged `D'.
If `bookmark-menu-confirm-deletion' is non-nil, prompt for
confirmation first."
  (interactive nil bookmark-bmenu-mode)
  (if (and bookmark-menu-confirm-deletion
           (not (yes-or-no-p "Delete selected bookmarks? ")))
      (message "Bookmarks not deleted.")
    (let ((reporter (make-progress-reporter "Deleting bookmarks..."))
          (o-point  (point))
          (o-str    (save-excursion
                      (beginning-of-line)
                      (unless (= (following-char) ?D)
                        (buffer-substring
                         (point)
                         (progn (end-of-line) (point))))))
          (o-col     (current-column)))
      (goto-char (point-min))
      (while (re-search-forward "^D" (point-max) t)
        (bookmark-delete (bookmark-bmenu-bookmark) t)) ; pass BATCH arg
      (bookmark-bmenu-list)
      (if o-str
          (progn
            (goto-char (point-min))
            (search-forward o-str)
            (beginning-of-line)
            (forward-char o-col))
        (goto-char o-point))
      (beginning-of-line)
      (progress-reporter-done reporter))))


(defun bookmark-bmenu-rename ()
  "Rename bookmark on current line.  Prompts for a new name."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (thispoint (point)))
    (bookmark-rename bmrk)
    (goto-char thispoint)))


(defun bookmark-bmenu-locate ()
  "Display location of this bookmark.  Displays in the minibuffer."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark)))
    (message "%s" (bookmark-location bmrk))))

(defun bookmark-bmenu-relocate ()
  "Change the absolute file name of the bookmark on the current line.
Prompt with completion for the new path."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmrk (bookmark-bmenu-bookmark))
        (thispoint (point)))
    (bookmark-relocate bmrk)
    (goto-char thispoint)))

;;; Bookmark-bmenu search

(defun bookmark-bmenu-filter-alist-by-regexp (regexp)
  "Filter `bookmark-alist' with bookmarks matching REGEXP and rebuild list."
  (let ((bookmark-alist
         (cl-loop for i in bookmark-alist
                  when (string-match regexp (car i)) collect i into new
                  finally return new)))
    (bookmark-bmenu-list)))


;;;###autoload
(defun bookmark-bmenu-search ()
  "Incremental search of bookmarks, hiding the non-matches as we go."
  (interactive nil bookmark-bmenu-mode)
  (let ((bmk (bookmark-bmenu-bookmark))
        (timer nil))
    (unwind-protect
        (minibuffer-with-setup-hook
            (lambda ()
              (setq timer (run-with-idle-timer
                           bookmark-search-delay 'repeat
                           (lambda (buf)
                             (with-current-buffer buf
                               (bookmark-bmenu-filter-alist-by-regexp
                                (minibuffer-contents))))
                           (current-buffer))))
          (read-string "Pattern: ")
          (when timer (cancel-timer timer) (setq timer nil)))
      (when timer ;; Signaled an error or a `quit'.
        (cancel-timer timer)
        (bookmark-bmenu-list)
        (bookmark-bmenu-goto-bookmark bmk)))))

(defun bookmark-bmenu-goto-bookmark (name)
  "Move point to bookmark with name NAME."
  (goto-char (point-min))
  (while (not (or (equal name (bookmark-bmenu-bookmark))
                  (eobp)))
    (forward-line 1))
  (forward-line 0))


\f
;;; Menu bar stuff.  Prefix is "bookmark-menu".

(defun bookmark-menu-popup-paned-menu (event name entries)
  "Pop up multi-paned menu at EVENT, return string chosen from ENTRIES.
That is, ENTRIES is a list of strings which appear as the choices
in the menu.
The number of panes depends on the number of entries.
The visible entries are truncated to `bookmark-menu-length', but the
strings returned are not."
  (let ((f-height (/ (frame-height) 2))
	(pane-list nil)
	(iter 0))
    (while entries
      (let (lst
	    (count 0))
	(while (and (< count f-height) entries)
	  (let ((str (car entries)))
	    (push (cons
		   (if (> (length str) bookmark-menu-length)
		       (substring str 0 bookmark-menu-length)
		     str)
		   str)
		  lst)
	    (setq entries (cdr entries))
	    (setq count (1+ count))))
	(setq iter (1+ iter))
	(push (cons
	       (format "-*- %s (%d) -*-" name iter)
	       (nreverse lst))
	      pane-list)))

    ;; Popup the menu and return the string.
    (x-popup-menu event (cons (concat "-*- " name " -*-")
			      (nreverse pane-list)))))


;; Thanks to Roland McGrath for fixing menubar.el so that the
;; following works, and for explaining what to do to make it work.

;; We MUST autoload EACH form used to set up this variable's value, so
;; that the whole job is done in loaddefs.el.

;; Emacs menubar stuff.

;;;###autoload
(defvar menu-bar-bookmark-map
  (let ((map (make-sparse-keymap "Bookmark functions")))
    (bindings--define-key map [load]
      '(menu-item "Load a Bookmark File..." bookmark-load
		  :help "Load bookmarks from a bookmark file)"))
    (bindings--define-key map [write]
      '(menu-item "Save Bookmarks As..." bookmark-write
		  :help "Write bookmarks to a file (reading the file name with the minibuffer)"))
    (bindings--define-key map [save]
      '(menu-item "Save Bookmarks" bookmark-save
		  :help "Save currently defined bookmarks"))
    (bindings--define-key map [edit]
      '(menu-item "Edit Bookmark List" bookmark-bmenu-list
		  :help "Display a list of existing bookmarks"))
    (bindings--define-key map [delete]
      '(menu-item "Delete Bookmark..." bookmark-delete
		  :help "Delete a bookmark from the bookmark list"))
    (bindings--define-key map [delete-all]
      '(menu-item "Delete all Bookmarks..." bookmark-delete-all
		  :help "Delete all bookmarks from the bookmark list"))
    (bindings--define-key map [rename]
      '(menu-item "Rename Bookmark..." bookmark-rename
		  :help "Change the name of a bookmark"))
    (bindings--define-key map [locate]
      '(menu-item "Insert Location..." bookmark-locate
		  :help "Insert the name of the file associated with a bookmark"))
    (bindings--define-key map [insert]
      '(menu-item "Insert Contents..." bookmark-insert
		  :help "Insert the text of the file pointed to by a bookmark"))
    (bindings--define-key map [set]
      '(menu-item "Set Bookmark..." bookmark-set
		  :help "Set a bookmark named inside a file."))
    (bindings--define-key map [jump]
      '(menu-item "Jump to Bookmark..." bookmark-jump
		  :help "Jump to a bookmark (a point in some file)"))
    map))

;;;###autoload
(defalias 'menu-bar-bookmark-map menu-bar-bookmark-map)

;; make bookmarks appear toward the right side of the menu.
(if (boundp 'menu-bar-final-items)
    (if menu-bar-final-items
        (push 'bookmark menu-bar-final-items))
  (setq menu-bar-final-items '(bookmark)))

;;;; end bookmark menu stuff ;;;;

\f
;; Load Hook
(defvar bookmark-load-hook nil
  "Hook run at the end of loading library `bookmark.el'.")
(make-obsolete-variable 'bookmark-load-hook
                        "use `with-eval-after-load' instead." "28.1")

;; Exit Hook, called from kill-emacs-hook
(defvar bookmark-exit-hook nil
  "Hook run when Emacs exits.")

(defun bookmark-exit-hook-internal ()
  "Save bookmark state, if necessary, at Emacs exit time.
This also runs `bookmark-exit-hook'."
  (run-hooks 'bookmark-exit-hook)
  (and (bookmark-time-to-save-p t)
       (bookmark-save)))

(unless noninteractive
  (add-hook 'kill-emacs-hook 'bookmark-exit-hook-internal))

(defun bookmark-unload-function ()
  "Unload the Bookmark library."
  (when bookmark-save-flag (bookmark-save))
  ;; continue standard unloading
  nil)


(run-hooks 'bookmark-load-hook)

\f
;;; Obsolete:

(define-obsolete-function-alias 'bookmark-send-edited-annotation
  #'bookmark-edit-annotation-confirm "29.1")

(provide 'bookmark)

;;; bookmark.el ends here

[-- Attachment #5: pdf-tools-handler-fix.el --]
[-- Type: application/octet-stream, Size: 11034 bytes --]

;; The original PDF Tools bookmark handler, plus some alternative definitions, which work
;; with both vanilla Emacs and Bookmark+, in two groups.  The first group work without
;; patching `bookmark.el'.  The second group require the patch.
;;
;; The original and the first group create a closure and put it on the after-jump hook.
;; They depend on lexical binding (for the closure).  The second group work with or
;; without lexical binding.



;; 1. ORIGINAL definition.  Handle what can be handled before displaying the destination,
;;    and put what needs to be done after displaying onto `bookmark-after-jump-hook'.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page (bookmark-prop-get bmk 'page))
        (slice (bookmark-prop-get bmk 'slice))
        (size (bookmark-prop-get bmk 'size))
        (origin (bookmark-prop-get bmk 'origin))
        (file (bookmark-prop-get bmk 'filename))
        (show-fn-sym (make-symbol "pdf-view-bookmark-after-jump-hook")))

    ;; Create a closure that encapsulates and uses PAGE, SLICE, SIZE, and ORIGIN.
    (fset show-fn-sym
          (lambda ()
            (remove-hook 'bookmark-after-jump-hook show-fn-sym)
            (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
            (with-selected-window
                (or (get-buffer-window (current-buffer) 0)
                    (selected-window))
              (when size (setq-local pdf-view-display-size size))
              (when slice (apply 'pdf-view-set-slice slice))
              (when (numberp page) (pdf-view-goto-page page))
              (when origin
                (let ((size (pdf-view-image-size t)))
                  (image-set-window-hscroll
                   (round (/ (* (car origin) (car size))
                             (frame-char-width))))
                  (image-set-window-vscroll
                   (round (/ (* (cdr origin) (cdr size))
                             (if pdf-view-have-image-mode-pixel-vscroll
                                 1
                               (frame-char-height))))))))))

    ;; Put the closure on the after-jump hook, visit FILE in the current buffer.
    (add-hook 'bookmark-after-jump-hook show-fn-sym)
    (set-buffer (or (find-buffer-visiting file)
                    (find-file-noselect file)))))

\f
;;---------------------------------------------------------------------------------
;; Alternative definitions that work with current `bookmark.el' and Bookmark+.
;;
;; These don't require the patch.  They also don't simplify the handler - it still uses
;; a closure on the after-jump hook.
;;---------------------------------------------------------------------------------


;; 2. Use a display function instead of just `set-buffer'.  Otherwise same as original.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page (bookmark-prop-get bmk 'page))
        (slice (bookmark-prop-get bmk 'slice))
        (size (bookmark-prop-get bmk 'size))
        (origin (bookmark-prop-get bmk 'origin))
        (file (bookmark-prop-get bmk 'filename))
        (show-fn-sym (make-symbol "pdf-view-bookmark-after-jump-hook")))

    ;; Create a closure that encapsulates and uses PAGE, SLICE, SIZE, and ORIGIN.
    (fset show-fn-sym
          (lambda ()
            (remove-hook 'bookmark-after-jump-hook show-fn-sym)
            (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
            (with-selected-window
                (or (get-buffer-window (current-buffer) 0)
                    (selected-window))
              (when size (setq-local pdf-view-display-size size))
              (when slice (apply 'pdf-view-set-slice slice))
              (when (numberp page) (pdf-view-goto-page page))
              (when origin
                (let ((size (pdf-view-image-size t)))
                  (image-set-window-hscroll
                   (round (/ (* (car origin) (car size))
                             (frame-char-width))))
                  (image-set-window-vscroll
                   (round (/ (* (cdr origin) (cdr size))
                             (if pdf-view-have-image-mode-pixel-vscroll
                                 1
                               (frame-char-height))))))))))

    ;; Put the closure on the after-jump hook, visit FILE in the current buffer.
    (add-hook 'bookmark-after-jump-hook show-fn-sym)

    ;; Display a buffer visiting FILE.
    (pop-to-buffer-same-window (or (find-buffer-visiting file)
                                   (find-file-noselect file)))))


;; 3. Invoke the default handler, so the destination gets displayed by `bookmark-jump'.
;;    Otherwise same as original.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page (bookmark-prop-get bmk 'page))
        (slice (bookmark-prop-get bmk 'slice))
        (size (bookmark-prop-get bmk 'size))
        (origin (bookmark-prop-get bmk 'origin))
        (file (bookmark-prop-get bmk 'filename))
        (show-fn-sym (make-symbol "pdf-view-bookmark-after-jump-hook")))
    (fset show-fn-sym
          (lambda ()

            ;; Invoke the default handler.
            (bookmark-default-handler (bookmark-get-bookmark bmk))

            (remove-hook 'bookmark-after-jump-hook show-fn-sym)
            (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
            (with-selected-window
                (or (get-buffer-window (current-buffer) 0) (selected-window))
              (when size (setq-local pdf-view-display-size size))
              (when slice (apply 'pdf-view-set-slice slice))
              (when (numberp page) (pdf-view-goto-page page))
              (when origin
                (let ((size (pdf-view-image-size t)))
                  (image-set-window-hscroll
                   (round (/ (* (car origin) (car size)) (frame-char-width))))
                  (image-set-window-vscroll
                   (round (/ (* (cdr origin) (cdr size))
                             (if pdf-view-have-image-mode-pixel-vscroll
                                 1
                               (frame-char-height))))))))))
    (add-hook 'bookmark-after-jump-hook show-fn-sym)
    (set-buffer (or (find-buffer-visiting file)
                    (find-file-noselect file)))))

;; 4. Advise the original handler, so it invokes the default handler.
;;
(defun my-bmk-pdf-handler-advice (bookmark)
  (bookmark-default-handler (bookmark-get-bookmark bookmark)))

(advice-add 'pdf-view-bookmark-jump-handler :after 'my-bmk-pdf-handler-advice)

\f
;;---------------------------------------------------------------------------------
;; Alternative definitions that work with patched `bookmark.el' and Bookmark+.
;;
;; Simpler handler code - no need to use the after-jump hook to finish handling.
;;---------------------------------------------------------------------------------


;; 5. Invoke default handler, to display destination using the default display function.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page    (bookmark-prop-get bmk 'page))
        (slice   (bookmark-prop-get bmk 'slice))
        (size    (bookmark-prop-get bmk 'size))
        (origin  (bookmark-prop-get bmk 'origin))
        (file    (bookmark-prop-get bmk 'filename)))
    (set-buffer (or (find-buffer-visiting file)  (find-file-noselect file)))

    ;; Invoke the default handler, to display the destination.
    (bookmark-default-handler (bookmark-get-bookmark bmk))

    ;; Do what was formerly relegated to the after-jump hook.
    (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
    (with-selected-window (or (get-buffer-window (current-buffer) 0)  (selected-window))
      (when size (setq-local pdf-view-display-size size))
      (when slice (apply 'pdf-view-set-slice slice))
      (when (numberp page) (pdf-view-goto-page page))
      (when origin
        (let ((size (pdf-view-image-size t)))
          (image-set-window-hscroll
           (round (/ (* (car origin) (car size)) (frame-char-width))))
          (image-set-window-vscroll
           (round (/ (* (cdr origin) (cdr size)) (if pdf-view-have-image-mode-pixel-vscroll
                                                     1
                                                   (frame-char-height))))))))))


;; 6. Call the default display function directly.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page    (bookmark-prop-get bmk 'page))
        (slice   (bookmark-prop-get bmk 'slice))
        (size    (bookmark-prop-get bmk 'size))
        (origin  (bookmark-prop-get bmk 'origin))
        (file    (bookmark-prop-get bmk 'filename)))
    (set-buffer (or (find-buffer-visiting file)  (find-file-noselect file)))

    ;; Display buffer using default display function, in `bookmark-jump-display-function'.
    (save-current-buffer (funcall bookmark-jump-display-function (current-buffer)))

    ;; Do what was formerly relegated to the after-jump hook.
    (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
    (with-selected-window (or (get-buffer-window (current-buffer) 0)  (selected-window))
      (when size (setq-local pdf-view-display-size size))
      (when slice (apply 'pdf-view-set-slice slice))
      (when (numberp page) (pdf-view-goto-page page))
      (when origin
        (let ((size (pdf-view-image-size t)))
          (image-set-window-hscroll
           (round (/ (* (car origin) (car size)) (frame-char-width))))
          (image-set-window-vscroll
           (round (/ (* (cdr origin) (cdr size)) (if pdf-view-have-image-mode-pixel-vscroll
                                                     1
                                                   (frame-char-height))))))))))


;; 7. Call a different display function directly.
;;
(defun pdf-view-bookmark-jump-handler (bmk)
  "..."
  (let ((page    (bookmark-prop-get bmk 'page))
        (slice   (bookmark-prop-get bmk 'slice))
        (size    (bookmark-prop-get bmk 'size))
        (origin  (bookmark-prop-get bmk 'origin))
        (file    (bookmark-prop-get bmk 'filename)))
    (set-buffer (or (find-buffer-visiting file)  (find-file-noselect file)))

    ;; Display buffer using another display function (display in another tab).
    (save-current-buffer (switch-to-buffer-other-tab (current-buffer)))

    ;; Do what was formerly relegated to the after-jump hook.
    (unless (derived-mode-p 'pdf-view-mode) (pdf-view-mode))
    (with-selected-window (or (get-buffer-window (current-buffer) 0)  (selected-window))
      (when size (setq-local pdf-view-display-size size))
      (when slice (apply 'pdf-view-set-slice slice))
      (when (numberp page) (pdf-view-goto-page page))
      (when origin
        (let ((size (pdf-view-image-size t)))
          (image-set-window-hscroll
           (round (/ (* (car origin) (car size)) (frame-char-width))))
          (image-set-window-vscroll
           (round (/ (* (cdr origin) (cdr size)) (if pdf-view-have-image-mode-pixel-vscroll
                                                     1
                                                   (frame-char-height))))))))))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Let bookmark handlers do everything, including display the destination
  2022-08-13 20:59 [PATCH] Let bookmark handlers do everything, including display the destination Drew Adams
@ 2022-08-18 18:14 ` Karl Fogel
  2022-08-29 15:28   ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Karl Fogel @ 2022-08-18 18:14 UTC (permalink / raw)
  To: Drew Adams; +Cc: emacs-devel@gnu.org

On 13 Aug 2022, Drew Adams wrote:
>tl;dr:
>
>Let's change the handling of a bookmark - including
>displaying the bookmark destination - to be the
>responsibility of its handler.

In general, +1 to this idea.

(My ability to be involved in actually making it happen may be 
limited right now, due to some family responsibilities.  I'll try 
to at least kibbitz constructively though.)

>Attached is a patch that does that.  It moves the displaying
>of a bookmark's "destination" from `bookmark-jump' to the
>bookmark itself - specifically, to its handler.
>
>Currently `bookmark-jump', rather than a bookmark's handler,
>displays the bookmark destination.  And the behavior's
>hard-coded: a bookmark always displays a destination, and
>that's always the last thing done before the after-jump
>hook.
>
>That's too rigid.  It means that a handler function can't do
>something more after the display, unless it finagles to put
>that after-display processing on `bookmark-after-jump-hook'
>(temporarily).  And it means that a handler can't choose its
>own display function - the DISPLAY-FUNCTION passed to
>`bookmark-jump' is always used.  And it means that a handler
>can't dispense with display altogether.
>
>By moving the responsibility for display to the handler, the
>patch gets rid of this rigidity.
>
>Specifically, instead of `bookmark--jump-via' systematically
>invoking its DISPLAY-FUNCTION argument, it saves the
>argument value in variable `bookmark-jump-display-function'.
>And then the default handler, `bookmark-default-handler'
>invokes that function.

*nod*

There's some parallelism-unsafety going on here, obviously.  I'd 
love a way to wrap all this in a clean closure, instead of setting 
a global variable which then retains that setting until the next 
time we happen to set it.

But I don't really see a way to do tihngs more cleanly than with 
the carry-out variable you propose.  If there's some creative 
solution for connecting `bookmark--jump-via' to the 
"corresponding" handler call that happens later, I don't see it, 
lexically or dynamically.  Maybe is just how Emacs usually handles 
these kinds of situations?

>Most bookmarks don't have their own handler; they use the
>default handler.  So their behavior doesn't change at all
>with this approach.  Most bookmarks use `bookmark-jump's
>DISPLAY FUNCTION as the last thing before running
>`bookmark-after-jump-hook' (and most should do that).
>
>The same applies to bookmarks that do have their own
>handler, if the handler invokes the default handler at the
>end.  For example, the Info bookmark handler,
>`Info-bookmark-jump' sets up what it needs, and ends by
>invoking the default handler:
>
>  (bookmark-default-handler
>   `("" (buffer . ,buf)
>     . ,(bookmark-get-bookmark-record bookmark)))
>
>What changes with the proposed approach is only what can
>happen for a bookmark that has its own handler, if that
>handler either (1) invokes the default handler at some point
>other than at the end - so it does something after
>displaying, or (2) it doesn't invoke the default handler.
>
>If a handler doesn't invoke the default handler, it can call
>whatever display function it likes, whenever it likes.  Or
>it can forego display altogether.
>
>This approach gives the handler more control - it can decide
>whether, when, and how to display the destination.  The
>entire behavior of a bookmark is then up to its handler.
>
>And this allows for more kinds of bookmarks, including
>bookmarks that don't display a destination.
>
>[...some example usages from Bookmark+...]

This is all very well-explained; thank you, Drew.

>[...]
>
>But really, the complications `pdf-view-bookmark-handler'
>goes through - creating a closure on the fly and adding it
>to the after-jump hook (and subsequently removing it) - are
>a workaround for the fact that vanilla `bookmark.el' doesn't
>let a handler display the destination when it wants to.
>
>PDF Tools wants to set up the buffer, display it, and then
>go to the right PDF page etc.  So it sets up the buffer,
>ends the handler, counts on `bookmark-jump' to display the
>buffer, and then has a (temporary) after-jump hook do the
>rest of the handling: go to the right page etc.  (The hook
>function captures some info from the bookmark, so it needs
>to be a closure.)  A clever workaround.
>
>With the approach provided by the patch, the handler can do
>all of that itself: set up the buffer, display it, then go
>to the page etc.  No after-jump-hook workaround.  The code
>is quite a bit simpler.

Yup; that makes sense to me.

>Between setting up the buffer and doing the additional stuff
>that was relegated to a temporary after-jump hook, the
>handler can either (1) just invoke the default handler or
>(2) just call a display function.  Code for each of those
>handler definitions is also attached, for comparison with
>the above code.
>
>Those solutions are examples #5 and #6, respectively, in
>attachment `pdf-tools-handler-fix.el'.
>
>I mention the PDF Tools handler as an example.  I'm not
>aware of other handlers that depend on `bookmark-jump'
>displaying some buffer and selecting its window, and then
>depend on the after-jump hook for additional handling in
>that buffer, but there may be some out there.
>
>This is a general problem for Bookmark+, even if it seems to
>be run into rarely because most handlers invoke the default
>handler (which in Bookmark+ displays the destination using
>the DISPLAY-FUNCTION passed to `bookmark-jump').
>
>But this is not just about getting rid of an incompatibility
>with Bookmark+'s bookmark handling.  It's also about
>improving vanilla `bookmark.el', by letting the handler
>handle a bookmark's behavior - all of it.  Give handlers the
>control over display, and have the default handler invoke
>the `bookmark-jump' DISPLAY-FUNCTION arg.

My main concern here is just that compatibility issue.  I have no 
idea how many bookmark-using packages will break if we install 
this patch.  You've explained very clearly how to fix them, and 
how in many cases their code is likely to get cleaner as a result, 
and I agree.

But still, compatibility-breaking changes are a serious thing. 
I'd like to know what others here think about this.  Your 
suggested guidelines below seem good *if* we decide to make this 
change:

>That makes possible bookmarks that don't necessarily display
>a destination - they perform some other action.  And it
>allows bookmarks to do any kind of displaying whatsoever
>(not just what the `bookmark-jump' DISPLAY-FUNCTION
>provides).
>
>Regardless of whether we adopt the proposed behavior, we
>might anyway want to post a guideline to let people who
>define custom bookmark handlers know that it's
>common/typical for a handler to invoke the default handler -
>that repositions the bookmark as needed etc.
>
>But with the new behavior we should also make clear that for
>a bookmark destination to be displayed, a custom handler
>needs to invoke either `bookmark-default-handler' or a
>display function.
>
>More generally, the thing to make clear is that a handler
>completely defines a bookmark's behavior.  And in
>particular, it decides whether, when, and how to display a
>bookmark destination.

Yup.

>I think the proposed approach (attached patch or similar)
>would affect very few people.  It would affect only library
>authors who define custom bookmark handlers (few do) that
>don't invoke the default handler (fewer still), AND whose
>code relegates some of the handling to the after-jump hook
>and depends on `bookmark-jump' invoking the DISPLAY-FUNCTION
>(very few, I expect).

I expect you are right.  I'm even inclined to risk the breakage 
and help callers fix it, just to get this long-term structural 
improvement.

But I'm not confident in this opinion, and would like to know what 
others here, especially the maintainers, think.

I reviewed the bookmark.el patch briefly, and overall it had the 
shape I expected.  I might have a few suggestions about doc 
strings and such; if we decide to go down this road, I'll do a 
real patch review.

Best regards,
-Karl










^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Let bookmark handlers do everything, including display the destination
  2022-08-18 18:14 ` Karl Fogel
@ 2022-08-29 15:28   ` Stefan Monnier
  2022-08-29 23:45     ` Karl Fogel
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2022-08-29 15:28 UTC (permalink / raw)
  To: Karl Fogel; +Cc: Drew Adams, emacs-devel@gnu.org

>>Specifically, instead of `bookmark--jump-via' systematically
>>invoking its DISPLAY-FUNCTION argument, it saves the
>>argument value in variable `bookmark-jump-display-function'.
>>And then the default handler, `bookmark-default-handler'
>>invokes that function.
>
> *nod*
>
> There's some parallelism-unsafety going on here, obviously.  I'd love a way
> to wrap all this in a clean closure, instead of setting a global variable
> which then retains that setting until the next time we happen to set it.

Indeed :-(

> But I don't really see a way to do tihngs more cleanly than with the
> carry-out variable you propose.  If there's some creative solution for
> connecting `bookmark--jump-via' to the "corresponding" handler call that
> happens later, I don't see it, lexically or dynamically.  Maybe is just how
> Emacs usually handles these kinds of situations?

Hmm... I must be missing something.  Why can't `bookmark--jump-via`
let-bind `bookmark-jump-display-function` around the call to
`bookmark-handle-bookmark`?

Regarding the patch, here are some comments:

> +  To display the destination, HANDLER can call the function that's the
> +  value of variable `bookmark-jump-display-function', which is set by
> +  `bookmark-jump' to automatically accommodate other-window etc.
> +  displaying that depends on the jump command.  For example:
> +
> +   (funcall bookmark-jump-display-function (current-buffer))
> +
> +  Or HANDLER can directly call another display function.  For example:
> +
> +   (switch-to-buffer-other-tab BUFFER)

I skipped most of the rest of the docstring's massaging, which I'm not
sure is an improvement but with which I guess I can live.  But the above
goes significantly beyond what one usually expects from a docstring.
It would fit much better in a Texinfo manual instead.

> +  Or HANDLER can invoke `bookmark-default-handler'.  That displays the
> +  destination.  It then moves to the recorded buffer position, POS,
> +  repositioning point, if necessary, to match the recorded context
> +  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, the Info
> +  handler, `Info-bookmark-jump', does this at its end:

And this is (or should be) redundant with the docstring of
`bookmark-default-handler` so better keep the link and the the readers
jump to it when they need/want to know what that function does.

> +(defvar bookmark-jump-display-function nil
> + "Function used currently to display a bookmark, or nil if no function.")

Please give it a function as default value (e.g. `ignore`).

> @@ -1350,6 +1391,9 @@
>        ((and buf (get-buffer buf)))
>        (t ;; If not, raise error.
>         (signal 'bookmark-error-no-filename (list 'stringp file)))))
> +    (when bookmark-jump-display-function
> +      (save-current-buffer (funcall bookmark-jump-display-function
> +                                    (current-buffer))))

Why `save-current-buffer`?

>      (if place (goto-char place))
>      ;; Go searching forward first.  Then, if forward-str exists and
>      ;; was found in the file, we can search backward for behind-str.

Hmm... `bookmark-default-handler` is also called via things like
`bookmark-insert`, so I think `bookmark-insert` should explicitly bind
`bookmark-jump-display-function` to `ignore`, and of course that begs
the question of what to do for handlers which don't obey
`bookmark-jump-display-function`.

I think it's good to provide more control to the bookmark's handler, but
there seems to be a need for the caller to control the display as well
to some extent.

BTW, your generalization of bookmarks to "anything" makes it all the
more obvious to me that these should be unified with "registers" (not
necessarily at the UI level, but a register should be able to hold
anything that can stored as a bookmark, and vice versa).


        Stefan





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Let bookmark handlers do everything, including display the destination
  2022-08-29 15:28   ` Stefan Monnier
@ 2022-08-29 23:45     ` Karl Fogel
  2022-09-06  0:07       ` [External] : " Drew Adams
  0 siblings, 1 reply; 6+ messages in thread
From: Karl Fogel @ 2022-08-29 23:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Drew Adams, emacs-devel@gnu.org

On 29 Aug 2022, Stefan Monnier wrote:
>> There's some parallelism-unsafety going on here, obviously. 
>> I'd love a way
>> to wrap all this in a clean closure, instead of setting a 
>> global variable
>> which then retains that setting until the next time we happen 
>> to set it.
>
> Indeed :-(
>
>> But I don't really see a way to do tihngs more cleanly than 
>> with the
>> carry-out variable you propose.  If there's some creative 
>> solution for
>> connecting `bookmark--jump-via' to the "corresponding" handler 
>> call that
>> happens later, I don't see it, lexically or dynamically.  Maybe 
>> is just how
>> Emacs usually handles these kinds of situations?
>
> Hmm... I must be missing something.  Why can't 
> `bookmark--jump-via`
> let-bind `bookmark-jump-display-function` around the call to
> `bookmark-handle-bookmark`?

Of course -- obvious, now that you point it out.  Thank you for 
reviewing, Stefan.

Drew, does that sound good to you?

> Regarding the patch, here are some comments:
>
>> +  To display the destination, HANDLER can call the function 
>> that's the
>> +  value of variable `bookmark-jump-display-function', which is 
>> set by
>> +  `bookmark-jump' to automatically accommodate other-window 
>> etc.
>> +  displaying that depends on the jump command.  For example:
>> +
>> +   (funcall bookmark-jump-display-function (current-buffer))
>> +
>> +  Or HANDLER can directly call another display function.  For 
>> example:
>> +
>> +   (switch-to-buffer-other-tab BUFFER)
>
> I skipped most of the rest of the docstring's massaging, which 
> I'm not
> sure is an improvement but with which I guess I can live.  But 
> the above
> goes significantly beyond what one usually expects from a 
> docstring.
> It would fit much better in a Texinfo manual instead.
>
>> +  Or HANDLER can invoke `bookmark-default-handler'.  That 
>> displays the
>> +  destination.  It then moves to the recorded buffer position, 
>> POS,
>> +  repositioning point, if necessary, to match the recorded 
>> context
>> +  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, the 
>> Info
>> +  handler, `Info-bookmark-jump', does this at its end:
>
> And this is (or should be) redundant with the docstring of
> `bookmark-default-handler` so better keep the link and the the 
> readers
> jump to it when they need/want to know what that function does.

Agreed.

>> +(defvar bookmark-jump-display-function nil
>> + "Function used currently to display a bookmark, or nil if no 
>> function.")
>
> Please give it a function as default value (e.g. `ignore`).

I think I understand: by taking your advice, we don't have to 
check the function's existence -- we can just call it 
unconditionally, and if it happens to do nothing (i.e., is 
`ignore'), that's fine.  A specific package may still override the 
default handler and do whatever display magic is called for; 
otherwise, bookmark.el's default handler will Do The Right Thing 
for displaying.  But all this raises the question you raise 
below...

>> @@ -1350,6 +1391,9 @@
>>        ((and buf (get-buffer buf)))
>>        (t ;; If not, raise error.
>>         (signal 'bookmark-error-no-filename (list 'stringp 
>>         file)))))
>> +    (when bookmark-jump-display-function
>> +      (save-current-buffer (funcall 
>> bookmark-jump-display-function
>> +                                    (current-buffer))))
>
> Why `save-current-buffer`?
>
>>      (if place (goto-char place))
>>      ;; Go searching forward first.  Then, if forward-str 
>>      exists and
>>      ;; was found in the file, we can search backward for 
>>      behind-str.
>
> Hmm... `bookmark-default-handler` is also called via things like
> `bookmark-insert`, so I think `bookmark-insert` should 
> explicitly bind
> `bookmark-jump-display-function` to `ignore`, and of course that 
> begs
> the question of what to do for handlers which don't obey
> `bookmark-jump-display-function`.
>
> I think it's good to provide more control to the bookmark's 
> handler, but
> there seems to be a need for the caller to control the display 
> as well
> to some extent.

...which is that we now seem to have two possibly-redundant ways 
to control displaying: write a custom handler, *or* bind 
`bookmark-jump-display-function' to something?

Or perhaps I'm misunderstanding something.  Drew, if you post the 
next revision of the patch incorporating Stefan's points above, I 
will review that in detail, when fully alert, and should be able 
to distill any remaining questions -- I think we'd be pretty close 
at that point.

> BTW, your generalization of bookmarks to "anything" makes it all 
> the
> more obvious to me that these should be unified with "registers" 
> (not
> necessarily at the UI level, but a register should be able to 
> hold
> anything that can stored as a bookmark, and vice versa).

I agree that that makes sense as a long term goal.

Best regards,
-Karl



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [External] : Re: [PATCH] Let bookmark handlers do everything, including display the destination
  2022-08-29 23:45     ` Karl Fogel
@ 2022-09-06  0:07       ` Drew Adams
  2022-09-26 19:47         ` Karl Fogel
  0 siblings, 1 reply; 6+ messages in thread
From: Drew Adams @ 2022-09-06  0:07 UTC (permalink / raw)
  To: Karl Fogel, Stefan Monnier; +Cc: emacs-devel@gnu.org

> >> There's some parallelism-unsafety going on here, obviously.
> >> I'd love a way to wrap all this in a clean closure,
> >> instead of setting a global variable which then retains
> >> that setting until the next time we happen to set it.

Retaining that setting till it's reset could
be a feature, not a bug. ;-)

I imagine I won't have a problem if you bind
it lexically in `bookmark--jump-via', but I'm
not sure.

Bookmark+ is designed to work also with Emacs
versions that don't have lexical binding.
Maybe using `lexical-let' here'll suffice,
maybe not.

A `nil' value does matter - it's not the same
as calling a function unconditionally, even
when the function is `ignore'.

E.g., my version of `bookmark-default-handler'
has this code, which doesn't raise the frame if
there's no display going on (a bookmark need
not display anything):

(when bmkp-jump-display-function
  (save-current-buffer
    (funcall bmkp-jump-display-function (current-buffer)))
  (raise-frame))

Sure, I could change (when XXXX ...) to
(when (eq XXXX 'ignore) ...), but that wouldn't
really improve anything, IMO.

And I test the display function in some handlers,
for conditional handling/dispatch.  E.g., my
handler for Dired bookmarks:

(defun bmkp-jump-dired (bookmark)
  "..."
  (let ((dir
         (bookmark-prop-get bookmark 'dired-directory))
        (mfiles
         (bookmark-prop-get bookmark 'dired-marked))
        (switches
         (bookmark-prop-get bookmark 'dired-switches))
        (subdirs
         (bookmark-prop-get bookmark 'dired-subdirs))
        (hidden-dirs
         (bookmark-prop-get bookmark 'dired-hidden-dirs)))
    (case bmkp-jump-display-function
      ((nil bmkp--pop-to-buffer-same-window display-buffer)
       (dired dir switches))
      ((bmkp-select-buffer-other-window
        pop-to-buffer
        switch-to-buffer-other-window)
       (dired-other-window dir switches))
      (t (dired dir switches)))
    (let ((inhibit-read-only  t))
      (dired-insert-old-subdirs subdirs)
      (dired-mark-remembered 
       (mapcar (lexical-let ((dd  dir))
                 (lambda (mf)
                   (cons (expand-file-name mf dd) 42)))
               mfiles))
      (save-excursion
        (dolist (dir  hidden-dirs)
          (when (dired-goto-subdir dir) (dired-hide-subdir 1)))))
    (let ((pos  (bookmark-get-position bookmark)))
      (when pos (goto-char pos)))))

Obviously testing for functional equality isn't
possible.  But testing a function symbol lets a
particular kind of bookmark jump differently
depending on current context/settings.  

> > Indeed :-(

Not so obvious to me.  Fine as a general guideline.
Not clear that it's a real win (no loss) here.

> > Why can't `bookmark--jump-via` let-bind
> > `bookmark-jump-display-function` around 
> > the call to `bookmark-handle-bookmark`?
> 
> Of course -- obvious... 
> Drew, does that sound good to you?

See above.  And please show your proposed code.

> > Regarding the patch, here are some comments:
> >
> >> +  To display the destination, HANDLER can call the function
> >> that's the
> >> +  value of variable `bookmark-jump-display-function', which is
> >> set by
> >> +  `bookmark-jump' to automatically accommodate other-window
> >> etc.
> >> +  displaying that depends on the jump command.  For example:
> >> +
> >> +   (funcall bookmark-jump-display-function (current-buffer))
> >> +
> >> +  Or HANDLER can directly call another display function.  For
> >> example:
> >> +
> >> +   (switch-to-buffer-other-tab BUFFER)
> > ...
> > the above goes significantly beyond what one usually
> > expects from a docstring.  It would fit much better
> > in a Texinfo manual instead.

Feel free to write a TexInfo manual for bookmarking,
including for writing bookmark handlers, etc.  What
exists now, and is as old as `bookmark.el', is just
some code comments and that doc string.

That doc string documents the data structure for a
bookmark - it's THE place where that's specified.

The handler part of that is important for people
who create a new bookmark type (always has been).
It's the one bit of bookmark data that really needs
explaining, as it's the one part that provides
behavior (code).

> >> +  Or HANDLER can invoke `bookmark-default-handler'.  That
> >> displays the
> >> +  destination.  It then moves to the recorded buffer position,
> >> POS,
> >> +  repositioning point, if necessary, to match the recorded
> >> context
> >> +  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, the
> >> Info
> >> +  handler, `Info-bookmark-jump', does this at its end:
> >
> > And this is (or should be) redundant with the docstring of
> > `bookmark-default-handler` so better keep the link and the the
> > readers jump to it when they need/want to know what that function does.

How is saying that a custom handler can invoke the
default handler, and summarizing what that does,
redundant here?

Sure, it could just say that your handler can invoke
the default handler, and leave it at that.  But that
just begs the question "Huh? Why?".  And then, when
you go off to look at the doc of that function, all
you find is this (unmodified):

 Default handler to jump to a particular bookmark location.
 BMK-RECORD is a bookmark record, not a bookmark name (i.e., not a string).
 Changes current buffer and point and returns nil, or signals a `file-error'.

 If BMK-RECORD has a property called `buffer', it should be a live
 buffer object, and this buffer will be selected.

(Why shouldn't it be able to be a bookmark name, BTW?)

Does that doc string help you, if you're writing
your own handler?  The code helps, but not that
doc string.  It doesn't tell you what jumping
_means/does_ in the default case.

IMO there's little sense in saying only that your
handler _can_ call the default handler.  And even
less sense in saying neither that it can nor what
calling it does, i.e., _why_ you might want your
handler to invoke the default handler.

But TexInfo manual - welcome.  Go for it, please.

> Agreed.

Please show your proposed code (e.g. doc string).

It's a deficiency of the existing doc string
that it doesn't really tell you anything useful
about the HANDLER part, IMO.

But as I said, feel free to put what you like in
any of the doc strings.

The patch doesn't _require_ any mention of the
possibility or use case of calling the default
handler from a nondefault handler.  I think it's
good to mention that, but you needn't do so to
make the code change.

> >> +(defvar bookmark-jump-display-function nil
> >> + "Function used currently to display a bookmark, or nil if no
> >> function.")
> >
> > Please give it a function as default value (e.g. `ignore`).

See above.  I don't think anything's really gained
by doing that here.  As a general guideline, sure,
no problem.  But let's hear a specific argument
for doing that here, please.

> I think I understand: by taking your advice, we don't have to
> check the function's existence -- we can just call it
> unconditionally, and if it happens to do nothing (i.e., is
> `ignore'), that's fine.

Not so fine, IMO.  And you might still need to test
its existence - with `(eq 'ignore)' instead of just
non-nil.  And then there's trying to test for
functions "equivalent" to `ignore'...

[FWIW, a somewhat related feature, which I didn't
include in my patch (but which could be considered
for another change), wraps most of the code of
`bookmark--jump-via' in a `catch', to which a
handler can `throw' to avoid doing anything else
(e.g. after-jump hook, auto-showing annotations).]

> A specific package may still override the
> default handler and do whatever display magic is called for;
> otherwise, bookmark.el's default handler will Do The Right Thing
> for displaying.

If "otherwise" means that if a handler doesn't
use a display function then the display of the
default handler is used, then that's wrong.  A
handler needs to be able to not display anything.

> But all this raises the question you raise below...

What question below?  Do you mean the question
"Why `save-current-buffer'?"

> >> @@ -1350,6 +1391,9 @@
> >>        ((and buf (get-buffer buf)))
> >>        (t ;; If not, raise error.
> >>         (signal 'bookmark-error-no-filename (list 'stringp
> >>         file)))))
> >> +    (when bookmark-jump-display-function
> >> +      (save-current-buffer
> >> +        (funcall bookmark-jump-display-function
> >> +                 (current-buffer))))
> >
> > Why `save-current-buffer`?

That's already in the code that the patch
replaces.  It's in `bookmark--jump-via'.
Displaying is now in the default handler (or
another handler).  That's all.

Take out that ` save-current-buffer', if you
think you don't need it.  I'll likely leave it
in my code.

You can fiddle with the `bookmark--jump-via'
code if you like, to ensure that the behavior
change I described is the only one that's made.
I think that's already the case, but feel free.

The code right after `(save-current-buffer...)'
expects the buffer that is current not to have
changed.  In the unpatched code that right-after
code is in `bookmark--jump-via'.  In the patched
code it's in the default handler.

> >>      (if place (goto-char place))
> >>      ;; Go searching forward first.  Then, if forward-str
> >>      exists and
> >>      ;; was found in the file, we can search backward for
> >>      behind-str.
> >
> > Hmm... `bookmark-default-handler` is also called via things like
> > `bookmark-insert`,

Not in the code I patched or in the patched result,
it isn't.

`bookmark-insert' calls `bookmark-handle-bookmark'.
That just invokes the default handler for _some_
bookmarks.  In general it doesn't mean that any
displaying occurs, and it doesn't say what
"displaying" means for any given kind of bookmark.

> > so I think `bookmark-insert` should explicitly bind
> > `bookmark-jump-display-function` to `ignore`,

But I don't have a problem if you want to do that.

The problem you raise comes from `bookmark-insert'
trying to use a handler to insert all of the text
from the buffer that happens to be current after
the bookmark is handled.  That assumes quite a lot.

`bookmark-insert' is old (a vestige).  It seems to
assume that you're just jumping to a file (see the
doc string: "text of the file").  Or at least a
text buffer.

A more reasonable fix might be for `bookmark-insert'
to do nothing or to raise an error if the bookmark
type isn't a file bookmark.  (Or maybe a text file.
Try it on a bookmark whose jumping displays an image
file.)

> > and of course that begs the question of what to do
> > for handlers which don't obey `bookmark-jump-display-function`.

(Obey?  How about use?)  See above.  It's more than
just a question of whether/how a bookmark displays
something.  `bookmark-insert' apparently assumes a
narrow (even if common) sort of bookmark.

(BTW, do you notice `save-current-buffer' there?
And that's before giving the handler any freedom
to display.)

> > I think it's good to provide more control to the bookmark's
> > handler, but
> > there seems to be a need for the caller to control the display
> > as well to some extent.

What caller?  Caller of what - a bookmark handler?
Do you see any other callers of handlers, besides
`bookmark-insert' (which is already broken for
bookmarks that don't go to a text buffer/file)?

What kind of display control do you want a caller
of a handler to have?  And why does that seem to
you to be a need?  Can you elaborate on some
handler-calling you have in mind?

> ...which is that we now seem to have two possibly-redundant ways
> to control displaying: write a custom handler, *or* bind
> `bookmark-jump-display-function' to something?

Redundant?  A handler's responsible for any
displaying.  It can, but it need not, use the
value of `bookmark-jump-display-function' to do
that.  That's all.  

What's the question?  By non-custom handler I
guess you mean the default handler - it invokes
`bookmark-jump-display-function'.

BTW, with & without the patch there's this
"redundancy": `bookmark-bmenu-switch-other-window'
calls `bookmark--jump-via', passing a display
function.  `bookmark-bmenu-other-frame' calls
`bookmark-jump', passing a display function.

The change the patch provides is to give handlers
control over display - including choosing not to
display anything.
 
> Or perhaps I'm misunderstanding something.  Drew, if you post the
> next revision of the patch incorporating Stefan's points above, I
> will review that in detail, when fully alert, and should be able
> to distill any remaining questions -- I think we'd be pretty close
> at that point.

I posted a patch that I think does what I propose.
It's very close to the code of Bookmark+, which
has been used for quite a while now.  Feel free to
not apply the patch or to apply it and change any
doc strings.

If you replace var `bookmark-jump-display-function'
with binding a function in `bookmark--jump-via'
then I'll try to adjust my code for that.  If I
can't do so reasonably then the syncing of handler
behavior between Bookmark+ and bookmark.el might
not be so seamless.

If you have an alternative patch to propose,
please do so.  I'll try to take a look.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] : Re: [PATCH] Let bookmark handlers do everything, including display the destination
  2022-09-06  0:07       ` [External] : " Drew Adams
@ 2022-09-26 19:47         ` Karl Fogel
  0 siblings, 0 replies; 6+ messages in thread
From: Karl Fogel @ 2022-09-26 19:47 UTC (permalink / raw)
  To: Drew Adams; +Cc: Stefan Monnier, emacs-devel@gnu.org

Drew, below you quite reasonably suggest that I show what I'm 
proposing by coding it up.  I didn't want to just let your message 
dangle without a reply, so this is just to say that I'm not sure 
if or when I'll have time to do that.  While I can volunteer to 
review code in a timely manner, I'm much more cautious about 
volunteering to write code these days.

I didn't want you to think my silence meant that I disagreed with 
your original idea or your followup -- I don't, it's just a matter 
of limited hacking time.

Best regards,
-Karl

On 06 Sep 2022, Drew Adams wrote:
>> >> There's some parallelism-unsafety going on here, obviously.
>> >> I'd love a way to wrap all this in a clean closure,
>> >> instead of setting a global variable which then retains
>> >> that setting until the next time we happen to set it.
>
>Retaining that setting till it's reset could
>be a feature, not a bug. ;-)
>
>I imagine I won't have a problem if you bind
>it lexically in `bookmark--jump-via', but I'm
>not sure.
>
>Bookmark+ is designed to work also with Emacs
>versions that don't have lexical binding.
>Maybe using `lexical-let' here'll suffice,
>maybe not.
>
>A `nil' value does matter - it's not the same
>as calling a function unconditionally, even
>when the function is `ignore'.
>
>E.g., my version of `bookmark-default-handler'
>has this code, which doesn't raise the frame if
>there's no display going on (a bookmark need
>not display anything):
>
>(when bmkp-jump-display-function
>  (save-current-buffer
>    (funcall bmkp-jump-display-function (current-buffer)))
>  (raise-frame))
>
>Sure, I could change (when XXXX ...) to
>(when (eq XXXX 'ignore) ...), but that wouldn't
>really improve anything, IMO.
>
>And I test the display function in some handlers,
>for conditional handling/dispatch.  E.g., my
>handler for Dired bookmarks:
>
>(defun bmkp-jump-dired (bookmark)
>  "..."
>  (let ((dir
>         (bookmark-prop-get bookmark 'dired-directory))
>        (mfiles
>         (bookmark-prop-get bookmark 'dired-marked))
>        (switches
>         (bookmark-prop-get bookmark 'dired-switches))
>        (subdirs
>         (bookmark-prop-get bookmark 'dired-subdirs))
>        (hidden-dirs
>         (bookmark-prop-get bookmark 'dired-hidden-dirs)))
>    (case bmkp-jump-display-function
>      ((nil bmkp--pop-to-buffer-same-window display-buffer)
>       (dired dir switches))
>      ((bmkp-select-buffer-other-window
>        pop-to-buffer
>        switch-to-buffer-other-window)
>       (dired-other-window dir switches))
>      (t (dired dir switches)))
>    (let ((inhibit-read-only  t))
>      (dired-insert-old-subdirs subdirs)
>      (dired-mark-remembered 
>       (mapcar (lexical-let ((dd  dir))
>                 (lambda (mf)
>                   (cons (expand-file-name mf dd) 42)))
>               mfiles))
>      (save-excursion
>        (dolist (dir  hidden-dirs)
>          (when (dired-goto-subdir dir) (dired-hide-subdir 1)))))
>    (let ((pos  (bookmark-get-position bookmark)))
>      (when pos (goto-char pos)))))
>
>Obviously testing for functional equality isn't
>possible.  But testing a function symbol lets a
>particular kind of bookmark jump differently
>depending on current context/settings.  
>
>> > Indeed :-(
>
>Not so obvious to me.  Fine as a general guideline.
>Not clear that it's a real win (no loss) here.
>
>> > Why can't `bookmark--jump-via` let-bind
>> > `bookmark-jump-display-function` around 
>> > the call to `bookmark-handle-bookmark`?
>> 
>> Of course -- obvious... 
>> Drew, does that sound good to you?
>
>See above.  And please show your proposed code.
>
>> > Regarding the patch, here are some comments:
>> >
>> >> +  To display the destination, HANDLER can call the function
>> >> that's the
>> >> +  value of variable `bookmark-jump-display-function', which 
>> >> is
>> >> set by
>> >> +  `bookmark-jump' to automatically accommodate other-window
>> >> etc.
>> >> +  displaying that depends on the jump command.  For 
>> >> example:
>> >> +
>> >> +   (funcall bookmark-jump-display-function 
>> >> (current-buffer))
>> >> +
>> >> +  Or HANDLER can directly call another display function. 
>> >> For
>> >> example:
>> >> +
>> >> +   (switch-to-buffer-other-tab BUFFER)
>> > ...
>> > the above goes significantly beyond what one usually
>> > expects from a docstring.  It would fit much better
>> > in a Texinfo manual instead.
>
>Feel free to write a TexInfo manual for bookmarking,
>including for writing bookmark handlers, etc.  What
>exists now, and is as old as `bookmark.el', is just
>some code comments and that doc string.
>
>That doc string documents the data structure for a
>bookmark - it's THE place where that's specified.
>
>The handler part of that is important for people
>who create a new bookmark type (always has been).
>It's the one bit of bookmark data that really needs
>explaining, as it's the one part that provides
>behavior (code).
>
>> >> +  Or HANDLER can invoke `bookmark-default-handler'.  That
>> >> displays the
>> >> +  destination.  It then moves to the recorded buffer 
>> >> position,
>> >> POS,
>> >> +  repositioning point, if necessary, to match the recorded
>> >> context
>> >> +  strings STR-BEFORE-POS and STR-AFTER-POS.  For instance, 
>> >> the
>> >> Info
>> >> +  handler, `Info-bookmark-jump', does this at its end:
>> >
>> > And this is (or should be) redundant with the docstring of
>> > `bookmark-default-handler` so better keep the link and the 
>> > the
>> > readers jump to it when they need/want to know what that 
>> > function does.
>
>How is saying that a custom handler can invoke the
>default handler, and summarizing what that does,
>redundant here?
>
>Sure, it could just say that your handler can invoke
>the default handler, and leave it at that.  But that
>just begs the question "Huh? Why?".  And then, when
>you go off to look at the doc of that function, all
>you find is this (unmodified):
>
> Default handler to jump to a particular bookmark location.
> BMK-RECORD is a bookmark record, not a bookmark name (i.e., not 
> a string).
> Changes current buffer and point and returns nil, or signals a 
> `file-error'.
>
> If BMK-RECORD has a property called `buffer', it should be a 
> live
> buffer object, and this buffer will be selected.
>
>(Why shouldn't it be able to be a bookmark name, BTW?)
>
>Does that doc string help you, if you're writing
>your own handler?  The code helps, but not that
>doc string.  It doesn't tell you what jumping
>_means/does_ in the default case.
>
>IMO there's little sense in saying only that your
>handler _can_ call the default handler.  And even
>less sense in saying neither that it can nor what
>calling it does, i.e., _why_ you might want your
>handler to invoke the default handler.
>
>But TexInfo manual - welcome.  Go for it, please.
>
>> Agreed.
>
>Please show your proposed code (e.g. doc string).
>
>It's a deficiency of the existing doc string
>that it doesn't really tell you anything useful
>about the HANDLER part, IMO.
>
>But as I said, feel free to put what you like in
>any of the doc strings.
>
>The patch doesn't _require_ any mention of the
>possibility or use case of calling the default
>handler from a nondefault handler.  I think it's
>good to mention that, but you needn't do so to
>make the code change.
>
>> >> +(defvar bookmark-jump-display-function nil
>> >> + "Function used currently to display a bookmark, or nil if 
>> >> no
>> >> function.")
>> >
>> > Please give it a function as default value (e.g. `ignore`).
>
>See above.  I don't think anything's really gained
>by doing that here.  As a general guideline, sure,
>no problem.  But let's hear a specific argument
>for doing that here, please.
>
>> I think I understand: by taking your advice, we don't have to
>> check the function's existence -- we can just call it
>> unconditionally, and if it happens to do nothing (i.e., is
>> `ignore'), that's fine.
>
>Not so fine, IMO.  And you might still need to test
>its existence - with `(eq 'ignore)' instead of just
>non-nil.  And then there's trying to test for
>functions "equivalent" to `ignore'...
>
>[FWIW, a somewhat related feature, which I didn't
>include in my patch (but which could be considered
>for another change), wraps most of the code of
>`bookmark--jump-via' in a `catch', to which a
>handler can `throw' to avoid doing anything else
>(e.g. after-jump hook, auto-showing annotations).]
>
>> A specific package may still override the
>> default handler and do whatever display magic is called for;
>> otherwise, bookmark.el's default handler will Do The Right 
>> Thing
>> for displaying.
>
>If "otherwise" means that if a handler doesn't
>use a display function then the display of the
>default handler is used, then that's wrong.  A
>handler needs to be able to not display anything.
>
>> But all this raises the question you raise below...
>
>What question below?  Do you mean the question
>"Why `save-current-buffer'?"
>
>> >> @@ -1350,6 +1391,9 @@
>> >>        ((and buf (get-buffer buf)))
>> >>        (t ;; If not, raise error.
>> >>         (signal 'bookmark-error-no-filename (list 'stringp
>> >>         file)))))
>> >> +    (when bookmark-jump-display-function
>> >> +      (save-current-buffer
>> >> +        (funcall bookmark-jump-display-function
>> >> +                 (current-buffer))))
>> >
>> > Why `save-current-buffer`?
>
>That's already in the code that the patch
>replaces.  It's in `bookmark--jump-via'.
>Displaying is now in the default handler (or
>another handler).  That's all.
>
>Take out that ` save-current-buffer', if you
>think you don't need it.  I'll likely leave it
>in my code.
>
>You can fiddle with the `bookmark--jump-via'
>code if you like, to ensure that the behavior
>change I described is the only one that's made.
>I think that's already the case, but feel free.
>
>The code right after `(save-current-buffer...)'
>expects the buffer that is current not to have
>changed.  In the unpatched code that right-after
>code is in `bookmark--jump-via'.  In the patched
>code it's in the default handler.
>
>> >>      (if place (goto-char place))
>> >>      ;; Go searching forward first.  Then, if forward-str
>> >>      exists and
>> >>      ;; was found in the file, we can search backward for
>> >>      behind-str.
>> >
>> > Hmm... `bookmark-default-handler` is also called via things 
>> > like
>> > `bookmark-insert`,
>
>Not in the code I patched or in the patched result,
>it isn't.
>
>`bookmark-insert' calls `bookmark-handle-bookmark'.
>That just invokes the default handler for _some_
>bookmarks.  In general it doesn't mean that any
>displaying occurs, and it doesn't say what
>"displaying" means for any given kind of bookmark.
>
>> > so I think `bookmark-insert` should explicitly bind
>> > `bookmark-jump-display-function` to `ignore`,
>
>But I don't have a problem if you want to do that.
>
>The problem you raise comes from `bookmark-insert'
>trying to use a handler to insert all of the text
>from the buffer that happens to be current after
>the bookmark is handled.  That assumes quite a lot.
>
>`bookmark-insert' is old (a vestige).  It seems to
>assume that you're just jumping to a file (see the
>doc string: "text of the file").  Or at least a
>text buffer.
>
>A more reasonable fix might be for `bookmark-insert'
>to do nothing or to raise an error if the bookmark
>type isn't a file bookmark.  (Or maybe a text file.
>Try it on a bookmark whose jumping displays an image
>file.)
>
>> > and of course that begs the question of what to do
>> > for handlers which don't obey 
>> > `bookmark-jump-display-function`.
>
>(Obey?  How about use?)  See above.  It's more than
>just a question of whether/how a bookmark displays
>something.  `bookmark-insert' apparently assumes a
>narrow (even if common) sort of bookmark.
>
>(BTW, do you notice `save-current-buffer' there?
>And that's before giving the handler any freedom
>to display.)
>
>> > I think it's good to provide more control to the bookmark's
>> > handler, but
>> > there seems to be a need for the caller to control the 
>> > display
>> > as well to some extent.
>
>What caller?  Caller of what - a bookmark handler?
>Do you see any other callers of handlers, besides
>`bookmark-insert' (which is already broken for
>bookmarks that don't go to a text buffer/file)?
>
>What kind of display control do you want a caller
>of a handler to have?  And why does that seem to
>you to be a need?  Can you elaborate on some
>handler-calling you have in mind?
>
>> ...which is that we now seem to have two possibly-redundant 
>> ways
>> to control displaying: write a custom handler, *or* bind
>> `bookmark-jump-display-function' to something?
>
>Redundant?  A handler's responsible for any
>displaying.  It can, but it need not, use the
>value of `bookmark-jump-display-function' to do
>that.  That's all.  
>
>What's the question?  By non-custom handler I
>guess you mean the default handler - it invokes
>`bookmark-jump-display-function'.
>
>BTW, with & without the patch there's this
>"redundancy": `bookmark-bmenu-switch-other-window'
>calls `bookmark--jump-via', passing a display
>function.  `bookmark-bmenu-other-frame' calls
>`bookmark-jump', passing a display function.
>
>The change the patch provides is to give handlers
>control over display - including choosing not to
>display anything.
> 
>> Or perhaps I'm misunderstanding something.  Drew, if you post 
>> the
>> next revision of the patch incorporating Stefan's points above, 
>> I
>> will review that in detail, when fully alert, and should be 
>> able
>> to distill any remaining questions -- I think we'd be pretty 
>> close
>> at that point.
>
>I posted a patch that I think does what I propose.
>It's very close to the code of Bookmark+, which
>has been used for quite a while now.  Feel free to
>not apply the patch or to apply it and change any
>doc strings.
>
>If you replace var `bookmark-jump-display-function'
>with binding a function in `bookmark--jump-via'
>then I'll try to adjust my code for that.  If I
>can't do so reasonably then the syncing of handler
>behavior between Bookmark+ and bookmark.el might
>not be so seamless.
>
>If you have an alternative patch to propose,
>please do so.  I'll try to take a look.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-26 19:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-13 20:59 [PATCH] Let bookmark handlers do everything, including display the destination Drew Adams
2022-08-18 18:14 ` Karl Fogel
2022-08-29 15:28   ` Stefan Monnier
2022-08-29 23:45     ` Karl Fogel
2022-09-06  0:07       ` [External] : " Drew Adams
2022-09-26 19:47         ` Karl Fogel

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).