patch: vc.el annotation prev/next "in place"

patch: vc.el annotation prev/next "in place"

[Thien-Thi Nguyen]

i month or so ago, i complained about the P and N (and J) commands in
vc-annotate buffer ending up (selecting) another buffer/window.  the
following patch reverts the old behavior, according to my tastes, but
i suspect there is a better way, by avoiding `switch-to-buffer', for
example.  what do you think?

thi

______________________________
Index: vc.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc.el,v
retrieving revision 1.414
diff -w -b -B -c -r1.414 vc.el
*** vc.el	7 Feb 2006 16:59:01 -0000	1.414
--- vc.el	5 Apr 2006 21:05:34 -0000
***************
*** 3076,3083 ****
  				  nil nil "20")))))))
    (vc-ensure-vc-buffer)
    (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
!   (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
!          (temp-buffer-show-function 'vc-annotate-display-select))
      (message "Annotating...")
      ;; If BUF is specified it tells in which buffer we should put the
      ;; annotations.  This is used when switching annotations to another
--- 3076,3087 ----
  				  nil nil "20")))))))
    (vc-ensure-vc-buffer)
    (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
!   (let* ((temp-buffer-setup-hook nil)
!          (temp-buffer-name (format "*Annotate %s (rev %s)*"
!                                    (buffer-name) rev))
!          (temp-buffer-show-function (lambda (buf)
!                                       (switch-to-buffer buf)
!                                       (vc-annotate-display-select))))
      (message "Annotating...")
      ;; If BUF is specified it tells in which buffer we should put the
      ;; annotations.  This is used when switching annotations to another
***************
*** 3086,3101 ****
  	      (rename-buffer temp-buffer-name t)
  	      ;; In case it had to be uniquified.
  	      (setq temp-buffer-name (buffer-name))))
!     (with-output-to-temp-buffer temp-buffer-name
!       (vc-call annotate-command file (get-buffer temp-buffer-name) rev))
      (with-current-buffer temp-buffer-name
        (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
        (set (make-local-variable 'vc-annotate-parent-file) file)
        (set (make-local-variable 'vc-annotate-parent-rev) rev)
        (set (make-local-variable 'vc-annotate-parent-display-mode)
! 	   display-mode))
  
!   (message "Annotating... done")))
  
  (defun vc-annotate-prev-version (prefix)
    "Visit the annotation of the version previous to this one.
--- 3090,3108 ----
                (rename-buffer temp-buffer-name t)
                ;; In case it had to be uniquified.
                (setq temp-buffer-name (buffer-name))))
!     (vc-call annotate-command file (get-buffer-create temp-buffer-name) rev)
!     (switch-to-buffer temp-buffer-name)
!     (delete-other-windows)
!     (vc-annotate-display-select)
!     (goto-char (point-min))
      (with-current-buffer temp-buffer-name
        (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
        (set (make-local-variable 'vc-annotate-parent-file) file)
        (set (make-local-variable 'vc-annotate-parent-rev) rev)
        (set (make-local-variable 'vc-annotate-parent-display-mode)
!            display-mode)))
  
!   (message "Annotating... done"))
  
  (defun vc-annotate-prev-version (prefix)
    "Visit the annotation of the version previous to this one.

Re: patch: vc.el annotation prev/next "in place"

[Stefan Monnier]

> !          (temp-buffer-show-function (lambda (buf)
> !                                       (switch-to-buffer buf)
> !                                       (vc-annotate-display-select))))

Any time you use switch-to-buffer, you have to take into account the fact
that it may signal an error (when running inside a dedicated window).

> !     (vc-call annotate-command file (get-buffer-create temp-buffer-name) rev)
> !     (switch-to-buffer temp-buffer-name)
> !     (delete-other-windows)
> !     (vc-annotate-display-select)
> !     (goto-char (point-min))

Same here, but additionally `delete-other-windows' is clearly wrong
in general.


        Stefan

Re: patch: vc.el annotation prev/next "in place"

[Kevin Rodgers]

