]> code.delx.au - gnu-emacs/blobdiff - lisp/bookmark.el
* keymap.c (Fmake_sparse_keymap): Purecopy the name.
[gnu-emacs] / lisp / bookmark.el
index 535b1f2516a4ea15c50d5bd919fb335201ffe989..331e392a9c701361fc6bc54b2a3abc3d31720cad 100644 (file)
@@ -1,7 +1,7 @@
 ;;; bookmark.el --- set bookmarks, maybe annotate them, jump to them later
 
 ;; Copyright (C) 1993, 1994, 1995, 1996, 1997, 2001, 2002, 2003,
-;;   2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+;;   2004, 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
 
 ;; Author: Karl Fogel <kfogel@red-bean.com>
 ;; Maintainer: Karl Fogel <kfogel@red-bean.com>
@@ -79,6 +79,7 @@
 ;;; Code:
 
 (require 'pp)
+(eval-when-compile (require 'cl))
 
 ;;; Misc comments:
 ;;
@@ -125,7 +126,7 @@ To specify the file in which to save them, modify the variable
 
 
 (defconst bookmark-old-default-file "~/.emacs-bkmrks"
-  "*The `.emacs.bmk' file used to be called this name.")
+  "The `.emacs.bmk' file used to be called this name.")
 
 
 ;; defvarred to avoid a compilation warning:
@@ -269,10 +270,16 @@ the new favored one.  PARAM-ALIST is typically of the form:
   (front-context-string . FRONT-STR)
   (rear-context-string  . REAR-STR)
   (position . POS)
-  (annotation . ANNOTATION)))")
+  (handler . HANDLER-FUNC)
+  (annotation . ANNOTATION))
 
+If the element `(handler . HANDLER-FUNC)' is present, HANDLER-FUNC
+will be used to open this bookmark instead of `bookmark-default-handler',
+whose calling discipline HANDLER-FUNC should of course match.")
 
-(defvar bookmarks-already-loaded nil)
+
+(defvar bookmarks-already-loaded nil
+  "Non-nil iff bookmarks have been loaded from `bookmark-default-file'.")
 
 
 ;; more stuff added by db.
@@ -293,11 +300,18 @@ through a file easier.")
   "Length of the context strings recorded on either side of a bookmark.")
 
 
