bug#6447: 23.1; revert-buffer in *vc-diff*

bug#6447: 23.1; revert-buffer in *vc-diff*

[Tom Tromey]

Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list,
and to the gnu.emacs.bug news group.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:


I typed '=' in a *vc-dir* buffer to get a diff.
It would be nice if revert-buffer in the *vc-diff* buffer re-ran the
same diff command.


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
/usr/share/emacs/23.1/etc/DEBUG for instructions.


In GNU Emacs 23.1.1 (i386-redhat-linux-gnu, GTK+ Version 2.16.6)
 of 2010-04-01 on x86-01.phx2.fedoraproject.org
Windowing system distributor `The X.Org Foundation', version 11.0.10603901
configured using `configure  '--build=i386-redhat-linux-gnu' '--host=i386-redhat-linux-gnu' '--target=i586-redhat-linux-gnu' '--program-prefix=' '--prefix=/usr' '--exec-prefix=/usr' '--bindir=/usr/bin' '--sbindir=/usr/sbin' '--sysconfdir=/etc' '--datadir=/usr/share' '--includedir=/usr/include' '--libdir=/usr/lib' '--libexecdir=/usr/libexec' '--localstatedir=/var' '--sharedstatedir=/var/lib' '--mandir=/usr/share/man' '--infodir=/usr/share/info' '--with-dbus' '--with-gif' '--with-jpeg' '--with-png' '--with-rsvg' '--with-tiff' '--with-xft' '--with-xpm' '--with-x-toolkit=gtk' 'build_alias=i386-redhat-linux-gnu' 'host_alias=i386-redhat-linux-gnu' 'target_alias=i586-redhat-linux-gnu' 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-bu
 ffer-size=4 -m32 -march=i586 -mtune=generic -fasynchronous-unwind-tables''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Group

Minor modes in effect:
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-spelling-mode: t
  erc-truncate-mode: t
  gnus-undo-mode: t
  erc-status-mode: t
  erc-services-mode: t
  erc-networks-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
u s t SPC b e c a u s e SPC m y SPC i m p r e s s i 
o n SPC i s SPC t h a t SPC m o s t SPC o f SPC t h 
e SPC g <backspace> t a r g e t SPC s t u f f SPC i 
s SPC r e a l l y SPC q u i t e SPC h o s t - i n d 
e p e n d e n t <return> b u t SPC I SPC h a v e SPC 
n o t SPC a c t u a l l y SPC t r i e s <backspace> 
d SPC s <backspace> a SPC b u i l d SPC o n SPC s o 
l a r i s , SPC s o SPC w h o SPC k n o w s <return> 
C-z n C-s SPC . SPC C-p C-p E E E C-u C-n E E S-SPC 
C-z o C-z o C-z o <f10> <f10> <f10> y e a h <return> 
a n y t h i n g SPC l i k e SPC t h a t SPC i s SPC 
p r <M-backspace> a SPC b u g , SPC b u t SPC o <M-backspace> 
I SPC r e a l i z e SPC t i m e SPC i s SPC l i m i 
t e d . . . <return> <f10> <f10> <f10> C-z n = E E 
S-SPC SPC = E E E S-SPC E E E q s C-u C-n C-n C-n SPC 
E q s M-x r e p o r t - e m <tab> b <tab> <return>

Recent messages:
Deleting article 21076 in mail.gnome
Deleting article 21077 in mail.gnome
Deleting article 21078 in mail.gnome
Deleting article 21079 in mail.gnome
Expiring articles...done
(No changes need to be saved)
Saving /home/tromey/.newsrc.eld...
Saving file /home/tromey/.newsrc.eld...
Wrote /home/tromey/.newsrc.eld
Saving /home/tromey/.newsrc.eld...done

Tom

bug#6447: 23.1; revert-buffer in *vc-diff*

[Juri Linkov]

> I typed '=' in a *vc-dir* buffer to get a diff.
> It would be nice if revert-buffer in the *vc-diff* buffer re-ran the
> same diff command.

