GNU bug report logs - #55310
27.2; vc-revert: unhelpful error message when modified buffers exist

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 55310 in the body.
You can then email your comments to 55310 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: "Alfred M. Szmidt" <ams <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.2; vc-revert: unhelpful error message when modified buffers exist
Date: Sun, 08 May 2022 03:41:44 -0400

When doing vc-revert in a vc-dir buffer, and when one has a bunch of
open files, one somtimes gets the unhelpful message:

  vc-revert: Please kill or save all modified buffers before reverting

Nicer would be prompting the user to kill/save those buffers, or list them.


In GNU Emacs 27.2 (build 1, x86_64-unknown-openbsd, GTK+ Version 3.24.33)
 of 2022-04-09 built on amd64.ports.openbsd.org
System Description: OpenBSD nitrogenium.mendeleev 7.1 GENERIC.MP#465 amd64

Recent messages:
  INFO     Scraping files for loaddefs.el...done
Loading /home/ams/loaddefs.el (source)...done
Loading /home/ams/quicklisp/slime-helper.el (source)...done
Loading /home/ams/quicklisp/clhs-use-local.el (source)...done
Loading /home/ams/private/emacs-nitrogenium.mendeleev...done
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --build=amd64-unknown-openbsd --without-sound
 --with-x-toolkit=gtk3 --prefix=/usr/local --sysconfdir=/etc
 --mandir=/usr/local/man --infodir=/usr/local/info
 --localstatedir=/var --disable-silent-rules --disable-gtk-doc
 'CFLAGS=-O2 -pipe -g' CPPFLAGS=-I/usr/local/include
 'LDFLAGS=-L/usr/local/lib -g''

Configured features:
XPM JPEG TIFF GIF PNG RSVG DBUS GSETTINGS GLIB NOTIFY KQUEUE GNUTLS
LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 XDBE XIM MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  locale-coding-system: nil

Major mode: Fundamental

Minor modes in effect:
  global-company-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  transient-mark-mode: t

Load-path shadows:
~/loaddefs hides /usr/local/share/emacs/27.2/lisp/loaddefs

Features:
(shadow mailalias emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec epa epg epg-config gnus-util rmail
rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader term/screen
term/xterm xterm rcirc time-date mail-queue sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils slime-asdf grep
slime-quicklisp slime-fancy slime-indentation slime-cl-indent
cl-indent slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree slime-scratch
slime-presentations advice bridge slime-macrostep macrostep
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl elp slime-parse slime derived cl-extra help-mode gud
apropos compile arc-mode archive-mode noutline outline easy-mmode pp
comint ansi-color hyperspec thingatpt slime-autoloads company-oddmuse
company-keywords company-etags etags fileloop generator xref project
ring company-gtags company-dabbrev-code company-dabbrev company-files
company-capf company-cmake company-xcode company-clang
company-semantic company-eclim company-template company-bbdb company
edmacro kmacro pcase cal-menu calendar cal-loaddefs autoload
radix-tree lisp-mnt finder-inf disp-table package easymenu browse-url
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese
eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese composite charscript charprop case-table
epa-hook jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind kqueue lcms2
dynamic-setting system-font-setting font-render-setting move-toolbar
gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 190926 10037)
 (symbols 48 15424 2)
 (strings 32 47013 758)
 (string-bytes 1 1409610)
 (vectors 16 17009)
 (vector-slots 8 200499 8642)
 (floats 8 62 187)
 (intervals 56 260 129)
 (buffers 1000 12))

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Alfred M. Szmidt" <ams <at> gnu.org>
Cc: 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Sun, 08 May 2022 13:44:45 +0200

"Alfred M. Szmidt" <ams <at> gnu.org> writes:

> When doing vc-revert in a vc-dir buffer, and when one has a bunch of
> open files, one somtimes gets the unhelpful message:
>
>   vc-revert: Please kill or save all modified buffers before reverting
>
> Nicer would be prompting the user to kill/save those buffers, or list them.

I don't think we want to have an interface that offers to kill buffers
in a loop -- it sounds like something that's really error-prone, which
is why that code is the way it is, I think.  (To make the user make the
decision themselves explicitly.)

