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

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

[Alfred M. Szmidt]

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))

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

[Lars Ingebrigtsen]

"Alfred M. Szmidt" <ams@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

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

[Alfred M. Szmidt]

   "Alfred M. Szmidt" <ams@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 ...  :-)

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

[Lars Ingebrigtsen]

"Alfred M. Szmidt" <ams@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