bug#4195: bookmark-alist and bookmark doc about structure

["Drew Adams" <drew.adams@oracle.com>]

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

> From: Karl Fogel Sent: Monday, October 05, 2009 8:40 PM
> 
> Drew, can you have a go at fixing this one?  It looks like you've got
> the necessary head state, perhaps more than I do.  I've also got some
> other bookmark.el bug reports (3646, 3645, 3375, 1199, 4485, 
> 4296, 4349, and 4348 -- though note that 3646 may actually get easier 
> after 4195 is fixed), most of which will require coding, so I'd like to 
> concentrate on them first.
> Fine to say no, obviously :-), but I thought I'd ask.

Hi Karl,

Attached is a patch against today's CVS file.

I updated the `bookmark-alist' doc string and the large comment about the
various formats and their evolution. I also made some minor changes to other doc
strings (e.g. some first lines that were too long). Please read over what I
wrote and correct it as you see fit. In particular, I'm no expert on the format
changes.

HTH, and thanks for all your work on the bugs.

 - Drew

[-- Attachment #2: bookmark-2009-10-10.patch --]
[-- Type: application/octet-stream, Size: 18035 bytes --]

diff -cw "bookmark-CVS-2009-10-10.el" "bookmark-patched-2009-10-10.el"
*** bookmark-CVS-2009-10-10.el	Sat Oct 10 10:51:36 2009
--- bookmark-patched-2009-10-10.el	Sat Oct 10 12:22:10 2009
***************
*** 252,282 ****
  ;;; 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.
  
! The format of the alist is
  
!        \(BOOKMARK1 BOOKMARK2 ...\)
  
! where each BOOKMARK is of the form
  
!   (NAME PARAM-ALIST) or (NAME . PARAM-ALIST)
  
! where the first form is the old deprecated one and the second is
! the new favored one.  PARAM-ALIST is typically of the form:
  
!  ((filename . FILE)
!   (front-context-string . FRONT-STR)
!   (rear-context-string  . REAR-STR)
    (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.")
! 
  
  (defvar bookmarks-already-loaded nil)
  
--- 252,290 ----
  ;;; Core variables and data structures:
  (defvar bookmark-alist ()
    "Association list of bookmarks and their records.
! Bookmark functions update the value automatically.
! You probably do NOT want to change the value yourself.
  
! The value is an alist with entries of the form
  
!  (BOOKMARK-NAME . PARAM-ALIST)
  
! or the deprecated form (BOOKMARK-NAME PARAM-ALIST).
  
!  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 possible entries are
!   described below.  An entry with a key but null value means the entry
!   is not used.
  
!   (filename . FILENAME)
    (position . POS)
!   (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)
  
***************
*** 556,572 ****
  \f
  ;;; File format stuff
  
! ;; The OLD format of the bookmark-alist was:
  ;;
! ;;       ((BOOKMARK-NAME . (FILENAME
  ;;                          STRING-IN-FRONT
  ;;                          STRING-BEHIND
  ;;                          POINT))
  ;;        ...)
  ;;
! ;; The NEW format of the bookmark-alist is:
  ;;
! ;;       ((BOOKMARK-NAME (filename   . FILENAME)
  ;;                       (front-context-string . STRING-IN-FRONT)
  ;;                       (rear-context-string  . STRING-BEHIND)
  ;;                       (position   . POINT)
--- 564,588 ----
  \f
  ;;; File format stuff
  
! ;; *THIS INFORMATION IS IMPORTANT* if you are manipulating bookmark
! ;; structure or thinking about modifying (redefining) it - please
! ;; don't.
! ;;
! ;; 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)
  ;;                        (front-context-string . STRING-IN-FRONT)
  ;;                        (rear-context-string  . STRING-BEHIND)
  ;;                        (position   . POINT)
***************
*** 576,606 ****
  ;;                       ))
  ;;        ...)
  ;;
  ;;
! ;; 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.
  ;;
! ;; FILENAME is the location of the file in which the bookmark is set.
  ;;
! ;; STRING-IN-FRONT is a string of `bookmark-search-size' chars of
! ;; context in front of the point at which the bookmark is set.
  ;;
! ;; 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.
  
  
  (defconst bookmark-file-format-version 1
--- 592,633 ----
  ;;                        ))
  ;;        ...)
  ;;
+ ;; 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)
! ;;                       (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, and it is still in use - 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
***************
*** 736,760 ****
    "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 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 \(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))
--- 763,787 ----
    "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 C-w while setting a bookmark.  Successive C-w's
  yank successive words.
  
! 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 \(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))
***************
*** 884,891 ****
  
  
  (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."
    (cond
     ;; Or are we a file?
     (buffer-file-name (file-name-nondirectory buffer-file-name))
--- 911,918 ----
  
  
  (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))
***************
*** 906,913 ****
  
  
  (defun bookmark-yank-word ()
!   "Get the next word from the buffer and append it to the name of the
! bookmark currently being set, advancing point by one word."
    (interactive)
    (let ((string (with-current-buffer bookmark-current-buffer
                    (goto-char bookmark-yank-point)
--- 933,939 ----
  
  
  (defun bookmark-yank-word ()
!   "Insert the next word from the bookmarked buffer into the current buffer."
    (interactive)
    (let ((string (with-current-buffer bookmark-current-buffer
                    (goto-char bookmark-yank-point)
***************
*** 1005,1012 ****
  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)))
--- 1031,1038 ----
  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 value for
! DISPLAY-FUNCC would be `switch-to-buffer-other-window'."
    (interactive
     (list (bookmark-completing-read "Jump to bookmark"
  				   bookmark-current-bookmark)))
***************
*** 1046,1053 ****
  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.
! "
    (condition-case err
        (funcall (or (bookmark-get-handler bookmark)
                     'bookmark-default-handler)
--- 1072,1078 ----
  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."
    (condition-case err
        (funcall (or (bookmark-get-handler bookmark)
                     'bookmark-default-handler)
***************
*** 1362,1369 ****
  
  
  (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."
    (let ((lst new-list)
          (names (bookmark-all-names)))
      (while lst
--- 1387,1395 ----
  
  
  (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 ((lst new-list)
          (names (bookmark-all-names)))
      (while lst
***************
*** 1375,1382 ****
  
  
  (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'."
    (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
--- 1401,1408 ----
  
  
  (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
***************
*** 1634,1642 ****
  
  (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
--- 1660,1667 ----
  
  (defun bookmark-bmenu-show-filenames (&optional force)
    "In an interactive bookmark list, show file names along with bookmarks.
! 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
***************
*** 1664,1672 ****
  
  (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
--- 1689,1696 ----
  
  (defun bookmark-bmenu-hide-filenames (&optional force)
    "In an interactive bookmark list, hide the filenames of the bookmarks.
! 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
***************
*** 1703,1714 ****
  
  
  (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
    (cond ((< (count-lines (point-min) (point)) 2)
           (goto-char (point-min))
           (forward-line 2)
--- 1727,1742 ----
  
  
  (defun bookmark-bmenu-check-position ()
!   "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)
***************
*** 1722,1728 ****
  
  (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
--- 1750,1755 ----

Diff finished at Sat Oct 10 12:23:03