Re: Base patch for bug #5975 (bookmarking from Gnus Article buffer).

[Thierry Volpiatto]

Hi Karl,

Karl Fogel <address@hidden> writes:

> I have a branch that prepares the way for setting bookmarks in Gnus
> Article buffers, based on Thierry's patch in bug #5975.
>
> I'm not sure where to apply this.  Is pushing changes to trunk is okay
> right now?  Starting from [1] and [2], I didn't find any notice that we
> are in feature freeze, but I'm pretty sure last time I pushed it turned
> out to be a mistake and that I should not have pushed to trunk.
>
> Independently of the above, Thierry, note I made some changes to your
> patch.  You used (or kept) parameter names `point-only' and
> `read-only';
kept:  point-only
added: read-only

> I changed those to `no-file' and `no-context', to describe what they are
Yes nice these names are better.

> actually doing (whether the buffer is read-only or not is irrelevant to
> whether one wants the front/rear context strings in bookmark record).  I
> also fixed up the doc string to mention your new POSN parameter, which I
> moved to the end of the parameter list.  (Don't worry, I will adjust the
> gnus-sum.el patch as needed when the time comes.)
>
> I have not (yet) included your changes to gnus/gnus-art.el and
> gnus/gnus-sum.el, which finish the new functionality.  Two reasons:
>
>   1. You said in the bug that you fixed C-w later.  Could you provide
>      that patch, please?
I sent the patch at the time that provide this feature, but it seem it
had been lost.
I have merged this feature in bookmark-extensions.el and it work fine
since long time now.
I will try to retrieve the patch for Emacs, if i don't retrieve it, it
would be good you commit the changes without the C-w feature and i will
create a patch that apply on top of this.(I really don't remember yet
what i changed to make that working, need to check my logs and
bookmark-extensions.el)

>   2. I'm not sure whether it's okay to commit significant code changes
>      under gnus/, since the master is maintained outside the Emacs tree. 
Can't tell you for gnus, i am not aware of the procedure you use in
Emacs for developping Emacs<==>Gnus.

>      If it is okay, then I am happy to apply your change (plus the C-w
>      fix).  I did test it, and it worked, except for C-w of course.

All is working fine here since long time now, C-w included. 

