* bookmark.el: Update documentation, especially documentation

  of `bookmark-alist' and of the bookmark file format.
  Patch by Drew Adams, with minor tweaks from me.  (Bug#4195)

2 files changed, 117 insertions, 82 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 7edfa9c4144..8021b71f656 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,9 @@
+2009-10-24  Karl Fogel  <kfogel@red-bean.com>
+
+        * bookmark.el: Update documentation, especially documentation
+        of `bookmark-alist' and of the bookmark file format.
+        Patch by Drew Adams, with minor tweaks from me.  (Bug#4195)
+
 2009-10-24  Chong Yidong  <cyd@stupidchicken.com>
 
         * mail/emacsbug.el (report-emacs-bug): Clarify that the
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 331e392a9c7..ad715d8a21e 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -252,31 +252,39 @@ functions have a binding in this keymap.")
 ;;; Core variables and data structures:
 (defvar bookmark-alist ()
   "Association list of bookmarks and their records.
-You probably don't want to change the value of this alist yourself;
-instead, let the various bookmark functions do it for you.
+Bookmark functions update the value automatically.
+You probably do NOT want to change the value yourself.
 
-The format of the alist is
+The value is an alist with entries of the form
 
-       \(BOOKMARK1 BOOKMARK2 ...\)
+ (BOOKMARK-NAME . PARAM-ALIST)
 
-where each BOOKMARK is of the form
+or the deprecated form (BOOKMARK-NAME PARAM-ALIST).
 
-  (NAME PARAM-ALIST) or (NAME . PARAM-ALIST)
+ BOOKMARK-NAME is the name you gave to the bookmark when creating it.
 
-where the first form is the old deprecated one and the second is
-the new favored one.  PARAM-ALIST is typically of the form:
+ PARAM-ALIST is an alist of bookmark information.  The order of the
+ entries in PARAM-ALIST is not important.  The possible entries are
+ described below.  An entry with a key but null value means the entry
+ is not used.
 
- ((filename . FILE)
-  (front-context-string . FRONT-STR)
-  (rear-context-string  . REAR-STR)
+  (filename . FILENAME)
   (position . POS)
-  (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.")
-
+  (front-context-string . STR-AFTER-POS)
+  (rear-context-string  . STR-BEFORE-POS)
+  (handler . HANDLER)
+  (annotation . ANNOTATION)
+
+ FILENAME names the bookmarked file.
+ POS is the bookmarked buffer position (position in the file).
+ 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.  This is the case for Info bookmarks,
+ for instance.  HANDLER must accept a bookmark as argument.")
 
 (defvar bookmarks-already-loaded nil
   "Non-nil iff bookmarks have been loaded from `bookmark-default-file'.")
@@ -564,51 +572,71 @@ record that pertains to the location within the buffer."
 
 ;;; File format stuff
 
-;; The OLD format of the bookmark-alist was:
+;; *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
+;;       ((BOOKMARK-NAME (FILENAME
 ;;                          STRING-IN-FRONT
 ;;                          STRING-BEHIND
 ;;                          POINT))
 ;;        ...)
 ;;
-;; The NEW format of the bookmark-alist is:
+;; 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)
-;;                       (front-context-string . STRING-IN-FRONT)
-;;                       (rear-context-string  . STRING-BEHIND)
-;;                       (position   . POINT)
+;;                       (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.
 ;;
-;; I switched to using an internal as well as external alist because I
-;; felt that would be a more flexible framework in which to add
-;; features.  It means that the order in which values appear doesn't
-;; matter, and it means that arbitrary values can be added without
-;; risk of interfering with existing ones.
-;;
-;; BOOKMARK-NAME is the string the user gives the bookmark and
-;; accesses it by from then on.
+;; 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.)
 ;;
-;; FILENAME is the location of the file in which the bookmark is set.
+;; To deal with the change from FIRST format to SECOND, conversion
+;; code was added, and it is still in use.  See
+;; `bookmark-maybe-upgrade-file-format'.
 ;;
-;; STRING-IN-FRONT is a string of `bookmark-search-size' chars of
-;; context in front of the point at which the bookmark is set.
+;; No conversion from SECOND to CURRENT is done.  Instead, the code
+;; handles both formats OK.  It must continue to do so.
 ;;
-;; STRING-BEHIND is the same thing, but after the point.
-;;
-;; The context strings exist so that modifications to a file don't
-;; necessarily cause a bookmark's position to be invalidated.
-;; bookmark-jump will search for STRING-BEHIND and STRING-IN-FRONT in
-;; case the file has changed since the bookmark was set.  It will
-;; attempt to place the user before the changes, if there were any.
-;; ANNOTATION is the annotation for the bookmark; it may not exist
-;; (for backward compatibility), be nil (no annotation), or be a
-;; string.
+;; 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
@@ -744,25 +772,25 @@ This expects to be called from `point-min' in a bookmark file."
   "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.
+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 C-w while setting a bookmark.  Successive C-w's
 yank successive words.
 
-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.
+Typing C-u 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
+C-u inserts an appropriate name based on the buffer or file.
 
-Use \\[bookmark-delete] to remove bookmarks \(give it a name and it
-removes only the first instance of a bookmark with that name from
+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* ((record (bookmark-make-record))
@@ -890,8 +918,8 @@ to the buffer's file name if `bookmark-current-bookmark' is nil."
 
 
 (defun bookmark-buffer-name ()
-  "Return the name of the current buffer (or its file, if any) in a
-way that is suitable as a bookmark 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))
@@ -1012,8 +1040,8 @@ 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'."
+bookmark.  It defaults to `switch-to-buffer'.  A typical value for
+DISPLAY-FUNC would be `switch-to-buffer-other-window'."
   (interactive
    (list (bookmark-completing-read "Jump to bookmark"
                                    bookmark-current-bookmark)))
@@ -1053,8 +1081,7 @@ 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'.
 
 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.
-"
+that file no longer exists, then offer interactively to relocate BOOKMARK."
   (condition-case err
       (funcall (or (bookmark-get-handler bookmark)
                    'bookmark-default-handler)
@@ -1368,8 +1395,9 @@ for a file, defaulting to the file defined by variable
 
 
 (defun bookmark-import-new-list (new-list)
-  "Add NEW-LIST of bookmarks to `bookmark-alist', rename new bookmarks
-with \"<N>\" extensions where they collide with existing bookmark names."
+  "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 ((lst new-list)
         (names (bookmark-all-names)))
     (while lst
@@ -1381,8 +1409,8 @@ with \"<N>\" extensions where they collide with existing bookmark names."
 
 
 (defun bookmark-maybe-rename (full-record names)
-  "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'."
+  "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
@@ -1640,9 +1668,8 @@ 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."
+Non-nil FORCE forces a redisplay showing the filenames.  FORCE is used
+mainly for debugging, and should not be necessary in normal use."
   (if (and (not force) bookmark-bmenu-toggle-filenames)
       nil ;already shown, so do nothing
     (save-excursion
@@ -1670,9 +1697,8 @@ used mainly for debugging, and should not be necessary in normal usage."
 
 (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."
+Non-nil FORCE forces a redisplay showing the filenames.  FORCE is used
+mainly for debugging, and should not be necessary in normal use."
   (if (and (not force) bookmark-bmenu-toggle-filenames)
       ;; nothing to hide if above is nil
       (save-excursion
@@ -1709,12 +1735,16 @@ used mainly for debugging, and should not be necessary in normal usage."
 
 
 (defun bookmark-bmenu-check-position ()
-  "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
+  "If point is not on a bookmark line, move it to one.
+If before the first bookmark line, move it to the first.
+If after the last, move it to the last.
+Return `bookmark-alist'"
+  ;; FIXME: The doc string originally implied that this returns nil if
+  ;; not on a bookmark, which is false.  Is there any real reason to
+  ;; return `bookmark-alist'?  This seems to be called in a few places
+  ;; as a check of whether point is on a bookmark line.  Those
+  ;; "checks" are in fact no-ops, since this never returns nil.
+  ;; -dadams, 2009-10-10
   (cond ((< (count-lines (point-min) (point)) 2)
          (goto-char (point-min))
          (forward-line 2)
@@ -1728,7 +1758,6 @@ still no bookmark, return nil."
 
 (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
         (save-window-excursion