-(defvar bookmark-current-point 0)
-(defvar bookmark-yank-point 0)
-(defvar bookmark-current-buffer nil)
+(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' and
+`bookmark-insert-current-bookmark' 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-curent-buffer'.")
+
 
-(defvar Info-suffix-list)
 \f
 ;; Helper functions.
 
@@ -317,19 +331,22 @@ through a file easier.")
   (mapcar 'bookmark-name-from-full-record bookmark-alist))
 
 
-(defun bookmark-get-bookmark (bookmark)
+(defun bookmark-get-bookmark (bookmark &optional noerror)
   "Return the bookmark record corresponding to BOOKMARK.
-If BOOKMARK is already a bookmark record, just return it,
-Otherwise look for the corresponding bookmark in `bookmark-alist'."
+If BOOKMARK is a string, look for the corresponding bookmark record in
+`bookmark-alist'; return it if found, otherwise error.  Else if
+BOOKMARK is already a bookmark record, just return it."
   (cond
    ((consp bookmark) bookmark)
    ((stringp bookmark)
-    (assoc-string bookmark bookmark-alist bookmark-completion-ignore-case))))
+    (or (assoc-string bookmark bookmark-alist bookmark-completion-ignore-case)
+        (unless noerror (error "Invalid bookmark %s" bookmark))))))
 
 
 (defun bookmark-get-bookmark-record (bookmark)
-  "Return the guts of the entry for BOOKMARK in `bookmark-alist'.
-That is, all information but the name."
+  "Return the record portion of the entry for BOOKMARK in
+`bookmark-alist' (that is, all information but the name).
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (let ((alist (cdr (bookmark-get-bookmark bookmark))))
     ;; The bookmark objects can either look like (NAME ALIST) or
     ;; (NAME . ALIST), so we have to distinguish the two here.
@@ -338,17 +355,20 @@ That is, all information but the name."
 
 
 (defun bookmark-set-name (bookmark newname)
-  "Set BOOKMARK's name to NEWNAME."
+  "Set BOOKMARK's name to NEWNAME.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (setcar
    (if (stringp bookmark) (bookmark-get-bookmark bookmark) bookmark)
    newname))
 
 (defun bookmark-prop-get (bookmark prop)
-  "Return the property PROP of BOOKMARK, or nil if none."
+  "Return the property PROP of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (cdr (assq prop (bookmark-get-bookmark-record bookmark))))
 
 (defun bookmark-prop-set (bookmark prop val)
-  "Set the property PROP of BOOKMARK to VAL."
+  "Set the property PROP of BOOKMARK to VAL.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (let ((cell (assq prop (bookmark-get-bookmark-record bookmark))))
     (if cell
         (setcdr cell val)
@@ -356,55 +376,67 @@ That is, all information but the name."
              (list (cons prop val))))))
 
 (defun bookmark-get-annotation (bookmark)
-  "Return the annotation of BOOKMARK, or nil if none."
+  "Return the annotation of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'annotation))
 
 (defun bookmark-set-annotation (bookmark ann)
-  "Set the annotation of BOOKMARK to ANN."
+  "Set the annotation of BOOKMARK to ANN.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-set bookmark 'annotation ann))
 
 
 (defun bookmark-get-filename (bookmark)
-  "Return the full filename of BOOKMARK."
+  "Return the full filename of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'filename))
 
 
 (defun bookmark-set-filename (bookmark filename)
-  "Set the full filename of BOOKMARK to FILENAME."
+  "Set the full filename of BOOKMARK to FILENAME.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-set bookmark 'filename filename))
 
 
 (defun bookmark-get-position (bookmark)
-  "Return the position \(i.e.: point\) of BOOKMARK."
+  "Return the position \(i.e.: point\) of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'position))
 
 
 (defun bookmark-set-position (bookmark position)
-  "Set the position \(i.e.: point\) of BOOKMARK to POSITION."
+  "Set the position \(i.e.: point\) of BOOKMARK to POSITION.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-set bookmark 'position position))
 
 
 (defun bookmark-get-front-context-string (bookmark)
-  "Return the front-context-string of BOOKMARK."
+  "Return the front-context-string of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'front-context-string))
 
 
 (defun bookmark-set-front-context-string (bookmark string)
-  "Set the front-context-string of BOOKMARK to STRING."
+  "Set the front-context-string of BOOKMARK to STRING.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-set bookmark 'front-context-string string))
 
 
 (defun bookmark-get-rear-context-string (bookmark)
-  "Return the rear-context-string of BOOKMARK."
+  "Return the rear-context-string of BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'rear-context-string))
 
 
 (defun bookmark-set-rear-context-string (bookmark string)
-  "Set the rear-context-string of BOOKMARK to STRING."
+  "Set the rear-context-string of BOOKMARK to STRING.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-set bookmark 'rear-context-string string))
 
 
 (defun bookmark-get-handler (bookmark)
+  "Return the handler function for BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-prop-get bookmark 'handler))
 
 (defvar bookmark-history nil
@@ -440,7 +472,7 @@ the empty string."
 We need this because sometimes bookmark functions are invoked from
 menus, so `completing-read' never gets a chance to set `bookmark-history'."
   `(or
-    (interactive-p)
+    (called-interactively-p 'interactive)
     (setq bookmark-history (cons ,string bookmark-history))))
 
 (defvar bookmark-make-record-function 'bookmark-make-record-default
@@ -455,10 +487,9 @@ 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 . SOME-FUNCTION) which sets the handler function
-that should be used to open this bookmark instead of
-`bookmark-default-handler'.  The handler should follow the same calling
-convention as the one used by `bookmark-default-handler'.
+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
@@ -486,7 +517,8 @@ old one."
         ;; XEmacs's `set-text-properties' doesn't work on
         ;; free-standing strings, apparently.
         (set-text-properties 0 (length stripped-name) nil stripped-name))
