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

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

[Karl Fogel]

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.

Thanks,
-Karl

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

[Drew Adams]

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

Yes, but not before next weekend. I just don't have the time now.

I'll try to take a look at it before too long, and send you something for
comment. (Yes, 3646 is related to 4195.)

Thanks for your work on these bugs.

 - Drew

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

[Drew Adams]

[-- 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