Re: doc-view support for bookmark.el

[Tassilo Horn <tassilo@member.fsf.org>]

Hi Stefan,

here's the implementation of your idea and doc-view using it.

--8<---------------cut here---------------start------------->8---
2007-12-25  Tassilo Horn  <tassilo@member.fsf.org>

	* bookmark.el (bookmark-make-cell-function): New variable.
	(bookmark-make): Call bookmark-make-cell-function's function
	instead of bookmark-make-cell.
	(bookmark-get-handler): New function.
	(bookmark-jump, bookmark-jump-other-window, bookmark-insert)
	(bookmark-bmenu-2-window, bookmark-bmenu-other-window): Use the
	bookmark's handler function if it has one defined.
	(bookmark-make-cell-for-text-file): Renamed from
	bookmark-make-cell.

	* doc-view.el (doc-view-bookmark-make-cell)
	(doc-view-bookmark-jump): New functions.
	(doc-view-mode): Set bookmark-make-cell-function buffer-locally.
--8<---------------cut here---------------end--------------->8---

Here's the patch:

--8<---------------cut here---------------start------------->8---
Index: lisp/doc-view.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/doc-view.el,v
retrieving revision 1.26
diff -u -r1.26 doc-view.el
--- lisp/doc-view.el	6 Dec 2007 15:04:29 -0000	1.26
+++ lisp/doc-view.el	25 Dec 2007 21:51:49 -0000
@@ -958,6 +958,8 @@
     (set (make-local-variable 'cursor-type) nil)
     (use-local-map doc-view-mode-map)
     (set (make-local-variable 'after-revert-hook) 'doc-view-reconvert-doc)
+    (set (make-local-variable 'bookmark-make-cell-function)
+			      'doc-view-bookmark-make-cell)
     (setq mode-name "DocView"
 	  buffer-read-only t
 	  major-mode 'doc-view-mode)
@@ -996,4 +998,32 @@
 ;; End:
 
 ;; arch-tag: 5d6e5c5e-095f-489e-b4e4-1ca90a7d79be
+;;;; Bookmark integration
+
+(defun doc-view-bookmark-make-cell (annotation &rest args)
+  (let ((the-record
+         `((filename . ,(buffer-file-name))
+           (page     . ,doc-view-current-page)
+           (handler  . doc-view-bookmark-jump))))
+
+    ;; Take no chances with text properties
+    (set-text-properties 0 (length annotation) nil annotation)
+
+    (when annotation
+      (nconc the-record (list (cons 'annotation annotation))))
+
+    ;; Finally, return the completed record.
+    the-record))
+
+;;;###autoload
+(defun doc-view-bookmark-jump (bmk)
+  (save-window-excursion
+    (let ((filename (bookmark-get-filename bmk))
+	  (page (cdr (assq 'page (bookmark-get-bookmark-record bookmark)))))
+      (find-file filename)
+      (when (not (eq major-mode 'doc-view-mode))
+	(doc-view-toggle-display))
+      (doc-view-goto-page page)
+      (cons (current-buffer) 1))))
+
 ;;; doc-view.el ends here
Index: lisp/bookmark.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/bookmark.el,v
retrieving revision 1.98
diff -u -r1.98 bookmark.el
--- lisp/bookmark.el	25 Sep 2007 10:43:39 -0000	1.98
+++ lisp/bookmark.el	25 Dec 2007 21:51:50 -0000
@@ -443,6 +443,8 @@
   (message "%S" (assq 'info-node (bookmark-get-bookmark-record bookmark)))
   (sit-for 4))
 
+(defun bookmark-get-handler (bookmark)
+  (cdr (assq 'handler (bookmark-get-bookmark-record bookmark))))
 
 (defvar bookmark-history nil
   "The history list for bookmark functions.")
@@ -480,6 +482,22 @@
     (interactive-p)
     (setq bookmark-history (cons ,string bookmark-history))))
 
+(defvar bookmark-make-cell-function 'bookmark-make-cell-for-text-file
+  "A function that should be called to create the bookmark
+record.  Modes may set this variable buffer-locally to enable
+bookmarking of non-text files like images or pdf documents.
+
+The function will be called with two arguments: ANNOTATION and
+INFO-NODE.  See `bookmark-make-cell-for-text-file' for a
+description.
+
+The returned record may contain a special cons (handler
+. some-function) which sets the handler function that should be
+used to open this bookmark instead of `bookmark-jump-noselect'.
+It should return a cons (BUFFER . POINT) indicating buffer
+showing the bookmarked location and the value of point in that
+buffer.  Like `bookmark-jump-noselect' the buffer shouldn't be
+selected by the handler.")
 
 (defun bookmark-make (name &optional annotation overwrite info-node)
   "Make a bookmark named NAME.
@@ -498,7 +516,7 @@
         ;; already existing bookmark under that name and
         ;; no prefix arg means just overwrite old bookmark
         (setcdr (bookmark-get-bookmark stripped-name)
-                (list (bookmark-make-cell annotation info-node)))
+                (list (funcall bookmark-make-cell-function annotation info-node)))
 
       ;; otherwise just cons it onto the front (either the bookmark
       ;; doesn't exist already, or there is no prefix arg.  In either
@@ -507,7 +525,7 @@
       (setq bookmark-alist
             (cons
              (list stripped-name
-                   (bookmark-make-cell annotation info-node))
+                   (funcall bookmark-make-cell-function annotation info-node))
              bookmark-alist)))
 
     ;; Added by db
@@ -518,7 +536,7 @@
         (bookmark-save))))
 
 
-(defun bookmark-make-cell (annotation &optional info-node)
+(defun bookmark-make-cell-for-text-file (annotation &optional info-node)
   "Return the record part of a new bookmark, given ANNOTATION.
 Must be at the correct position in the buffer in which the bookmark is
 being set.  This might change someday.
@@ -1068,7 +1086,10 @@
   (unless bookmark
     (error "No bookmark specified"))
   (bookmark-maybe-historicize-string bookmark)
-  (let ((cell (bookmark-jump-noselect bookmark)))
+  (let* ((handler (bookmark-get-handler bookmark))
+	 (cell (if handler
+		   (funcall handler bookmark)
+		 (bookmark-jump-noselect bookmark))))
     (and cell
          (switch-to-buffer (car cell))
          (goto-char (cdr cell))
@@ -1090,7 +1111,10 @@
          (list bkm) bkm)))
   (when bookmark
     (bookmark-maybe-historicize-string bookmark)
-    (let ((cell (bookmark-jump-noselect bookmark)))
+    (let* ((handler (bookmark-get-handler bookmark))
+	   (cell (if handler
+		     (funcall handler bookmark)
+		   (bookmark-jump-noselect bookmark))))
       (and cell
            (switch-to-buffer-other-window (car cell))
            (goto-char (cdr cell))
@@ -1272,10 +1296,14 @@
   (interactive (list (bookmark-completing-read "Insert bookmark contents")))
   (bookmark-maybe-historicize-string bookmark)
   (bookmark-maybe-load-default-file)
-  (let ((orig-point (point))
+  (let* ((orig-point (point))
+	 (handler (bookmark-get-handler bookmark))
+	 (cell (if handler
+		   (funcall handler bookmark)
+		 (bookmark-jump-noselect bookmark)))
         (str-to-insert
          (save-excursion
-           (set-buffer (car (bookmark-jump-noselect bookmark)))
+           (set-buffer (car cell))
            (buffer-string))))
     (insert str-to-insert)
     (push-mark)
@@ -1904,7 +1932,10 @@
             (pop-up-windows t))
         (delete-other-windows)
         (switch-to-buffer (other-buffer))
-	(let* ((pair (bookmark-jump-noselect bmrk))
+	(let* ((handler (bookmark-get-handler bookmark))
+	       (pair (if handler
+			 (funcall handler bookmark)
+		       (bookmark-jump-noselect bookmark)))
                (buff (car pair))
                (pos  (cdr pair)))
           (pop-to-buffer buff)
@@ -1924,7 +1955,10 @@
   (interactive)
   (let ((bookmark (bookmark-bmenu-bookmark)))
     (if (bookmark-bmenu-check-position)
-	(let* ((pair (bookmark-jump-noselect bookmark))
+	(let* ((handler (bookmark-get-handler bookmark))
+	       (pair (if handler
+			 (funcall handler bookmark)
+		       (bookmark-jump-noselect bookmark)))
                (buff (car pair))
                (pos  (cdr pair)))
 	  (switch-to-buffer-other-window buff)
@@ -1942,7 +1976,10 @@
         same-window-buffer-names
         same-window-regexps)
     (if (bookmark-bmenu-check-position)
-	(let* ((pair (bookmark-jump-noselect bookmark))
+	(let* ((handler (bookmark-get-handler bookmark))
+	       (pair (if handler
+			 (funcall handler bookmark)
+		       (bookmark-jump-noselect bookmark)))
                (buff (car pair))
                (pos  (cdr pair)))
 	  (display-buffer buff)
--8<---------------cut here---------------end--------------->8---

Do you think it's ok and I should install it?

I've tested it with normal text files, a pdf and a ps file and it works
very nice.

Bye,
Tassilo