Thien-Thi Nguyen wrote:
> i month or so ago, i complained about the P and N (and J) commands in
> vc-annotate buffer ending up (selecting) another buffer/window.  the
> following patch reverts the old behavior, according to my tastes, but
> i suspect there is a better way, by avoiding `switch-to-buffer', for
> example.  what do you think?

vc-annotate binds temp-buffer-show-function to
vc-annotate-display-select in order to display the annotation buffer,
which basically just delegates to vc-annotate-display-default or
-autoscale; those in turn call vc-annotate-display, which depends on
font-lock-mode to do the work.

 From its name we can assume vc-annotate-display-select is supposed to
select the annotation buffer as well as display it, but it doesn't.  In
fact, it doesn't even make sure the current buffer is displayed.

Wouldn't it make more sense if this bit in vc-annotate-display-select:

   (when buffer
     (set-buffer buffer)
     (display-buffer buffer))

were changed to:

   (pop-to-buffer (or buffer (current-buffer)))

> Index: vc.el
> ===================================================================
> RCS file: /sources/emacs/emacs/lisp/vc.el,v
> retrieving revision 1.414
> diff -w -b -B -c -r1.414 vc.el
> *** vc.el	7 Feb 2006 16:59:01 -0000	1.414
> --- vc.el	5 Apr 2006 21:05:34 -0000
> ***************
> *** 3076,3083 ****
>   				  nil nil "20")))))))
>     (vc-ensure-vc-buffer)
>     (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
> !   (let* ((temp-buffer-name (format "*Annotate %s (rev %s)*" (buffer-name) rev))
> !          (temp-buffer-show-function 'vc-annotate-display-select))
>       (message "Annotating...")
>       ;; If BUF is specified it tells in which buffer we should put the
>       ;; annotations.  This is used when switching annotations to another
> --- 3076,3087 ----
>   				  nil nil "20")))))))
>     (vc-ensure-vc-buffer)
>     (setq vc-annotate-display-mode display-mode) ;Not sure why.  --Stef
> !   (let* ((temp-buffer-setup-hook nil)
> !          (temp-buffer-name (format "*Annotate %s (rev %s)*"
> !                                    (buffer-name) rev))
> !          (temp-buffer-show-function (lambda (buf)
> !                                       (switch-to-buffer buf)
> !                                       (vc-annotate-display-select))))
>       (message "Annotating...")
>       ;; If BUF is specified it tells in which buffer we should put the
>       ;; annotations.  This is used when switching annotations to another
> ***************
> *** 3086,3101 ****
>   	      (rename-buffer temp-buffer-name t)
>   	      ;; In case it had to be uniquified.
>   	      (setq temp-buffer-name (buffer-name))))
> !     (with-output-to-temp-buffer temp-buffer-name
> !       (vc-call annotate-command file (get-buffer temp-buffer-name) rev))
>       (with-current-buffer temp-buffer-name
>         (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
>         (set (make-local-variable 'vc-annotate-parent-file) file)
>         (set (make-local-variable 'vc-annotate-parent-rev) rev)
>         (set (make-local-variable 'vc-annotate-parent-display-mode)
> ! 	   display-mode))
>   
> !   (message "Annotating... done")))
>   
>   (defun vc-annotate-prev-version (prefix)
>     "Visit the annotation of the version previous to this one.
> --- 3090,3108 ----
>                 (rename-buffer temp-buffer-name t)
>                 ;; In case it had to be uniquified.
>                 (setq temp-buffer-name (buffer-name))))
> !     (vc-call annotate-command file (get-buffer-create temp-buffer-name) rev)
> !     (switch-to-buffer temp-buffer-name)
> !     (delete-other-windows)
> !     (vc-annotate-display-select)
> !     (goto-char (point-min))
>       (with-current-buffer temp-buffer-name
>         (set (make-local-variable 'vc-annotate-backend) (vc-backend file))
>         (set (make-local-variable 'vc-annotate-parent-file) file)
>         (set (make-local-variable 'vc-annotate-parent-rev) rev)
>         (set (make-local-variable 'vc-annotate-parent-display-mode)
> !            display-mode)))
>   
> !   (message "Annotating... done"))
>   
>   (defun vc-annotate-prev-version (prefix)
>     "Visit the annotation of the version previous to this one.

-- 
Kevin Rodgers

Re: patch: vc.el annotation prev/next "in place"

[Stefan Monnier]

> From its name we can assume vc-annotate-display-select is supposed to
> select the annotation buffer as well as display it, but it doesn't.  In
> fact, it doesn't even make sure the current buffer is displayed.

> Wouldn't it make more sense if this bit in vc-annotate-display-select:

>   (when buffer
>     (set-buffer buffer)
>     (display-buffer buffer))

> were changed to:

>   (pop-to-buffer (or buffer (current-buffer)))

Yes, that'd make sense.


        Stefan

Re: patch: vc.el annotation prev/next "in place"

[Thien-Thi Nguyen]

looks like the sum user-visible effect of the spate of changes since
2005-12-28 is that `vc-annotate' now honors `pop-up-windows', which is
fine, IMHO.

so, to wrap up this thread, i just installed a change to vc-annotate
that makes point end up on the "current line" when BUF is unspecified,
as is the case for `C-x v g' and `C-u C-x v g'.  the behavior is now
similar to that of P, N and J commands (in VC Annotate mode), which use
`vc-annotate-warp-version' (which does its own context preservation).

i suppose another round of factoring would move context preservation
into vc-annotate proper, but that is not user-visible so i'll turn to
other things, now...

thanks everyone for improving vc annotate functionality.

thi

Re: patch: vc.el annotation prev/next "in place"

[Romain Francoise]

Thien-Thi Nguyen <ttn@gnu.org> writes:

> i just installed a change to vc-annotate that makes point end up on
> the "current line" when BUF is unspecified, as is the case for `C-x v
> g' and `C-u C-x v g'.

That's extremely useful.  Many thanks!

-- 
Romain Francoise <romain@orebokech.com> | The sea! the sea! the open
it's a miracle -- http://orebokech.com/ | sea! The blue, the fresh, the
                                        | ever free! --Bryan W. Procter