* patch: vc.el annotation prev/next "in place"
@ 2006-04-05 21:15 Thien-Thi Nguyen
2006-04-05 22:04 ` Stefan Monnier
2006-04-05 22:37 ` Kevin Rodgers
0 siblings, 2 replies; 6+ messages in thread
From: Thien-Thi Nguyen @ 2006-04-05 21:15 UTC (permalink / raw)
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: patch: vc.el annotation prev/next "in place"
2006-04-05 21:15 patch: vc.el annotation prev/next "in place" Thien-Thi Nguyen
@ 2006-04-05 22:04 ` Stefan Monnier
2006-04-05 22:37 ` Kevin Rodgers
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2006-04-05 22:04 UTC (permalink / raw)
Cc: emacs-devel
> ! (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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: patch: vc.el annotation prev/next "in place"
2006-04-05 21:15 patch: vc.el annotation prev/next "in place" Thien-Thi Nguyen
2006-04-05 22:04 ` Stefan Monnier
@ 2006-04-05 22:37 ` Kevin Rodgers
2006-04-11 20:29 ` Stefan Monnier
2006-04-13 16:07 ` Thien-Thi Nguyen
1 sibling, 2 replies; 6+ messages in thread
From: Kevin Rodgers @ 2006-04-05 22:37 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: patch: vc.el annotation prev/next "in place"
2006-04-05 22:37 ` Kevin Rodgers
@ 2006-04-11 20:29 ` Stefan Monnier
2006-04-13 16:07 ` Thien-Thi Nguyen
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2006-04-11 20:29 UTC (permalink / raw)
Cc: emacs-devel
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: patch: vc.el annotation prev/next "in place"
2006-04-05 22:37 ` Kevin Rodgers
2006-04-11 20:29 ` Stefan Monnier
@ 2006-04-13 16:07 ` Thien-Thi Nguyen
2006-04-13 16:26 ` Romain Francoise
1 sibling, 1 reply; 6+ messages in thread
From: Thien-Thi Nguyen @ 2006-04-13 16:07 UTC (permalink / raw)
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-04-13 16:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-05 21:15 patch: vc.el annotation prev/next "in place" Thien-Thi Nguyen
2006-04-05 22:04 ` Stefan Monnier
2006-04-05 22:37 ` Kevin Rodgers
2006-04-11 20:29 ` Stefan Monnier
2006-04-13 16:07 ` Thien-Thi Nguyen
2006-04-13 16:26 ` Romain Francoise
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).