GNU bug report logs - #11605
24.1.50; vc-ediff revert annoyance

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11605 in the body.
You can then email your comments to 11605 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: William Xu <william.xwl <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.1.50; vc-ediff revert annoyance
Date: Sat, 2 Jun 2012 12:31:10 +0800

[Message part 1 (text/plain, inline)]

Steps to reproduce:

  1. emacs -Q
  2. visit a version controlled file, say `foo'.
  3. M-x vc-ediff, run it couple of times.

You will see this annoying message many times:

  File foo.~master~ changed on disk.  Reread from disk? (yes or no)

The reason is the ~master~ file is checked out everytime we run
vc-ediff, and after ediff session, the ~master~ buffer is always left
there, hence, when running vc-ediff multiple times it constantly asks
you whether you want to revert.  Why not let it revert automatically?

How about this patch?

-- 
William

http://xwl.appspot.com

[0001-vc-find-revision-Auto-revert-file-revision-buffer-qu.patch (application/octet-stream, attachment)]

Message #10 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: William Xu <william.xwl <at> gmail.com>
Cc: 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Wed, 24 Feb 2016 17:33:44 +1100

William Xu <william.xwl <at> gmail.com> writes:

>   1. emacs -Q
>   2. visit a version controlled file, say `foo'.
>   3. M-x vc-ediff, run it couple of times.
>
> You will see this annoying message many times:
>
>   File foo.~master~ changed on disk.  Reread from disk? (yes or no)

I can confirm that this bug is still present in the Emacs trunk.

> The reason is the ~master~ file is checked out everytime we run
> vc-ediff, and after ediff session, the ~master~ buffer is always left
> there, hence, when running vc-ediff multiple times it constantly asks
> you whether you want to revert.  Why not let it revert automatically?
>
> How about this patch?

[...]

I think the patch sounds reasonable, but the vc machinery is something
with which I'm not overly familiar.

Could somebody take a peek at this patch?

