bug#8927: 24.0.50; Flaw in C-x v u

bug#8927: 24.0.50; Flaw in C-x v u

[Richard Stallman]

I did C-x v = to see the changes in a file, then reverted the file
with C-x v u so I could install and commit those changes one by one.

Unfortunately, C-x v u deleted the *vc-diff* buffer and made this fail.

C-x v u should not delete the *vc-diff* buffer.



In GNU Emacs 24.0.50.1 (mips64el-unknown-linux-gnu, GTK+ Version 2.12.12)
 of 2011-06-10 on theobromine2
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: Mail

Minor modes in effect:
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  gpm-mouse-mode: t
  display-battery-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent input:
SPC v SPC - SPC DEL DEL = SPC t o SPC s e e SPC t h 
e SPC c h a n g e s SPC i n SPC a SPC f i l e , SPC 
t h e n SPC r e v e r t e d RET t h e SPC f i l e SPC 
w i t h SPC C - x SPC v SPC u SPC s o SPC I SPC c o 
u l d SPC i n s t a l l SPC a m d SPC DEL DEL DEL n 
d SPC c o m m i t SPC t h o s e SPC c h a n g e s RET 
o n e SPC b y SPC o n e . ESC q RET RET U n f o r t 
u n a t e l y , SPC C - x SPC v SPC u SPC d e l e t 
e d SPC t h e SPC * v c - d i f f * SPC b u f f e r 
SPC a n d SPC m a d e SPC t h i s SPC f a i l . RET 
RET C - x SPC v SPC u SPC s h o u l d SPC n o t SPC 
d e l e t e SPC t h e SPC * v c - d i f f * SPC b u 
f f e r . C-a C-p C-p C-p C-p C-p C-@ C-u C-n C-n C-n 
ESC w C-x m C-g ESC x r e o o r t DEL DEL DEL DEL p 
o r t SPC e m a c s TAB RET

Recent messages:
Quit
Mark set
There are unresolved conflicts in this file
Mark set
Saving file /home/rms/emacs-bzr/trunk/lisp/ChangeLog...
Wrote /home/rms/emacs-bzr/trunk/lisp/ChangeLog
Type C-h for help.
Quit [2 times]
Auto-saving...done
Mark set
Quit

Load-path shadows:
None found.

Features:
(shadow emacsbug smerge-mode ansi-color diff expect rmailedit mh-e
mh-compat mh-acros cl mh-buffers mh-loaddefs gnus gnus-ems nnheader
gnus-util time-date wid-edit cal-move cal-menu calendar cal-loaddefs
mule-util log-edit pcvs-util add-log parse-time vc-cvs sgml-mode
ispell derived byte-opt bytecomp byte-compile cconv macroexp debug
compare-w whitespace diff-mode easy-mmode vc ediff-merg ediff-diff
ediff-wind ediff-help ediff-util ediff-mult ediff-init ediff
vc-dispatcher dired-aux vc-bzr find-func help-fns warnings ind-util
rmailout kmacro quail help-mode view dabbrev shell pcomplete grep
compile multi-isearch newcomment qp mailalias rmailmm message sendmail
format-spec mml mml-sec mm-decode mm-bodies mm-encode mailabbrev
gmm-utils mailheader mail-parse rfc2231 rmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils dired t-mouse battery paren
cus-start cus-load mailcrypt rfc822 comint regexp-opt ring easymenu
mailcrypt-init tooltip ediff-hook vc-hooks lisp-float-type mwheel
x-win x-dnd tool-bar dnd fontset image fringe lisp-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer
loaddefs button faces cus-face files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind
dynamic-setting system-font-setting font-render-setting move-toolbar
gtk x-toolkit x multi-tty emacs)


-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#8927: 24.0.50; Flaw in C-x v u

[Glenn Morris]

Richard Stallman wrote:

> I did C-x v = to see the changes in a file, then reverted the file
> with C-x v u so I could install and commit those changes one by one.
>
> Unfortunately, C-x v u deleted the *vc-diff* buffer and made this fail.
>
> C-x v u should not delete the *vc-diff* buffer.


On a related note, it has always annoyed me that vc-revert
unconditionally always shows me the diff of the file in question before
letting me revert. Basically every time I use vc-revert, I have already
checked the diff beforehand, so all this does is introduce an annoying
delay when the backend is slow to produce a diff. I'm not aware of a
command-line VCS that show you the diff when you run its revert command,
so I don't see why Emacs should do so.

Anyway, that's why it kills your *vc-diff* buffer - because it assumes
it has just created it. The fix for your problem would presumably be for
vc-revert to only kill the buffer if it did not exist beforehand.

But personally I still wish it would not always insist on showing me the
diff.

bug#8927: 24.0.50; Flaw in C-x v u

[Richard Stallman]

    Anyway, that's why it kills your *vc-diff* buffer - because it assumes
    it has just created it. The fix for your problem would presumably be for
    vc-revert to only kill the buffer if it did not exist beforehand.

C-x v u should preserve the previous contents of *vc-diff*.


