[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: doc-view support for bookmark.el

From: Karl Fogel
Subject: Re: doc-view support for bookmark.el
Date: Wed, 26 Dec 2007 10:53:39 -0800
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1.50 (gnu/linux)

I like your new bookmark abstractions, Tassilo!  A few comments:

Tassilo Horn <address@hidden> writes:
> --- lisp/bookmark.el  25 Sep 2007 10:43:39 -0000      1.98
> +++ lisp/bookmark.el  25 Dec 2007 21:51:50 -0000
> @@ -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.")

Okay, so "(BUFFER . POINT)" is now an exported interface, not just an
internal interface that we would be free to change inside bookmark.el.
Therefore, let's make it more extensible than a dotted pair.  The
cleanest thing would probably be an association list:

   '((buffer BUFFER) (point POINT))

That way, callers will be compatible even if we extend the type later:

   '((buffer BUFFER) (point POINT) ...)

And a comment regarding documentation:

Normally, a function type's documentation should all be in one place,
I think, not referring out to specific instances.  So normally, I
would suggest replacing this...

   The function will be called with two arguments: ANNOTATION and
   INFO-NODE.  See `bookmark-make-cell-for-text-file' for a

... with full documentation of the INFO-NODE argument.  But in this
case, the better thing might be to implement
`bookmark-make-cell-for-info-node' as a separate instance of
`bookmark-make-cell-function', adjusting everything else in the
obvious way.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]