> From d2af0f4dcc81b39d9360135244d6870388a264a3 Mon Sep 17 00:00:00 2001
> From: William Xu <william.xwl <at> gmail.com>
> Date: Sat, 2 Jun 2012 12:16:45 +0800
> Subject: [PATCH] (vc-find-revision): Auto revert file revision buffer
>  quietly.
>
> ---
>  lisp/vc/vc.el |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 87e4e1c..bd337d0 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -1815,11 +1815,13 @@ Use BACKEND as the VC backend if specified."
>  		(delete-file filename))))
>  	  (vc-mode-line file))
>  	(message "Checking out %s...done" filename)))
> -    (let ((result-buf (find-file-noselect filename)))
> +    (let ((result-buf (or (get-file-buffer filename)
> +                          (find-file-noselect filename))))
>        (with-current-buffer result-buf
>  	;; Set the parent buffer so that things like
>  	;; C-x v g, C-x v l, ... etc work.
> -	(set (make-local-variable 'vc-parent-buffer) filebuf))
> +	(set (make-local-variable 'vc-parent-buffer) filebuf)
> +        (revert-buffer nil t))
>        result-buf)))
>  
>  ;; Header-insertion code

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #13 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, William Xu <william.xwl <at> gmail.com>
Cc: 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Fri, 26 Feb 2016 02:38:35 +0200

On 02/24/2016 08:33 AM, Lars Ingebrigtsen wrote:

>> @@ -1815,11 +1815,13 @@ Use BACKEND as the VC backend if specified."
>>  		(delete-file filename))))
>>  	  (vc-mode-line file))
>>  	(message "Checking out %s...done" filename)))
>> -    (let ((result-buf (find-file-noselect filename)))
>> +    (let ((result-buf (or (get-file-buffer filename)
>> +                          (find-file-noselect filename))))

Doesn't find-file-noselect call get-file-buffer anyway?

>>        (with-current-buffer result-buf
>>  	;; Set the parent buffer so that things like
>>  	;; C-x v g, C-x v l, ... etc work.
>> -	(set (make-local-variable 'vc-parent-buffer) filebuf))
>> +	(set (make-local-variable 'vc-parent-buffer) filebuf)
>> +        (revert-buffer nil t))

It seems like this might conflict with other uses of vc-find-revision, 
like vc-revision-other-window. Where the user is allowed to change the 
contents of the returned buffer, and might've done so before we do this 
automatic silent revert.

Maybe do it on ediff's side instead?

Message #16 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: William Xu <william.xwl <at> gmail.com>, 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Fri, 26 Feb 2016 16:31:47 +1030

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 02/24/2016 08:33 AM, Lars Ingebrigtsen wrote:
>
>>> @@ -1815,11 +1815,13 @@ Use BACKEND as the VC backend if specified."
>>>  		(delete-file filename))))
>>>  	  (vc-mode-line file))
>>>  	(message "Checking out %s...done" filename)))
>>> -    (let ((result-buf (find-file-noselect filename)))
>>> +    (let ((result-buf (or (get-file-buffer filename)
>>> +                          (find-file-noselect filename))))
>
> Doesn't find-file-noselect call get-file-buffer anyway?

It does, but then it goes into all the "File %s changed on disk.  Reread
from disk?" stuff, which is what the bug is about.  But, I mean, the
file may have changed, so...

>>>        (with-current-buffer result-buf
>>>  	;; Set the parent buffer so that things like
>>>  	;; C-x v g, C-x v l, ... etc work.
>>> -	(set (make-local-variable 'vc-parent-buffer) filebuf))
>>> +	(set (make-local-variable 'vc-parent-buffer) filebuf)
>>> +        (revert-buffer nil t))
>
> It seems like this might conflict with other uses of vc-find-revision,
> like vc-revision-other-window. Where the user is allowed to change the
> contents of the returned buffer, and might've done so before we do
> this automatic silent revert.
>
> Maybe do it on ediff's side instead?

If the buffer with the comparison file had been killed before all this
had happened, we wouldn't have gotten the "file changed on disk" thing,
yes...  That may be a better fix.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #19 received at submit <at> debbugs.gnu.org (full text, mbox):

From: William Xu <william.xwl <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Fri, 26 Feb 2016 10:38:49 +0100

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> It seems like this might conflict with other uses of vc-find-revision,
> like vc-revision-other-window. Where the user is allowed to change the
> contents of the returned buffer, and might've done so before we do
> this automatic silent revert.

It sounds strange that one would try to modify it.  I thought the
checkout version is only for comparison.  The file name XX.~yy~ looks
also very similar to automatic backup files, e.g., "!tmp!foo.cpp.~1~".

-- 
William

My idea of roughing it turning the air conditioner too low.

Message #22 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: William Xu <william.xwl <at> gmail.com>, 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Fri, 26 Feb 2016 12:10:21 +0200

On 02/26/2016 11:38 AM, William Xu wrote:

> It sounds strange that one would try to modify it.  I thought the
> checkout version is only for comparison.  The file name XX.~yy~ looks
> also very similar to automatic backup files, e.g., "!tmp!foo.cpp.~1~".

I agree it is strange, but it's not out of the question. Maybe the user 
wants to restore an old version of a function, and on a whim, started 
retouching it in the "old version" buffer?

Message #25 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: William Xu <william.xwl <at> gmail.com>, 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Fri, 26 Feb 2016 12:36:18 +0200

On 02/26/2016 08:01 AM, Lars Ingebrigtsen wrote:

> If the buffer with the comparison file had been killed before all this
> had happened, we wouldn't have gotten the "file changed on disk" thing,
> yes...  That may be a better fix.

Maybe so. But I'm not sure it's easy to compute the exact buffer name, 
to kill it.

Or disable the automatic prompting somehow (add a NOWARN argument to 
vc-find-revision?), and call revert-buffer from ediff after.

Message #28 received at 11605 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: William Xu <william.xwl <at> gmail.com>, 11605 <at> debbugs.gnu.org
Subject: Re: bug#11605: 24.1.50; vc-ediff revert annoyance
Date: Thu, 27 Jun 2019 17:16:21 +0200

Dmitry Gutov <dgutov <at> yandex.ru> writes:

>>>        (with-current-buffer result-buf
>>>  	;; Set the parent buffer so that things like
>>>  	;; C-x v g, C-x v l, ... etc work.
>>> -	(set (make-local-variable 'vc-parent-buffer) filebuf))
>>> +	(set (make-local-variable 'vc-parent-buffer) filebuf)
>>> +        (revert-buffer nil t))
>
> It seems like this might conflict with other uses of vc-find-revision,
> like vc-revision-other-window. Where the user is allowed to change the
> contents of the returned buffer, and might've done so before we do
> this automatic silent revert.
>
> Maybe do it on ediff's side instead?

Yeah, after poking at this a bit more, I agree -- ediff should set up
this better, and just calling revert in the vc functions does not seem
to be the logical choice.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #35 received at 11605-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 11605-done <at> debbugs.gnu.org
Subject: Fixed on master
Date: Tue, 04 Mar 2025 12:00:19 +0800

Version 31.1

I've installed a patch fixing this.

-- 
Sean Whitton

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.