[patch, 21.0-ish] Re: bookmark.el

[patch for fsf-compat package at end] Graham Todd <gtodd(a)yorku.ca&gt; writes: The thing is that bookmark.el generates a FSF style popup menu dynamically from the bookmarks. It then tries to convert them to XEmacs style menus on the fly: (cond ((fboundp 'x-popup-menu) ; Emacs (x-popup-menu event (bookmark-menu-build-paned-menu name entries))) (t ; XEmacs (get-popup-menu-response (cons name (mapcar (function (lambda (x) (if (stringp x) (vector x nil nil) (vector (car x) (list (car x)) t)))) (bookmark-menu-build-paned-menu name entries))))))) Apparently somebody changed the format of the popup menus generated without changing the code that converts them back. The funny thing is. It (almost) works correctly on 21.0. This is "by accident". The XEmacs 21.0 fsf-compat package now contains an x-popup-menu.el that emulates the function and does the conversion. Luckily the code in bookmark.el is such that it is prefers that function. you = Karl from now on. Not intentionally at least. In fact we would very much prefer if there were only one version that works on both emacsen. I think the XEmacs versions are just plain copies of what ever you release. I think it would be very nice if you could maintain it for XEmacs too in the sense that you either get check-in access to the CVS yourself or simply send any new versions you release to xemacs-patches(a)xemacs.org. You then just need somebody to make sure the new releases work on XEmacs (i.e. testing). Graham? If any of you then need XEmacs specific lisp help you can just mail to xemacs-beta. As a start you can simply ditch the the XEmacs specific code above since it doesn't work anyway and is not needed on 21.0. In fact that way it is clear that you need the fsf-compat package. He, wait a second! That already happened the version in FSF 20.3 does NOT contain that particular code any more. However it does work. It calls set-text-properties on 'nil' which is bad. At the end there is a patch against the FSF 20.3 version of which makes it work on XEmacs (together with the popup menu patch). This patch just removes ALL of the XEmacs specifics. Hooray! Could you fold that into your version and then send it to xemacs-patches(a)xemacs.org/ Jan P.S. There is one but... Unfortunately the current version of x-popup-menu.el for 21.0 is broken. It needs this patch diff -u fsf-compat/x-popup-menu.el~ fsf-compat/x-popup-menu.el --- fsf-compat/x-popup-menu.el~ Thu Dec 17 19:45:39 1998 +++ fsf-compat/x-popup-menu.el Thu Dec 17 19:45:39 1998 ＠＠ -22,7 +22,7 ＠＠ ;; Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA ;; 02111-1307, USA. -;;; Synched up with: Not in FSF +;;; Synched up with: Not in FSF ;;; Commentary: ＠＠ -30,7 +30,7 ＠＠ ;;;###autoload (defun x-popup-menu (event menu) - "Pop up menu for Mouse-2 for selected date in the calendar window." + "Let user select a value with from a menu." (save-excursion (let ((title (car menu)) ;; try to ignore just a "" string, XEmacs will typically add two ＠＠ -44,16 +44,11 ＠＠ (setq selection (get-popup-menu-response (cons title (convert_fsf_popup mb-items)) event)) - ;; normally, we'll get a <#event (call-intercatively function)> - ;; return, but if nothing was selected, we'll have <#event - ;; (run-hooks menu-no-select-hook. So, if something is selected, - ;; return it, other run the hook - (if (string-match (symbol-name (event-function selection)) - "call-interactively") - (setq selection (event-object selection)) - (funcall (event-function selection) (event-object selection)) - )))) + ;; Now that we wrap the stuff to be returned in a list + ;; we can simply call the function. Really popup-menu should simply + ;; return the return value of the function run! + (funcall (event-function selection) (event-object selection))))) (defun convert_fsf_popup (menu) "Convert FSF style menu notation to the XEmacs format." ＠＠ -67,7 +62,7 ＠＠ (;; alist -> vector (and (stringp (car x)) (not (true-list-p x))) - (vector (car x) (cdr x))) + (vector (car x) (list 'identity (cdr x)))) (;; submenu (and (stringp (car x)) (true-list-p (cdr x))) [Note this is against the FSF 20.3 version of bookmark.el, for Karl] diff -u /u/users/vroonhof/scratch/xemacs/bookmark.el~ /u/users/vroonhof/scratch/xemacs/bookmark.el --- /u/users/vroonhof/scratch/xemacs/bookmark.el~ Thu Dec 17 20:44:22 1998 +++ /u/users/vroonhof/scratch/xemacs/bookmark.el Thu Dec 17 20:44:22 1998 ＠＠ -215,14 +215,6 ＠＠ ;; some random value higher than 9600 (setq baud-rate 19200)) -;; XEmacs apparently call this `buffer-substring-without-properties', -;; sigh. -(or (fboundp 'buffer-substring-no-properties) - (if (fboundp 'buffer-substring-without-properties) - (fset 'buffer-substring-no-properties - 'buffer-substring-without-properties) - (fset 'buffer-substring-no-properties 'buffer-substring))) -  ;;; Keymap stuff: ＠＠ -516,10 +508,10 ＠＠ INFO-NODE, so record this fact in the bookmark's entry." (bookmark-maybe-load-default-file) (let ((stripped-name (copy-sequence name))) - (or bookmark-xemacsp ;; XEmacs's `set-text-properties' doesn't work on ;; free-standing strings, apparently. - (set-text-properties 0 (length stripped-name) nil stripped-name)) + ;; JV well it should! Just do it. + (set-text-properties 0 (length stripped-name) nil stripped-name) (if (and (bookmark-get-bookmark stripped-name) (not overwrite)) ;; already existing bookmark under that name and ;; no prefix arg means just overwrite old bookmark ＠＠ -570,13 +562,15 ＠＠ ;; Now fill in the optional parts: ;; Take no chances with text properties - (set-text-properties 0 (length annotation) nil annotation) - (set-text-properties 0 (length info-node) nil info-node) (if annotation - (nconc the-record (list (cons 'annotation annotation)))) + (progn + (set-text-properties 0 (length annotation) nil annotation) + (nconc the-record (list (cons 'annotation annotation))))) (if info-node - (nconc the-record (list (cons 'info-node info-node)))) + (progn + (set-text-properties 0 (length info-node) nil info-node) + (nconc the-record (list (cons 'info-node info-node))))) ;; Finally, return the completed record. the-record))