> Anyone who has comments or answers re the above, please chime in.
>
> Just for reference, the current (preparatory) patch is below.  I will
> follow up separately with the gnus/* patch, so it's in the archives.
>
> Thanks for the patch, Thierry.  We're getting close.
>
> -Karl
>
> [1] http://www.gnu.org/software/emacs/
> [2] http://savannah.gnu.org/projects/emacs
>
> === modified file 'lisp/ChangeLog'
> --- lisp/ChangeLog    2010-07-13 10:41:49 +0000
> +++ lisp/ChangeLog    2010-07-13 21:19:42 +0000
> @@ -1,3 +1,26 @@
> +2010-07-13  Karl Fogel  <address@hidden>
> +
> +     Prepare the way for setting bookmarks in Gnus article buffers.
> +
> +     * lisp/bookmark.el (bookmark-make-record-default): Allow unneeded
> +     information to be omitted from the record.  Based on part of a
> +     patch by Thierry Volpiatto (Bug#5975).
> +
> +     Adjust declarations and calls:
> +
> +     * info.el (bookmark-make-record-default): Adjust declaration.
> +     (Info-bookmark-make-record): Adjust call.
> +
> +     * woman.el (bookmark-make-record-default): Adjust declaration.
> +     (woman-bookmark-make-record): Adjust call.
> +
> +     * man.el (bookmark-make-record-default): Adjust declaration.
> +     (Man-bookmark-make-record): Adjust call.
> +
> +     * image-mode.el (bookmark-make-record-default): Adjust declaration.
> +
> +     * doc-view.el (bookmark-make-record-default): Adjust declaration.
> +
>  2010-07-13  Adrian Robert  <address@hidden>
>  
>       * term/ns-win.el: Bind M-~ to 'ns-prev-frame (due to Matthew
>
> === modified file 'lisp/bookmark.el'
> --- lisp/bookmark.el  2010-07-10 18:52:53 +0000
> +++ lisp/bookmark.el  2010-07-13 21:19:42 +0000
> @@ -528,26 +528,36 @@
>      (setq bookmark-current-bookmark stripped-name)
>      (bookmark-bmenu-surreptitiously-rebuild-list)))
>  
> -(defun bookmark-make-record-default (&optional point-only)
> +(defun bookmark-make-record-default (&optional no-file no-context posn)
>    "Return the record describing the location of a new bookmark.
> -Must be at the correct position in the buffer in which the bookmark is
> -being set.
> -If POINT-ONLY is non-nil, then only return the subset of the
> -record that pertains to the location within the buffer."
> -  `(,@(unless point-only `((filename . ,(bookmark-buffer-file-name))))
> -    (front-context-string
> -     . ,(if (>= (- (point-max) (point)) bookmark-search-size)
> -            (buffer-substring-no-properties
> -             (point)
> -             (+ (point) bookmark-search-size))
> -          nil))
> -    (rear-context-string
> -     . ,(if (>= (- (point) (point-min)) bookmark-search-size)
> -            (buffer-substring-no-properties
> -             (point)
> -             (- (point) bookmark-search-size))
> -          nil))
> -    (position . ,(point))))
> +Point should be at the buffer in which the bookmark is being set,
> +and normally should be at the position where the bookmark is desired,
> +but see the optional arguments for other possibilities.
> +
> +If NO-FILE is non-nil, then only return the subset of the
> +record that pertains to the location within the buffer, leaving off
> +the part that records the filename.
> +
> +If NO-CONTEXT is non-nil, do not include the front- and rear-context
> +strings in the record -- the position is enough.
> +
> +If POSN is non-nil, record POSN as the point instead of `(point)'."
> +  `(,@(unless no-file `((filename . ,(bookmark-buffer-file-name))))
> +    ,@(unless no-context `((front-context-string
> +                           . ,(if (>= (- (point-max) (point))
> +                                      bookmark-search-size)
> +                                  (buffer-substring-no-properties
> +                                   (point)
> +                                   (+ (point) bookmark-search-size))
> +                                  nil))))
> +    ,@(unless no-context `((rear-context-string
> +                           . ,(if (>= (- (point) (point-min))
> +                                      bookmark-search-size)
> +                                  (buffer-substring-no-properties
> +                                   (point)
> +                                   (- (point) bookmark-search-size))
> +                                  nil))))
> +    (position . ,(or posn (point)))))
>  
>  
>  ;;; File format stuff
>
> === modified file 'lisp/doc-view.el'
> --- lisp/doc-view.el  2010-02-16 14:35:45 +0000
> +++ lisp/doc-view.el  2010-07-13 21:19:42 +0000
> @@ -1349,8 +1349,8 @@
>  
>  ;;;; Bookmark integration
>  
> -(declare-function bookmark-make-record-default "bookmark"
> -                  (&optional point-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  
>
> === modified file 'lisp/gnus/ChangeLog'
> --- lisp/gnus/ChangeLog       2010-06-22 16:48:53 +0000
> +++ lisp/gnus/ChangeLog       2010-07-13 21:19:42 +0000
> @@ -1,3 +1,8 @@
> +2010-07-13  Karl Fogel  <address@hidden>
> +
> +     * gnus/gnus-sum.el (bookmark-make-record-default): Adjust
> +     declaration, based on changes in bookmark.el.
> +
>  2010-06-22  Mark A. Hershberger  <address@hidden>
>  
>       * mm-url.el (mm-url-encode-multipart-form-data): New function to handle
>
> === modified file 'lisp/gnus/gnus-sum.el'
> --- lisp/gnus/gnus-sum.el     2010-06-10 00:30:13 +0000
> +++ lisp/gnus/gnus-sum.el     2010-07-13 21:19:42 +0000
> @@ -12621,7 +12621,8 @@
>      (gnus-summary-position-point)))
>  
>  ;;; Bookmark support for Gnus.
> -(declare-function bookmark-make-record-default "bookmark" (&optional 
> pos-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  (declare-function bookmark-get-bookmark-record "bookmark" (bmk))
>
> === modified file 'lisp/image-mode.el'
> --- lisp/image-mode.el        2010-06-14 03:19:46 +0000
> +++ lisp/image-mode.el        2010-07-13 21:19:42 +0000
> @@ -516,8 +516,8 @@
>  
>  
>  ;;; Support for bookmark.el
> -(declare-function bookmark-make-record-default "bookmark"
> -                  (&optional point-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  
>
> === modified file 'lisp/info.el'
> --- lisp/info.el      2010-06-17 20:56:17 +0000
> +++ lisp/info.el      2010-07-13 21:19:42 +0000
> @@ -4901,7 +4901,8 @@
>            '(Info-mode . Info-restore-desktop-buffer))
>  
>  ;;;; Bookmark support
> -(declare-function bookmark-make-record-default "bookmark" (&optional 
> pos-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  (declare-function bookmark-get-bookmark-record "bookmark" (bmk))
> @@ -4910,7 +4911,7 @@
>    "This implements the `bookmark-make-record-function' type (which see)
>  for Info nodes."
>    `(,Info-current-node
> -    ,@(bookmark-make-record-default 'point-only)
> +    ,@(bookmark-make-record-default 'no-file)
>      (filename . ,Info-current-file)
>      (info-node . ,Info-current-node)
>      (handler . Info-bookmark-jump)))
>
> === modified file 'lisp/man.el'
> --- lisp/man.el       2010-06-01 02:34:49 +0000
> +++ lisp/man.el       2010-07-13 21:19:42 +0000
> @@ -1674,7 +1674,8 @@
>      complete-path))
>  
>  ;;; Bookmark Man Support
> -(declare-function bookmark-make-record-default "bookmark" (&optional 
> pos-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  (declare-function bookmark-get-bookmark-record "bookmark" (bmk))
> @@ -1691,7 +1692,7 @@
>  (defun Man-bookmark-make-record ()
>    "Make a bookmark entry for a Man buffer."
>    `(,(Man-default-bookmark-title)
> -    ,@(bookmark-make-record-default 'point-only)
> +    ,@(bookmark-make-record-default 'no-file)
>      (location . ,(concat "man " Man-arguments))
>      (man-args . ,Man-arguments)
>      (handler . Man-bookmark-jump)))
>
> === modified file 'lisp/woman.el'
> --- lisp/woman.el     2010-05-25 02:11:08 +0000
> +++ lisp/woman.el     2010-07-13 21:19:42 +0000
> @@ -4521,7 +4521,8 @@
>    nil)                                       ; for woman-file-readable-p etc.
>  
>  ;;; Bookmark Woman support.
> -(declare-function bookmark-make-record-default "bookmark" (&optional 
> pos-only))
> +(declare-function bookmark-make-record-default
> +                  "bookmark" (&optional no-file no-context posn))
>  (declare-function bookmark-prop-get "bookmark" (bookmark prop))
>  (declare-function bookmark-default-handler "bookmark" (bmk))
>  (declare-function bookmark-get-bookmark-record "bookmark" (bmk))
> @@ -4532,7 +4533,7 @@
>  (defun woman-bookmark-make-record ()
>    "Make a bookmark entry for a Woman buffer."
>    `(,(Man-default-bookmark-title)
> -    ,@(bookmark-make-record-default 'point-only)
> +    ,@(bookmark-make-record-default 'no-file)
>      (location . ,(concat "woman " woman-last-file-name))
>      ;; Use the same form as man's bookmarks, as much as possible.
>      (man-args . ,woman-last-file-name)
>
>
>

-- 
Thierry Volpiatto
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997