Like the command `diff' after calling `diff-mode' sets
`revert-buffer-function' to lambda with its function arguments,
`vc-diff-internal' could do the same:

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
+++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
@@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
           (message "%s" (cdr messages))
           nil)
       (diff-mode)
+      (set (make-local-variable 'revert-buffer-function)
+	   `(lambda (ignore-auto noconfirm)
+	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
       ;; Make the *vc-diff* buffer read only, the diff-mode key
       ;; bindings are nicer for read only buffers. pcl-cvs does the
       ;; same thing.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#6447: 23.1; revert-buffer in *vc-diff*

[Chong Yidong]

Juri Linkov <juri@jurta.org> writes:

> Like the command `diff' after calling `diff-mode' sets
> `revert-buffer-function' to lambda with its function arguments,
> `vc-diff-internal' could do the same:
>
> === modified file 'lisp/vc/vc.el'
> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>            (message "%s" (cdr messages))
>            nil)
>        (diff-mode)
> +      (set (make-local-variable 'revert-buffer-function)
> +	   `(lambda (ignore-auto noconfirm)
> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>        ;; Make the *vc-diff* buffer read only, the diff-mode key
>        ;; bindings are nicer for read only buffers. pcl-cvs does the
>        ;; same thing.

Looks like a fine idea, please go ahead.

bug#6447: 23.1; revert-buffer in *vc-diff*

[Juri Linkov]

>> Like the command `diff' after calling `diff-mode' sets
>> `revert-buffer-function' to lambda with its function arguments,
>> `vc-diff-internal' could do the same:
>>
>> === modified file 'lisp/vc/vc.el'
>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>            (message "%s" (cdr messages))
>>            nil)
>>        (diff-mode)
>> +      (set (make-local-variable 'revert-buffer-function)
>> +	   `(lambda (ignore-auto noconfirm)
>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>        ;; Make the *vc-diff* buffer read only, the diff-mode key
>>        ;; bindings are nicer for read only buffers. pcl-cvs does the
>>        ;; same thing.
>
> Looks like a fine idea, please go ahead.

Done.

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#6447: 23.1; revert-buffer in *vc-diff*

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

>>> Like the command `diff' after calling `diff-mode' sets
>>> `revert-buffer-function' to lambda with its function arguments,
>>> `vc-diff-internal' could do the same:
>>>
>>> === modified file 'lisp/vc/vc.el'
>>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>>            (message "%s" (cdr messages))
>>>            nil)
>>>        (diff-mode)
>>> +      (set (make-local-variable 'revert-buffer-function)
>>> +	   `(lambda (ignore-auto noconfirm)
>>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>>        ;; Make the *vc-diff* buffer read only, the diff-mode key
>>>        ;; bindings are nicer for read only buffers. pcl-cvs does the
>>>        ;; same thing.
>>
>> Looks like a fine idea, please go ahead.
>
> Done.

BTW, the log and log-incoming and log-outgoing buffers could use the same treatment...

bug#6447: 23.1; revert-buffer in *vc-diff*

[Juri Linkov]

> BTW, the log and log-incoming and log-outgoing buffers could use the same treatment...

Do you think it should be implemented in `vc-log-internal-common'?

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#6447: 23.1; revert-buffer in *vc-diff*

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

>> BTW, the log and log-incoming and log-outgoing buffers could use the same treatment...
>
> Do you think it should be implemented in `vc-log-internal-common'?

Probably, that's where the log buffer gets created.  But, IMHO, if you
find a better place, that should be fine too.

bug#6447: 23.1; revert-buffer in *vc-diff*

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

>>> Like the command `diff' after calling `diff-mode' sets
>>> `revert-buffer-function' to lambda with its function arguments,
>>> `vc-diff-internal' could do the same:
>>>
>>> === modified file 'lisp/vc/vc.el'
>>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>>            (message "%s" (cdr messages))
>>>            nil)
>>>        (diff-mode)
>>> +      (set (make-local-variable 'revert-buffer-function)
>>> +	   `(lambda (ignore-auto noconfirm)
>>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>>        ;; Make the *vc-diff* buffer read only, the diff-mode key
>>>        ;; bindings are nicer for read only buffers. pcl-cvs does the
>>>        ;; same thing.
>>
>> Looks like a fine idea, please go ahead.
>
> Done.

It would be nice to also have revert-buffer bound to "g" in this case,
that would make this feature easier to use (and consistent with what
we do for other modes).

bug#6447: 23.1; revert-buffer in *vc-diff*

[Stefan Monnier]

>>>> Like the command `diff' after calling `diff-mode' sets
>>>> `revert-buffer-function' to lambda with its function arguments,
>>>> `vc-diff-internal' could do the same:
>>>> 
>>>> === modified file 'lisp/vc/vc.el'
>>>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>>>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>>>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>>> (message "%s" (cdr messages))
>>>> nil)
>>>> (diff-mode)
>>>> +      (set (make-local-variable 'revert-buffer-function)
>>>> +	   `(lambda (ignore-auto noconfirm)
>>>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>>> ;; Make the *vc-diff* buffer read only, the diff-mode key
>>>> ;; bindings are nicer for read only buffers. pcl-cvs does the
>>>> ;; same thing.
>>> 
>>> Looks like a fine idea, please go ahead.
>> 
>> Done.

> It would be nice to also have revert-buffer bound to "g" in this case,
> that would make this feature easier to use (and consistent with what
> we do for other modes).

Fine by me (tho only when the diff-mode buffer is read-only, of course,
it would be rather awkward for editing otherwise).


        Stefan

bug#6447: 23.1; revert-buffer in *vc-diff*

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>>> Like the command `diff' after calling `diff-mode' sets
>>>>> `revert-buffer-function' to lambda with its function arguments,
>>>>> `vc-diff-internal' could do the same:
>>>>> 
>>>>> === modified file 'lisp/vc/vc.el'
>>>>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>>>>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>>>>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>>>> (message "%s" (cdr messages))
>>>>> nil)
>>>>> (diff-mode)
>>>>> +      (set (make-local-variable 'revert-buffer-function)
>>>>> +	   `(lambda (ignore-auto noconfirm)
>>>>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>>>> ;; Make the *vc-diff* buffer read only, the diff-mode key
>>>>> ;; bindings are nicer for read only buffers. pcl-cvs does the
>>>>> ;; same thing.
>>>> 
>>>> Looks like a fine idea, please go ahead.
>>> 
>>> Done.
>
>> It would be nice to also have revert-buffer bound to "g" in this case,
>> that would make this feature easier to use (and consistent with what
>> we do for other modes).
>
> Fine by me (tho only when the diff-mode buffer is read-only, of course,
> it would be rather awkward for editing otherwise).

Done.
Should this also go in the menu?
q is also missing from the menu.

bug#6447: 23.1; revert-buffer in *vc-diff*

[Dan Nicolaescu]

Dan Nicolaescu <dann@gnu.org> writes:

> Juri Linkov <juri@jurta.org> writes:
>
>>>> Like the command `diff' after calling `diff-mode' sets
>>>> `revert-buffer-function' to lambda with its function arguments,
>>>> `vc-diff-internal' could do the same:
>>>>
>>>> === modified file 'lisp/vc/vc.el'
>>>> --- lisp/vc/vc.el	2010-06-11 19:09:57 +0000
>>>> +++ lisp/vc/vc.el	2010-06-19 14:51:14 +0000
>>>> @@ -1547,6 +1547,9 @@ (defun vc-diff-internal (async vc-filese
>>>>            (message "%s" (cdr messages))
>>>>            nil)
>>>>        (diff-mode)
>>>> +      (set (make-local-variable 'revert-buffer-function)
>>>> +	   `(lambda (ignore-auto noconfirm)
>>>> +	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
>>>>        ;; Make the *vc-diff* buffer read only, the diff-mode key
>>>>        ;; bindings are nicer for read only buffers. pcl-cvs does the
>>>>        ;; same thing.
>>>
>>> Looks like a fine idea, please go ahead.
>>
>> Done.
>
> BTW, the log and log-incoming and log-outgoing buffers could use the same treatment...

I've done this implemented this in my local tree.