-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#8927: 24.0.50; Flaw in C-x v u

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     Anyway, that's why it kills your *vc-diff* buffer - because it assumes
>     it has just created it. The fix for your problem would presumably be for
>     vc-revert to only kill the buffer if it did not exist beforehand.
>
> C-x v u should preserve the previous contents of *vc-diff*.

I've checked in a fix to avoid clobbering an existing vc-diff buffer
when vc-revert shows a diff temporarily.

I agree with Glenn, though, that showing a vc-diff buffer for this
operation is not a good idea.  Apart from the problems he mentioned, the
diff is not useful anyway, since there is no way to scroll the window
during the yes-nor-no prompt.  I added a new option vc-revert-show-diff
to control whether vc-revert should show a diff, and made it default to
nil.

bug#8927: 24.0.50; Flaw in C-x v u

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     Anyway, that's why it kills your *vc-diff* buffer - because it assumes
>>     it has just created it. The fix for your problem would presumably be for
>>     vc-revert to only kill the buffer if it did not exist beforehand.
>>
>> C-x v u should preserve the previous contents of *vc-diff*.
>
> I've checked in a fix to avoid clobbering an existing vc-diff buffer
> when vc-revert shows a diff temporarily.
>
> I agree with Glenn, though, that showing a vc-diff buffer for this
> operation is not a good idea.  Apart from the problems he mentioned, the
> diff is not useful anyway, since there is no way to scroll the window
> during the yes-nor-no prompt.  I added a new option vc-revert-show-diff
> to control whether vc-revert should show a diff, and made it default to
> nil.

I strongly disagree with this change of a long standing behavior.
Showing the diff is an good visual aid in checking that the user indeed
intends to revert that file.  It's not that hard to type C-x v u in the
wrong window if one has many windows opened, and the files have similar
names.  
C-x v u is destructive, it's easy to lose data, so it seems that it
would be much better to err on the side of caution.

Can't RMS' original complaint be solved by making vc-revert use a 
*vc revert diff* buffer instead of just *vc diff*?

bug#8927: 24.0.50; Flaw in C-x v u

[Romain Francoise]

Dan Nicolaescu <dann@gnu.org> writes:

> I strongly disagree with this change of a long standing behavior.

FWIW, me too.

> Showing the diff is an good visual aid in checking that the user indeed
> intends to revert that file.  It's not that hard to type C-x v u in the
> wrong window if one has many windows opened, and the files have similar
> names.

Yes. And if you use vc-dir, you may not have the file opened at all.

Also, it's not true that the diff buffer cannot be scrolled from the
yes-or-no prompt as Chong mentioned earlier in this thread,
`scroll-other-window' and `scroll-other-window-down' work as expected.

bug#8927: 24.0.50; Flaw in C-x v u

[Chong Yidong]

Romain Francoise <romain@orebokech.com> writes:

> Also, it's not true that the diff buffer cannot be scrolled from the
> yes-or-no prompt as Chong mentioned earlier in this thread,
> `scroll-other-window' and `scroll-other-window-down' work as expected.

Ah yes, forgot about that; I seldom use that functionality.

Since several are opposed, I guess it's wise to keep the old behavior.
If someone feels strongly about a change, we can discuss on emacs-devel.
Otherwise, we at least have an option to prevent the diff from being
shown.

bug#8927: 24.0.50; Flaw in C-x v u

[Thierry Volpiatto]

Chong Yidong <cyd@stupidchicken.com> writes:

> Romain Francoise <romain@orebokech.com> writes:
>
>> Also, it's not true that the diff buffer cannot be scrolled from the
>> yes-or-no prompt as Chong mentioned earlier in this thread,
>> `scroll-other-window' and `scroll-other-window-down' work as expected.
>
> Ah yes, forgot about that; I seldom use that functionality.

Sorry, i don't follow here, for me scrolling-other-window or whatever is
not possible from a yes-or-no-p prompt.
I can do only yes or no (as expected).
Did i miss something?

BTW I have a version ov vc-revert that:

"C-v: scroll-up, M-v: scroll-down, RET:revert, c:cancel"

If interested let me know.

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

bug#8927: 24.0.50; Flaw in C-x v u

[Óscar Fuentes]

Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

>>> Also, it's not true that the diff buffer cannot be scrolled from the
>>> yes-or-no prompt as Chong mentioned earlier in this thread,
>>> `scroll-other-window' and `scroll-other-window-down' work as expected.
>>
>> Ah yes, forgot about that; I seldom use that functionality.
>
> Sorry, i don't follow here, for me scrolling-other-window or whatever is
> not possible from a yes-or-no-p prompt.
> I can do only yes or no (as expected).
> Did i miss something?

scroll-other-window is bound to C-M-v, which works from the minibuffer.

[snip]

bug#8927: 24.0.50; Flaw in C-x v u

[Thierry Volpiatto]

Óscar Fuentes <ofv@wanadoo.es> writes:

> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>
>>>> Also, it's not true that the diff buffer cannot be scrolled from the
>>>> yes-or-no prompt as Chong mentioned earlier in this thread,
>>>> `scroll-other-window' and `scroll-other-window-down' work as expected.
>>>
>>> Ah yes, forgot about that; I seldom use that functionality.
>>
>> Sorry, i don't follow here, for me scrolling-other-window or whatever is
>> not possible from a yes-or-no-p prompt.
>> I can do only yes or no (as expected).
>> Did i miss something?
>
> scroll-other-window is bound to C-M-v, which works from the minibuffer.
I know C-M-v, but it doesn't work from the minibuffer in a y-or-n-p case.
I guess some external library break this because it is working with
emacs -Q.

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

bug#8927: 24.0.50; Flaw in C-x v u

[Thierry Volpiatto]

Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Óscar Fuentes <ofv@wanadoo.es> writes:
>
>> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>>
>>>>> Also, it's not true that the diff buffer cannot be scrolled from the
>>>>> yes-or-no prompt as Chong mentioned earlier in this thread,
>>>>> `scroll-other-window' and `scroll-other-window-down' work as expected.
>>>>
>>>> Ah yes, forgot about that; I seldom use that functionality.
>>>
>>> Sorry, i don't follow here, for me scrolling-other-window or whatever is
>>> not possible from a yes-or-no-p prompt.
>>> I can do only yes or no (as expected).
>>> Did i miss something?
>>
>> scroll-other-window is bound to C-M-v, which works from the minibuffer.
> I know C-M-v, but it doesn't work from the minibuffer in a y-or-n-p case.
> I guess some external library break this because it is working with
> emacs -Q.
This is the culprit:

(fset 'yes-or-no-p 'y-or-n-p)

that was since a long time in my .emacs.

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

bug#8927: 24.0.50; Flaw in C-x v u

[Thierry Volpiatto]

Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:

> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>
>> Óscar Fuentes <ofv@wanadoo.es> writes:
>>
>>> Thierry Volpiatto <thierry.volpiatto@gmail.com> writes:
>>>
>>>>>> Also, it's not true that the diff buffer cannot be scrolled from the
>>>>>> yes-or-no prompt as Chong mentioned earlier in this thread,
>>>>>> `scroll-other-window' and `scroll-other-window-down' work as expected.
>>>>>
>>>>> Ah yes, forgot about that; I seldom use that functionality.
>>>>
>>>> Sorry, i don't follow here, for me scrolling-other-window or whatever is
>>>> not possible from a yes-or-no-p prompt.
>>>> I can do only yes or no (as expected).
>>>> Did i miss something?
>>>
>>> scroll-other-window is bound to C-M-v, which works from the minibuffer.
>> I know C-M-v, but it doesn't work from the minibuffer in a y-or-n-p case.
>> I guess some external library break this because it is working with
>> emacs -Q.
> This is the culprit:
>
> (fset 'yes-or-no-p 'y-or-n-p)
>
> that was since a long time in my .emacs.
NOTE:

yes-or-no-p:
yes-or-no-p is a built-in function in `fns.c'.
It support scrolling

y-or-n-p:
y-or-n-p is a compiled Lisp function in `subr.el'.
It doesn't support scrolling.

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

bug#8927: 24.0.50; Flaw in C-x v u

[Óscar Fuentes]

Chong Yidong <cyd@stupidchicken.com> writes:

>> C-x v u should preserve the previous contents of *vc-diff*.
>
> I've checked in a fix to avoid clobbering an existing vc-diff buffer
> when vc-revert shows a diff temporarily.
>
> I agree with Glenn, though, that showing a vc-diff buffer for this
> operation is not a good idea.  Apart from the problems he mentioned, the
> diff is not useful anyway, since there is no way to scroll the window
> during the yes-nor-no prompt.

C-M-v works fine here, and it is posible to select the other windows
with C-x o or with the mouse while the prompt is active. I'm using an
old version (2010-11-11) maybe yes-or-no changed since?

> I added a new option vc-revert-show-diff to control whether vc-revert
> should show a diff, and made it default to nil.

FWIW, I agree with Dan.

bug#8927: 24.0.50; Flaw in C-x v u

[Richard Stallman]

Thanks.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/

bug#8927: 24.0.50; Flaw in C-x v u

[Stefan Monnier]

> it has just created it. The fix for your problem would presumably be for
> vc-revert to only kill the buffer if it did not exist beforehand.

Right, that's the least we can do.

> But personally I still wish it would not always insist on showing me
> the diff.

I'd agree.  It should still prompt for confirmation, of course, at which
point the user can go get the diff if there's any doubt.


        Stefan

bug#8927: 24.0.50; Flaw in C-x v u

[Richard Stallman]

    > it has just created it. The fix for your problem would presumably be for
    > vc-revert to only kill the buffer if it did not exist beforehand.

    Right, that's the least we can do.

If C-x v u does make and show a diff, it should do that in a
different buffer, so as to preserve the old contents of *vc-diff*.
Keeping that buffer but wiping its contents does not fix the problem.

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use free telephony http://directory.fsf.org/category/tel/