We could do this, of course:

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 3508f684c4..cb5e42db4c 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2780,6 +2780,10 @@ vc-revert
     ;; show the changes and ask for confirmation to discard them.
     (when (or (not files) (memq (buffer-file-name) files))
       (vc-buffer-sync nil))
+    ;; Offer to save all the buffers we're reverting.
+    (save-some-buffers
+     nil (lambda ()
+           (member (buffer-file-name) files)))
     (dolist (file files)
       (let ((buf (get-file-buffer file)))
 	(when (and buf (buffer-modified-p buf))


But if the user answers "no", then it'll just signal an error anyway, so
that's just confusing.

So I think leaving it the way it is is the best option here, since this
command is one of the most potentially destructive ones we have in
Emacs.  Anybody have an opinion here?

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

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

From: "Alfred M. Szmidt" <ams <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Sun, 08 May 2022 10:22:58 -0400

   "Alfred M. Szmidt" <ams <at> gnu.org> writes:

   > When doing vc-revert in a vc-dir buffer, and when one has a bunch of
   > open files, one somtimes gets the unhelpful message:
   >
   >   vc-revert: Please kill or save all modified buffers before reverting
   >
   > Nicer would be prompting the user to kill/save those buffers, or list them.

   I don't think we want to have an interface that offers to kill buffers
   in a loop -- it sounds like something that's really error-prone, which
   is why that code is the way it is, I think.  (To make the user make the
   decision themselves explicitly.)

We already support such mechanism I think in several other places,
like ibuffer, etc.   


My main problem is really that it is impossible to know _which_
buffers are modified, if you have several hundred open in several
different projects -- in addition to the fact that the error message
is just wrong ("all buffers" -- it is just some buffers, that are
marked).

Idea (ideas are cheap): maybe if one could add some sort of
high-light, or something in vc-dired that shows that this or that file
has a open buffer that is unmodified, and then the error could be the
same but just saying that one should check the highlighted buffers.


   So I think leaving it the way it is is the best option here, since this
   command is one of the most potentially destructive ones we have in
   Emacs.  Anybody have an opinion here?

The destructiveness of vc-revert also depends on the version control
system, fossil provides a undo mechanism.  But doing delete on files
in dired is far worse ...  :-)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Alfred M. Szmidt" <ams <at> gnu.org>
Cc: 55310 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Mon, 09 May 2022 11:38:26 +0200

"Alfred M. Szmidt" <ams <at> gnu.org> writes:

>    I don't think we want to have an interface that offers to kill buffers
>    in a loop -- it sounds like something that's really error-prone, which
>    is why that code is the way it is, I think.  (To make the user make the
>    decision themselves explicitly.)
>
> We already support such mechanism I think in several other places,
> like ibuffer, etc.   

Sure, but that's a mode to list and act on buffers -- having vc-revert
kill buffers (even after querying the user) would be surprising.

> My main problem is really that it is impossible to know _which_
> buffers are modified, if you have several hundred open in several
> different projects -- in addition to the fact that the error message
> is just wrong ("all buffers" -- it is just some buffers, that are
> marked).

Yes, it would be nice if it actually said which buffers it's talking
about.

> Idea (ideas are cheap): maybe if one could add some sort of
> high-light, or something in vc-dired that shows that this or that file
> has a open buffer that is unmodified, and then the error could be the
> same but just saying that one should check the highlighted buffers.

I think that's a good idea -- perhaps Dmitry has some comments; added to
the CCs.

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

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: "Alfred M. Szmidt" <ams <at> gnu.org>,  55310 <at> debbugs.gnu.org,  Dmitry Gutov
 <dgutov <at> yandex.ru>
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Mon, 10 Mar 2025 15:06:30 +0800

Hello,

On Mon 09 May 2022 at 11:38am +02, Lars Ingebrigtsen wrote:

> "Alfred M. Szmidt" <ams <at> gnu.org> writes:
>
>>    I don't think we want to have an interface that offers to kill buffers
>>    in a loop -- it sounds like something that's really error-prone, which
>>    is why that code is the way it is, I think.  (To make the user make the
>>    decision themselves explicitly.)
>>
>> We already support such mechanism I think in several other places,
>> like ibuffer, etc.
>
> Sure, but that's a mode to list and act on buffers -- having vc-revert
> kill buffers (even after querying the user) would be surprising.

Currently C-x v u from an individual file's buffer does prompt you to
save it, so it would be good to extend it to this case in VC-Dir.

>> My main problem is really that it is impossible to know _which_
>> buffers are modified, if you have several hundred open in several
>> different projects -- in addition to the fact that the error message
>> is just wrong ("all buffers" -- it is just some buffers, that are
>> marked).
>
> Yes, it would be nice if it actually said which buffers it's talking
> about.
>
>> Idea (ideas are cheap): maybe if one could add some sort of
>> high-light, or something in vc-dired that shows that this or that file
>> has a open buffer that is unmodified, and then the error could be the
>> same but just saying that one should check the highlighted buffers.
>
> I think that's a good idea -- perhaps Dmitry has some comments; added to
> the CCs.

ISTM that a project-save-some-buffers is what's wanted here.

Dmitry, has there been a discussion about having something like that?

-- 
Sean Whitton

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, "Alfred M. Szmidt"
 <ams <at> gnu.org>, 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when modified
 buffers exist
Date: Wed, 12 Mar 2025 10:11:45 +0200

On 10/03/2025 09:06, Sean Whitton wrote:
>>> Idea (ideas are cheap): maybe if one could add some sort of
>>> high-light, or something in vc-dired that shows that this or that file
>>> has a open buffer that is unmodified, and then the error could be the
>>> same but just saying that one should check the highlighted buffers.
>> I think that's a good idea -- perhaps Dmitry has some comments; added to
>> the CCs.
> ISTM that a project-save-some-buffers is what's wanted here.
> 
> Dmitry, has there been a discussion about having something like that?

Last time this came up, a new value for 
save-some-buffers-default-predicate was added: save-some-buffers-root 
(bug#46374). So:

  (setopt save-some-buffers-default-predicate #'save-some-buffers-root)

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>, "Alfred M. Szmidt" <ams <at> gnu.org>
Cc: 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Fri, 14 Mar 2025 11:37:13 +0800

Hello,

On Wed 12 Mar 2025 at 10:11am +02, Dmitry Gutov wrote:

> On 10/03/2025 09:06, Sean Whitton wrote:
>>>> Idea (ideas are cheap): maybe if one could add some sort of
>>>> high-light, or something in vc-dired that shows that this or that file
>>>> has a open buffer that is unmodified, and then the error could be the
>>>> same but just saying that one should check the highlighted buffers.
>>> I think that's a good idea -- perhaps Dmitry has some comments; added to
>>> the CCs.
>> ISTM that a project-save-some-buffers is what's wanted here.
>> Dmitry, has there been a discussion about having something like that?
>
> Last time this came up, a new value for save-some-buffers-default-predicate
> was added: save-some-buffers-root (bug#46374). So:
>
>   (setopt save-some-buffers-default-predicate #'save-some-buffers-root)

Ah, thanks for the reference.

I would like to do one of the following things:

(1) Do add the call to save-some-buffers in Lars's message.
    We can change the error message to
      "Some unsaved buffers remain; cannot revert"
    and then I don't agree with him that the situation is confusing.

(2) Add a project-save-some-buffers which just binds
    save-some-buffers-default-predicate around a call to
    save-some-buffers, bind is to 'C-x p C-x s' (useful anyway, I think)
    and change the message to suggest using it:
      "Use C-x p C-x s to save or kill modified buffers before reverting"

WDYT?

-- 
Sean Whitton

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, "Alfred M. Szmidt" <ams <at> gnu.org>
Cc: 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when modified
 buffers exist
Date: Sat, 15 Mar 2025 04:18:59 +0200

On 14/03/2025 05:37, Sean Whitton wrote:
> Hello,
> 
> On Wed 12 Mar 2025 at 10:11am +02, Dmitry Gutov wrote:
> 
>> On 10/03/2025 09:06, Sean Whitton wrote:
>>>>> Idea (ideas are cheap): maybe if one could add some sort of
>>>>> high-light, or something in vc-dired that shows that this or that file
>>>>> has a open buffer that is unmodified, and then the error could be the
>>>>> same but just saying that one should check the highlighted buffers.
>>>> I think that's a good idea -- perhaps Dmitry has some comments; added to
>>>> the CCs.
>>> ISTM that a project-save-some-buffers is what's wanted here.
>>> Dmitry, has there been a discussion about having something like that?
>>
>> Last time this came up, a new value for save-some-buffers-default-predicate
>> was added: save-some-buffers-root (bug#46374). So:
>>
>>    (setopt save-some-buffers-default-predicate #'save-some-buffers-root)
> 
> Ah, thanks for the reference.
> 
> I would like to do one of the following things:
> 
> (1) Do add the call to save-some-buffers in Lars's message.
>      We can change the error message to
>        "Some unsaved buffers remain; cannot revert"
>      and then I don't agree with him that the situation is confusing.

Re-reading the discussion again, maybe Lars's original suggestion - from 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55310#8 - is better?

This way we'd only require that the buffers to be reverted, are saved. 
And abort otherwise.

Also probably change the error to something like

  "Please kill or save all affected buffers before reverting"

If we can't print all the buffer names, which we also could try, though.

Requiring all buffers across the project (never mind the session) to be 
saved before any can be vc-reverted, would be a more disruptive change.

> (2) Add a project-save-some-buffers which just binds
>      save-some-buffers-default-predicate around a call to
>      save-some-buffers, bind is to 'C-x p C-x s' (useful anyway, I think)
>      and change the message to suggest using it:
>        "Use C-x p C-x s to save or kill modified buffers before reverting"

Adding a new command with a binding sounds good to me, irrespective of 
this issue.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: "Alfred M. Szmidt" <ams <at> gnu.org>, 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Sat, 15 Mar 2025 15:37:19 +0800

Hello,

On Sat 15 Mar 2025 at 04:18am +02, Dmitry Gutov wrote:

> Re-reading the discussion again, maybe Lars's original suggestion - from
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55310#8 - is better?

Yes, this is exactly what I was suggesting :)  I'll go ahead and do that.

> This way we'd only require that the buffers to be reverted, are saved. And
> abort otherwise.
>
> Also probably change the error to something like
>
>   "Please kill or save all affected buffers before reverting"
>
> If we can't print all the buffer names, which we also could try, though.
>
> Requiring all buffers across the project (never mind the session) to be saved
> before any can be vc-reverted, would be a more disruptive change.
>
>> (2) Add a project-save-some-buffers which just binds
>>      save-some-buffers-default-predicate around a call to
>>      save-some-buffers, bind is to 'C-x p C-x s' (useful anyway, I think)
>>      and change the message to suggest using it:
>>        "Use C-x p C-x s to save or kill modified buffers before reverting"
>
> Adding a new command with a binding sounds good to me, irrespective of this
> issue.

Okay, I'll send you a patch.

-- 
Sean Whitton

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: "Alfred M. Szmidt" <ams <at> gnu.org>, control <at> debbugs.gnu.org,
 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Sat, 15 Mar 2025 16:04:53 +0800

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

tag 55310 + patch
thanks

Hello again Dmitry,

Okay, how do these look?

-- 
Sean Whitton

[0001-vc-revert-Offer-to-save-modified-buffers-bug-55310.patch (text/x-diff, attachment)]

[0002-New-project-save-some-buffers-command.patch (text/x-diff, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: "Alfred M. Szmidt" <ams <at> gnu.org>, control <at> debbugs.gnu.org,
 55310 <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when modified
 buffers exist
Date: Sat, 15 Mar 2025 22:18:10 +0200

Hi Sean,

On 15/03/2025 09:37, Sean Whitton wrote:
> Yes, this is exactly what I was suggesting 🙂  I'll go ahead and do that.

Ah perfect. Sorry, guess I worked off the context in the GGGP email.

On 15/03/2025 10:04, Sean Whitton wrote:
> Hello again Dmitry,
> 
> Okay, how do these look?

Looking good, thanks.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 55310-done <at> debbugs.gnu.org
Subject: Re: bug#55310: 27.2; vc-revert: unhelpful error message when
 modified buffers exist
Date: Sun, 16 Mar 2025 11:33:45 +0800

Version: 31.1

Hello,

On Sat 15 Mar 2025 at 10:18pm +02, Dmitry Gutov wrote:

> On 15/03/2025 10:04, Sean Whitton wrote:
>> Hello again Dmitry,
>> Okay, how do these look?
>
> Looking good, thanks.

Thanks for looking them over.  Installed and closing the bug.

-- 
Sean Whitton

Previous Next

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