PATCH: bookmark.el LIFO preservation

PATCH: bookmark.el LIFO preservation

[Thien-Thi Nguyen]

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

according to comments in bookmark.el, if `bookmark-sort-flag' is
non-nil, the entries in the menu list are to be displayed in LIFO order.

unfortunately, while `bookmark-maybe-sort-alist' currently does manage a
properly non-destructive sort, it then goes on to assign the result to
`bookmark-alist' anyway, thus losing the original order.  entries have
no timestamp, either, so the order cannot even be recovered.

to see this behavior:

  (setq bookmark-sort-flag t)         ; default t anyway
  (bookmark-bmenu-list)               ; note lexical ordering
  (setq bookmark-sort-flag nil)
  (bookmark-bmenu-list)               ; note lexical ordering
  (describe-variable 'bookmark-alist)

the last form is not strictly necessary; i include it to demonstrate the
lossage at the data-structure level.  below is a small patch that makes
the display option actually only affect display.  here is a ChangeLog
entry:

	* bookmark.el (bookmark-maybe-sort-alist): Don't modify
	bookmark-alist.  Instead, if not sorting, simply return it.
	(bookmark-bmenu-list): Call bookmark-maybe-sort-alist
	for its return value, not for its side effect.

do you mind if i install it?

thi

________________________________________________________

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: .ttn.bookmark.diff --]
[-- Type: text/x-diff, Size: 1852 bytes --]

Index: bookmark.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/bookmark.el,v
retrieving revision 1.94
diff -c -r1.94 bookmark.el
*** bookmark.el	16 Jul 2007 02:16:00 -0000	1.94
--- bookmark.el	16 Jul 2007 10:11:25 -0000
***************
*** 1045,1054 ****
    ;;Return the bookmark-alist for display.  If the bookmark-sort-flag
    ;;is non-nil, then return a sorted copy of the alist.
    (if bookmark-sort-flag
!       (setq bookmark-alist
!             (sort (copy-alist bookmark-alist)
!                   (function
!                    (lambda (x y) (string-lessp (car x) (car y))))))))
  
  
  (defvar bookmark-after-jump-hook nil
--- 1045,1054 ----
    ;;Return the bookmark-alist for display.  If the bookmark-sort-flag
    ;;is non-nil, then return a sorted copy of the alist.
    (if bookmark-sort-flag
!       (sort (copy-alist bookmark-alist)
!             (function
!              (lambda (x y) (string-lessp (car x) (car y)))))
!     bookmark-alist))
  
  
  (defvar bookmark-after-jump-hook nil
***************
*** 1590,1596 ****
      (insert "% Bookmark\n- --------\n")
      (add-text-properties (point-min) (point)
  			 '(font-lock-face bookmark-menu-heading))
-     (bookmark-maybe-sort-alist)
      (mapcar
       (lambda (full-record)
         ;; if a bookmark has an annotation, prepend a "*"
--- 1590,1595 ----
***************
*** 1613,1619 ****
  		  help-echo "mouse-2: go to this bookmark in other window")))
  	   (insert "\n")
  	   )))
!      bookmark-alist))
    (goto-char (point-min))
    (forward-line 2)
    (bookmark-bmenu-mode)
--- 1612,1618 ----
  		  help-echo "mouse-2: go to this bookmark in other window")))
  	   (insert "\n")
  	   )))
!      (bookmark-maybe-sort-alist)))
    (goto-char (point-min))
    (forward-line 2)
    (bookmark-bmenu-mode)

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: PATCH: bookmark.el LIFO preservation

[Karl Fogel]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:
> according to comments in bookmark.el, if `bookmark-sort-flag' is
> non-nil, the entries in the menu list are to be displayed in LIFO order.
>
> unfortunately, while `bookmark-maybe-sort-alist' currently does manage a
> properly non-destructive sort, it then goes on to assign the result to
> `bookmark-alist' anyway, thus losing the original order.  entries have
> no timestamp, either, so the order cannot even be recovered.
>
> to see this behavior:
>
>   (setq bookmark-sort-flag t)         ; default t anyway
>   (bookmark-bmenu-list)               ; note lexical ordering
>   (setq bookmark-sort-flag nil)
>   (bookmark-bmenu-list)               ; note lexical ordering
>   (describe-variable 'bookmark-alist)
>
> the last form is not strictly necessary; i include it to demonstrate the
> lossage at the data-structure level.  below is a small patch that makes
> the display option actually only affect display.  here is a ChangeLog
> entry:
>
> 	* bookmark.el (bookmark-maybe-sort-alist): Don't modify
> 	bookmark-alist.  Instead, if not sorting, simply return it.
> 	(bookmark-bmenu-list): Call bookmark-maybe-sort-alist
> 	for its return value, not for its side effect.
>
> do you mind if i install it?

Thank you for the clear explanation and the fix!  It'd be great if you
would install it, yes.

-Karl