-    (if (and (bookmark-get-bookmark stripped-name) (not no-overwrite))
+    (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
         ;; Use the new (NAME . ALIST) format.
@@ -708,27 +740,29 @@ This expects to be called from `point-min' in a bookmark file."
     map))
 
 ;;;###autoload
-(defun bookmark-set (&optional name parg)
-  "Set a bookmark named NAME inside a file.
-If name is nil, then the user will be prompted.
-With prefix arg, will not overwrite a bookmark that has the same name
-as NAME if such a bookmark already exists, but instead will \"push\"
-the new bookmark onto the bookmark alist.  Thus the most recently set
-bookmark with name NAME would be the one in effect at any given time,
-but the others are still there, should you decide to delete the most
-recent one.
+(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 prefix arg (NO-OVERWRITE), do not overwrite a bookmark that
+has the same name as NAME if such a bookmark already exists, but
+instead push the new bookmark onto the bookmark alist.  Thus the
+most recently set bookmark with name NAME would be 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 C-w while setting a bookmark.  Successive C-w's
 yank successive words.
 
-Typing C-u inserts the name of the last bookmark used in the buffer
-\(as an aid in using a single bookmark name to track your progress
-through a large file\).  If no bookmark was used, then C-u inserts the
-name of the file being visited.
+Typing C-u will insert (at the bookmark name prompt) the name of the
+last bookmark used in the document where the new bookmark is being set;
+this helps one use a single bookmark name to track progress through
+a large document.  If there is no prior bookmark for this document,
+then C-u 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
+Use \\[bookmark-delete] to remove bookmarks \(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* ((record (bookmark-make-record))
@@ -736,7 +770,6 @@ the list of bookmarks.\)"
 
     (bookmark-maybe-load-default-file)
 
-    (setq bookmark-current-point (point))
     (setq bookmark-yank-point (point))
     (setq bookmark-current-buffer (current-buffer))
 
@@ -748,12 +781,11 @@ the list of bookmarks.\)"
                 bookmark-minibuffer-read-name-map
                 nil nil default))))
       (and (string-equal str "") (setq str default))
-      (bookmark-store str (cdr record) parg)
-      
+      (bookmark-store str (cdr record) no-overwrite)
+
       ;; Ask for an annotation buffer for this bookmark
-      (if bookmark-use-annotations
-          (bookmark-edit-annotation str)
-        (goto-char bookmark-current-point)))))
+      (when bookmark-use-annotations
+        (bookmark-edit-annotation str)))))
 
 (defun bookmark-kill-line (&optional newline-too)
   "Kill from point to end of line.
@@ -773,6 +805,9 @@ whose annotation is being edited.")
 
 
 (defun bookmark-default-annotation-text (bookmark)
+  "Return default annotation text for BOOKMARK (a string, not a record).
+The default annotation text is simply some text explaining how to use
+annotations."
   (concat "#  Type the annotation for bookmark '" bookmark "' here.\n"
          "#  All lines which start with a '#' will be deleted.\n"
          "#  Type C-c C-c when done.\n#\n"
@@ -799,6 +834,8 @@ It takes one argument, the name of the bookmark, as a string.")
   "Mode for editing the annotation of bookmark BOOKMARK.
 When you have finished composing, type \\[bookmark-send-annotation].
 
+BOOKMARK is a bookmark name (a string) or a bookmark record.
+
 \\{bookmark-edit-annotation-mode-map}"
   (interactive)
   (kill-all-local-variables)
@@ -829,20 +866,21 @@ Lines beginning with `#' are ignored."
   (let ((annotation (buffer-substring-no-properties (point-min) (point-max)))
        (bookmark bookmark-annotation-name))
     (bookmark-set-annotation bookmark annotation)
-    (bookmark-bmenu-surreptitiously-rebuild-list)
-    (goto-char bookmark-current-point))
+    (bookmark-bmenu-surreptitiously-rebuild-list))
   (kill-buffer (current-buffer)))
 
 
 (defun bookmark-edit-annotation (bookmark)
-  "Pop up a buffer for editing bookmark BOOKMARK's annotation."
+  "Pop up a buffer for editing bookmark BOOKMARK's annotation.
+BOOKMARK is a bookmark name (a string) or a bookmark record."
   (pop-to-buffer (generate-new-buffer-name "*Bookmark Annotation Compose*"))
   (bookmark-edit-annotation-mode bookmark))
 
 
 (defun bookmark-insert-current-bookmark ()
-  "Insert this buffer's value of `bookmark-current-bookmark'.
-Default to file name if it's nil."
+  "Insert into the bookmark name currently being set the value of
+`bookmark-current-bookmark' in `bookmark-current-buffer', defaulting
+to the buffer's file name if `bookmark-current-bookmark' is nil."
   (interactive)
   (let ((str
         (with-current-buffer bookmark-current-buffer
@@ -852,7 +890,8 @@ Default to file name if it's nil."
 
 
 (defun bookmark-buffer-name ()
-  "Return the name of the current buffer's file, non-directory."
+  "Return the name of the current buffer (or its file, if any) in a
+way that is suitable as a bookmark name."
   (cond
    ;; Or are we a file?
    (buffer-file-name (file-name-nondirectory buffer-file-name))
@@ -873,9 +912,10 @@ Default to file name if it's nil."
 
 
 (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)
-  ;; get the next word from the buffer and append it to the name of
-  ;; the bookmark currently being set.
   (let ((string (with-current-buffer bookmark-current-buffer
                   (goto-char bookmark-yank-point)
                   (buffer-substring-no-properties
@@ -887,20 +927,21 @@ Default to file name if it's nil."
 
 (defun bookmark-buffer-file-name ()
   "Return the current buffer's file in a way useful for bookmarks."
-  (cond
-   (buffer-file-name
-    ;; 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 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"))))
+  ;; 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")))))
 
 
 (defun bookmark-maybe-load-default-file ()
+  "If bookmarks have not been loaded from the default place, load them."
   (and (not bookmarks-already-loaded)
        (null bookmark-alist)
        (prog2
@@ -920,8 +961,8 @@ Default to file name if it's nil."
 
 
 (defun bookmark-maybe-sort-alist ()
-  ;;Return the bookmark-alist for display.  If the bookmark-sort-flag
-  ;;is non-nil, then return a sorted copy of the alist.
+  "Return `bookmark-alist' for display.
+If `bookmark-sort-flag' is non-nil, then return a sorted copy of the alist."
   (if bookmark-sort-flag
       (sort (copy-alist bookmark-alist)
             (function
@@ -934,7 +975,13 @@ Default to file name if it's nil."
 Useful for example to unhide text in `outline-mode'.")
 
 (defun bookmark--jump-via (bookmark display-function)
-  (bookmark-jump-noselect bookmark)
+  "Handle BOOKMARK, then call DISPLAY-FUNCTION with current buffer as argument.
+Bookmark may be a bookmark name (a string) or a bookmark record.
+
+After calling DISPLAY-FUNCTION, set window point to the point specified
+by BOOKMARK, if necessary, run `bookmark-after-jump-hook', and then show
+any annotations for this bookmark."
+  (bookmark-handle-bookmark bookmark)
   (save-current-buffer
     (funcall display-function (current-buffer)))
   (let ((win (get-buffer-window (current-buffer) 0)))
@@ -946,10 +993,10 @@ Useful for example to unhide text in `outline-mode'.")
       ;; if there is an annotation for this bookmark,
       ;; show it in a buffer.
       (bookmark-show-annotation bookmark)))
-  
+
 
 ;;;###autoload
-(defun bookmark-jump (bookmark)
+(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
@@ -959,55 +1006,55 @@ 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."
+of the old one in the permanent bookmark record.
+
+BOOKMARK may be a bookmark name (a string) or a bookmark record, but
+the latter is usually only used by programmatic callers.
+
+If DISPLAY-FUNC is non-nil, it is a function to invoke to display the
+bookmark.  It defaults to `switch-to-buffer'; a typical other value
+would be, e.g., `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)
-  (bookmark--jump-via bookmark 'switch-to-buffer))
+  (bookmark--jump-via bookmark (or display-func 'switch-to-buffer)))
 
 
 ;;;###autoload
 (defun bookmark-jump-other-window (bookmark)
-  "Jump to BOOKMARK (a point in some file) in another window.
-See `bookmark-jump'."
+  "Jump to BOOKMARK in another window.  See `bookmark-jump' for more."
   (interactive
-   (let ((bkm (bookmark-completing-read "Jump to bookmark (in another window)"
-                                        bookmark-current-bookmark)))
-     (if (> emacs-major-version 21)
-         (list bkm) bkm)))
-  (when bookmark
-    (bookmark-maybe-historicize-string bookmark)
-    (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))
-
-
-(defun bookmark-file-or-variation-thereof (file)
-  "Return FILE (a string) if it exists, or return a reasonable
-variation of FILE if that exists.  Reasonable variations are checked
-by appending suffixes defined in `Info-suffix-list'.  If cannot find FILE
-nor a reasonable variation thereof, then still return FILE if it can
-be retrieved from a VC backend, else return nil."
-  (if (file-exists-p file)
-      file
-    (or
-     (progn (require 'info)  ; ensure Info-suffix-list is bound
-            (catch 'found
-              (mapc (lambda (elt)
-                      (let ((suffixed-file (concat file (car elt))))
-                        (if (file-exists-p suffixed-file)
-                            (throw 'found suffixed-file))))
-                    Info-suffix-list)
-              nil))
-     ;; Last possibility: try VC
-     (if (vc-backend file) file))))
+   (list (bookmark-completing-read "Jump to bookmark (in another window)"
+                                   bookmark-current-bookmark)))
+  (bookmark-jump bookmark 'switch-to-buffer-other-window))
+
 
 (defun bookmark-jump-noselect (bookmark)
+  "Return the location pointed to by the bookmark BOOKMARK.
+The return value has the form (BUFFER . POINT).
+
+BOOKMARK may be a bookmark name (a string) or a bookmark record.
+
+Note: this function is deprecated and is present for Emacs 22
+compatibility only."
+  (save-excursion
+    (bookmark-handle-bookmark bookmark)
+    (cons (current-buffer) (point))))
+
+(make-obsolete 'bookmark-jump-noselect 'bookmark-handle-bookmark "23.1")
+
+(defun bookmark-handle-bookmark (bookmark)
   "Call BOOKMARK's handler or `bookmark-default-handler' if it has none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record.
+
 Changes current buffer and point and returns nil, or signals a `file-error'.
-BOOKMARK can be a bookmark record used internally by some other
-elisp package, or the name of a bookmark to be found in `bookmark-alist'."
+
+If BOOKMARK has no file, this is a no-op.  If BOOKMARK has a file, but
+that file no longer exists, then offer interactively to relocate BOOKMARK.
+"
   (condition-case err
       (funcall (or (bookmark-get-handler bookmark)
                    'bookmark-default-handler)
@@ -1016,51 +1063,57 @@ elisp package, or the name of a bookmark to be found in `bookmark-alist'."
      ;; 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)
-       ;; `bookmark' can either be a bookmark name (found in
-       ;; `bookmark-alist') or a bookmark object.  If it's an object, we
-       ;; assume it's a bookmark used internally by some other package.
+       ;; `bookmark' 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)))
          (when file        ;Don't know how to relocate if there's no `file'.
-           (setq file (expand-file-name file))
-           (ding)
-           (if (y-or-n-p (concat (file-name-nondirectory file)
-                                 " nonexistent.  Relocate \""
-                                 bookmark
-                                 "\"? "))
-               (progn
-                 (bookmark-relocate bookmark)
-                 ;; Try again.
-                 (funcall (or (bookmark-get-handler bookmark)
-                              'bookmark-default-handler)
-                          (bookmark-get-bookmark bookmark)))
-             (message
-              "Bookmark not relocated; consider removing it \(%s\)." bookmark)
-             (signal (car err) (cdr err))))))))
+           ;; 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 "\"? "))
+                   (progn
+                     (bookmark-relocate bookmark)
+                     ;; Try again.
+                     (funcall (or (bookmark-get-handler bookmark)
+                                  'bookmark-default-handler)
+                              (bookmark-get-bookmark bookmark)))
+                 (message
+                  "Bookmark not relocated; consider removing it \(%s\)."
+                  bookmark)
+                 (signal (car err) (cdr err))))))))))
   ;; Added by db.
   (when (stringp bookmark)
     (setq bookmark-current-bookmark bookmark))
   nil)
 
-(defun bookmark-default-handler (bmk)
+(put 'bookmark-error-no-filename
+     'error-conditions
+     '(error bookmark-errors bookmark-error-no-filename))
+(put 'bookmark-error-no-filename
+     'error-message
+     "Bookmark has no associated file (or directory)")
+
+(defun bookmark-default-handler (bmk-record)
   "Default handler to jump to a particular bookmark location.
-BMK is a bookmark record.
+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'."
-  (let* ((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)))
-    ;; FIXME: bookmark-file-or-variation-thereof was needed for Info files,
-    ;; but now that Info bookmarks are handled elsewhere it seems that we
-    ;; should be able to get rid of it.  --Stef
-    (if (not (if buf (buffer-live-p buf)
-               (setq file (bookmark-file-or-variation-thereof file))))
-        (signal 'file-error
-                `("Jumping to bookmark" "No such file or directory"
-                  (bookmark-get-filename bmk)))
-      (set-buffer (or buf (find-file-noselect file)))
+  (let ((file          (bookmark-get-filename bmk-record))
+        (forward-str   (bookmark-get-front-context-string bmk-record))
+        (behind-str    (bookmark-get-rear-context-string bmk-record))
+        (place         (bookmark-get-position bmk-record)))
+    (if (not file)
+        (signal 'bookmark-error-no-filename (list 'stringp file))
+      (set-buffer (find-file-noselect 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
@@ -1077,6 +1130,8 @@ Changes current buffer and point and returns nil, or signals a `file-error'."
 ;;;###autoload
 (defun bookmark-relocate (bookmark)
   "Relocate BOOKMARK to another file (reading file name with minibuffer).
+BOOKMARK is a bookmark name (a string), not a bookmark record.
+
 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."
@@ -1099,6 +1154,8 @@ after a bookmark was set in it."
 ;;;###autoload
 (defun bookmark-insert-location (bookmark &optional no-history)
   "Insert the name of the file associated with BOOKMARK.
+BOOKMARK is a bookmark name (a string), not a bookmark record.
+
 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")))
@@ -1120,7 +1177,8 @@ minibuffer history list `bookmark-history'."
 (defalias 'bookmark-locate 'bookmark-insert-location)
 
 (defun bookmark-location (bookmark)
-  "Return the name of the file associated with BOOKMARK."
+  "Return the name of the file associated with BOOKMARK, or nil if none.
+BOOKMARK may be a bookmark name (a string) or a bookmark record."
   (bookmark-maybe-load-default-file)
   (bookmark-get-filename bookmark))
 
@@ -1131,6 +1189,8 @@ minibuffer history list `bookmark-history'."
 If called from keyboard, prompt for OLD and NEW.  If called from
 menubar, select OLD from a menu and prompt for NEW.
 
+Both OLD and NEW are bookmark names (strings), never bookmark records.
+
 If called from Lisp, prompt for NEW if only OLD was passed as an
 argument.  If called with two strings, then no prompting is done.  You
 must pass at least OLD when calling from Lisp.
@@ -1142,7 +1202,6 @@ name."
   (bookmark-maybe-historicize-string old)
   (bookmark-maybe-load-default-file)
 
-  (setq bookmark-current-point (point))
   (setq bookmark-yank-point (point))
   (setq bookmark-current-buffer (current-buffer))
   (let ((newname
@@ -1167,6 +1226,8 @@ name."
 ;;;###autoload
 (defun bookmark-insert (bookmark)
   "Insert the text of the file pointed to by bookmark BOOKMARK.
+BOOKMARK 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
@@ -1177,7 +1238,7 @@ this."
   (let ((orig-point (point))
        (str-to-insert
         (save-current-buffer
-           (bookmark-jump-noselect bookmark)
+           (bookmark-handle-bookmark bookmark)
           (buffer-string))))
     (insert str-to-insert)
     (push-mark)
@@ -1187,6 +1248,8 @@ this."
 ;;;###autoload
 (defun bookmark-delete (bookmark &optional batch)
   "Delete BOOKMARK from the bookmark list.
+BOOKMARK is a bookmark name (a string), not a bookmark record.
+
 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
@@ -1198,30 +1261,27 @@ probably because we were called from there."
                                   bookmark-current-bookmark)))
   (bookmark-maybe-historicize-string bookmark)
   (bookmark-maybe-load-default-file)
-  (let ((will-go (bookmark-get-bookmark bookmark)))
+  (let ((will-go (bookmark-get-bookmark bookmark 'noerror)))
     (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)
+    (or (bookmark-get-bookmark bookmark-current-bookmark 'noerror)
         (setq bookmark-current-bookmark nil)))
-  ;; Don't rebuild the list
-  (if batch
-      nil
-    (bookmark-bmenu-surreptitiously-rebuild-list)
-    (setq bookmark-alist-modification-count
-          (1+ bookmark-alist-modification-count))
-    (if (bookmark-time-to-save-p)
-        (bookmark-save))))
-
-
-(defun bookmark-time-to-save-p (&optional last-time)
-  ;; By Gregory M. Saunders <saunders@cis.ohio-state.edu>
-  ;; finds out whether it's time to save bookmarks to a file, by
-  ;; examining the value of variable bookmark-save-flag, and maybe
-  ;; bookmark-alist-modification-count.  Returns t if they should be
-  ;; saved, nil otherwise.  if last-time is non-nil, then this is
-  ;; being called when emacs is killed.
-  (cond (last-time
+  (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)))
+
+
+(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)
@@ -1279,6 +1339,7 @@ for a file, defaulting to the file defined by variable
 
 \f
 (defun bookmark-write-file (file)
+  "Write `bookmark-alist' to FILE."
   (bookmark-maybe-message "Saving bookmarks to file %s..." file)
   (with-current-buffer (get-buffer-create " *Bookmarks*")
     (goto-char (point-min))
@@ -1286,7 +1347,12 @@ for a file, defaulting to the file defined by variable
     (let ((print-length nil)
           (print-level nil))
       (bookmark-insert-file-format-version-stamp)
-      (pp bookmark-alist (current-buffer))
+      (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 ")")
       (let ((version-control
              (cond
               ((null bookmark-version-control) nil)
@@ -1302,10 +1368,8 @@ for a file, defaulting to the file defined by variable
 
 
 (defun bookmark-import-new-list (new-list)
-  ;; Walk over the new list, adding each individual bookmark
-  ;; carefully.  "Carefully" means checking against the existing
-  ;; bookmark-alist and renaming the new bookmarks with <N> extensions
-  ;; as necessary.
+  "Add NEW-LIST of bookmarks to `bookmark-alist', rename new bookmarks
+with \"<N>\" extensions where they collide with existing bookmark names."
   (let ((lst new-list)
         (names (bookmark-all-names)))
     (while lst
@@ -1317,11 +1381,8 @@ for a file, defaulting to the file defined by variable
 
 
 (defun bookmark-maybe-rename (full-record names)
-  ;; just a helper for bookmark-import-new-list; it is only for
-  ;; readability that this is not inlined.
-  ;;
-  ;; Once this has found a free name, it sets full-record to that
-  ;; name.
+  "If bookmark record FULL-RECORD collides with anything in NAMES, give
+FULL-RECORD a new name.  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
@@ -1474,7 +1535,7 @@ 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)
-  (if (interactive-p)
+  (if (called-interactively-p 'interactive)
       (switch-to-buffer (get-buffer-create "*Bookmark List*"))
     (set-buffer (get-buffer-create "*Bookmark List*")))
   (let ((inhibit-read-only t))
@@ -1578,6 +1639,10 @@ Optional argument SHOW means show them unconditionally."
 
 
 (defun bookmark-bmenu-show-filenames (&optional force)
+  "In an interactive bookmark list, show filenames along with bookmarks.
+
+If FORCE is non-nil, force a redisplay showing the filenames; this is
+used mainly for debugging, and should not be necessary in normal usage."
   (if (and (not force) bookmark-bmenu-toggle-filenames)
       nil ;already shown, so do nothing
     (save-excursion
@@ -1604,6 +1669,10 @@ Optional argument SHOW means show them unconditionally."
 
 
 (defun bookmark-bmenu-hide-filenames (&optional force)
+  "In an interactive bookmark list, hide the filenames of the bookmarks.
+
+If FORCE is non-nil, force a redisplay hiding the filenames; this is
+used mainly for debugging, and should not be necessary in normal usage."
   (if (and (not force) bookmark-bmenu-toggle-filenames)
       ;; nothing to hide if above is nil
       (save-excursion
@@ -1640,9 +1709,12 @@ Optional argument SHOW means show them unconditionally."
 
 
 (defun bookmark-bmenu-check-position ()
-  ;; Returns non-nil if on a line with a bookmark.
-  ;; (The actual value returned is bookmark-alist).
-  ;; Else reposition and try again, else return nil.
+  "Return non-nil if on a line with a bookmark (the actual value
+returned is `bookmark-alist').  Else reposition and try again; else if
+still no bookmark, return nil."
+  ;; FIXME: I don't believe this doc string.  As far as I can tell,
+  ;; this function always just returns bookmark-alist.  So what is
+  ;; it for, really?  -kfogel, 2009-10-04
   (cond ((< (count-lines (point-min) (point)) 2)
          (goto-char (point-min))
          (forward-line 2)
@@ -1655,6 +1727,7 @@ Optional argument SHOW means show them unconditionally."
 
 
 (defun bookmark-bmenu-bookmark ()
+  "Return the bookmark for this line in an interactive bookmark list buffer."
   ;; return a string which is bookmark of this line.
   (if (bookmark-bmenu-check-position)
       (save-excursion
@@ -1949,10 +2022,6 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
           (forward-char o-col))
       (goto-char o-point))
     (beginning-of-line)
-    (setq bookmark-alist-modification-count
-          (1+ bookmark-alist-modification-count))
-    (if (bookmark-time-to-save-p)
-        (bookmark-save))
     (message "Deleting bookmarks...done")
     ))
 
@@ -1964,7 +2033,6 @@ To carry out the deletions that you've marked, use \\<bookmark-bmenu-mode-map>\\
       (let ((bmrk (bookmark-bmenu-bookmark))
             (thispoint (point)))
         (bookmark-rename bmrk)
-        (bookmark-bmenu-list)
         (goto-char thispoint))))
 
 
@@ -2033,16 +2101,36 @@ strings returned are not."
 ;;;###autoload
 (defvar menu-bar-bookmark-map
   (let ((map (make-sparse-keymap "Bookmark functions")))
-    (define-key map [load]     '("Load a Bookmark File..." . bookmark-load))
-    (define-key map [write]    '("Save Bookmarks As..." . bookmark-write))
-    (define-key map [save]     '("Save Bookmarks" . bookmark-save))
-    (define-key map [edit]     '("Edit Bookmark List" . bookmark-bmenu-list))
-    (define-key map [delete]   '("Delete Bookmark..." . bookmark-delete))
-    (define-key map [rename]   '("Rename Bookmark..." . bookmark-rename))
-    (define-key map [locate]   '("Insert Location..." . bookmark-locate))
-    (define-key map [insert]   '("Insert Contents..." . bookmark-insert))
-    (define-key map [set]      '("Set Bookmark..." . bookmark-set))
-    (define-key map [jump]     '("Jump to Bookmark..." . bookmark-jump))
+    (define-key map [load]
+      `(menu-item ,(purecopy "Load a Bookmark File...") bookmark-load
+                 :help ,(purecopy "Load bookmarks from a bookmark file)")))
+    (define-key map [write]
+      `(menu-item ,(purecopy "Save Bookmarks As...") bookmark-write
+                 :help ,(purecopy "Write bookmarks to a file (reading the file name with the minibuffer)")))
+    (define-key map [save]
+      `(menu-item ,(purecopy "Save Bookmarks") bookmark-save
+                 :help ,(purecopy "Save currently defined bookmarks")))
+    (define-key map [edit]
+      `(menu-item ,(purecopy "Edit Bookmark List") bookmark-bmenu-list
+                 :help ,(purecopy "Display a list of existing bookmarks")))
+    (define-key map [delete]
+      `(menu-item ,(purecopy "Delete Bookmark...") bookmark-delete
+                 :help ,(purecopy "Delete a bookmark from the bookmark list")))
+    (define-key map [rename]
+      `(menu-item ,(purecopy "Rename Bookmark...") bookmark-rename
+                 :help ,(purecopy "Change the name of a bookmark")))
+    (define-key map [locate]
+      `(menu-item ,(purecopy "Insert Location...") bookmark-locate
+                 :help ,(purecopy "Insert the name of the file associated with a bookmark")))
+    (define-key map [insert]
+      `(menu-item ,(purecopy "Insert Contents...") bookmark-insert
+                 :help ,(purecopy "Insert the text of the file pointed to by a bookmark")))
+    (define-key map [set]
+      `(menu-item ,(purecopy "Set Bookmark...") bookmark-set
+                 :help ,(purecopy "Set a bookmark named inside a file.")))
+    (define-key map [jump]
+      `(menu-item ,(purecopy "Jump to Bookmark...") bookmark-jump
+                 :help ,(purecopy "Jump to a bookmark (a point in some file)")))
     map))
 
 ;;;###autoload
@@ -2078,6 +2166,12 @@ This also runs `bookmark-exit-hook'."
 
 (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)