bug#33669: 26.1; Buffer-menu-mode bad UX

bug#33669: 26.1; Buffer-menu-mode bad UX

[Devon Sean McCullough]

When Buffer-menu-execute asks

	Buffer foo modified; kill anyway? (yes or no)

and the reply is no,
that line is removed anyway, deceiving the user
by making the buffer menu incorrect.

The user will promptly seek out such buffers — but they disappeared,
giving the distressing wrong impression that they were killed anyway.

Better keep the buffer menu accurate and show a helpful message like

	7 buffers killed, 1 modified buffer kept

to maintain harmony.

		Peace
			—Devon

    Je vous écris une longue lettre parce que je n'ai pas le temps d'en écrire une courte.
    	Les provinciales (1656) de Blaise Pascal

P.S. Sorry for not including a patch.

In GNU Emacs 26.1 (build 1, x86_64-apple-darwin14.5.0, NS appkit-1348.17 Version 10.10.5 (Build 14F2511))
 of 2018-05-31 built on builder10-10.porkrind.org
Windowing system distributor 'Apple', version 10.3.1404
Recent messages:
Saving file /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf...
Renaming /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf to /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf.~2~...done
Tramp: Encoding local file ‘/var/folders/jv/9lctn91j0xqfv3f2kxc1lxx80000gn/T/tramp.dDYlW2.conf’ using ‘base64-encode-region’...done
Tramp: Decoding remote file ‘/ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf’ using ‘base64 -d -i >%s’...done
Wrote /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf
Saving file /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf...
Tramp: Encoding local file ‘/var/folders/jv/9lctn91j0xqfv3f2kxc1lxx80000gn/T/tramp.QgPFNM.conf’ using ‘base64-encode-region’...done
Tramp: Decoding remote file ‘/ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf’ using ‘base64 -d -i >%s’...done
Wrote /ssh:pi@RaspberryPi.local|sudo:root@RaspberryPi.local:/etc/wpa_supplicant/wpa_supplicant.conf
(New file)

Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES THREADS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-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
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq rfc822 mml mml-sec
epa derived epg epg-config gnus-util rmail rmail-loaddefs mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tramp-cmds cl-print debug help-fns radix-tree help-mode easymenu
sh-script smie executable conf-mode view misearch multi-isearch
dired-aux files-x tramp-cache tramp-sh tramp tramp-compat tramp-loaddefs
trampver shell pcomplete comint ansi-color ring parse-time format-spec
advice auth-source cl-seq eieio byte-opt bytecomp byte-compile cconv
eieio-core cl-macs gv eieio-loaddefs cl-loaddefs cl-lib password-cache
dired dired-loaddefs elec-pair time-date tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame 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 minibuffer 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 kqueue cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 246729 19322)
 (symbols 48 22835 1)
 (miscs 40 132 409)
 (strings 32 42302 1687)
 (string-bytes 1 1185010)
 (vectors 16 43513)
 (vector-slots 8 1207050 59552)
 (floats 8 71 383)
 (intervals 56 2095 247)
 (buffers 992 30))

bug#33669: 26.1; Buffer-menu-mode bad UX

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2018@jovi.net>
> Date: Sat, 8 Dec 2018 10:06:48 +0800
> 
> When Buffer-menu-execute asks
> 
> 	Buffer foo modified; kill anyway? (yes or no)
> 
> and the reply is no,
> that line is removed anyway, deceiving the user
> by making the buffer menu incorrect.
> 
> The user will promptly seek out such buffers — but they disappeared,
> giving the distressing wrong impression that they were killed anyway.

Thanks for reporting this.  Does the patch below give good results?

diff --git a/lisp/buff-menu.el b/lisp/buff-menu.el
index bc1288a..95746b3 100644
--- a/lisp/buff-menu.el
+++ b/lisp/buff-menu.el
@@ -476,9 +476,10 @@ Buffer-menu-execute
 			 (tabulated-list-set-col 2 " " t))
 		     (error (warn "Error saving %s" buffer))))
 		 (if delete
-		     (unless (eq buffer (current-buffer))
-		       (kill-buffer buffer)
-		       (tabulated-list-delete-entry))
+		     (if (and (not (eq buffer (current-buffer)))
+                              (kill-buffer buffer))
+                         (tabulated-list-delete-entry)
+                       (forward-line 1))
 		   (forward-line 1)))))))))
 
 (defun Buffer-menu-select ()

bug#33669: 26.1; Buffer-menu-mode bad UX

[Eli Zaretskii]

Ping!  Devon, could you please try the proposed fix?

> Date: Sat, 08 Dec 2018 13:07:42 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 33669@debbugs.gnu.org
> 
> > From: Devon Sean McCullough <Emacs-Hacker2018@jovi.net>
> > Date: Sat, 8 Dec 2018 10:06:48 +0800
> > 
> > When Buffer-menu-execute asks
> > 
> > 	Buffer foo modified; kill anyway? (yes or no)
> > 
> > and the reply is no,
> > that line is removed anyway, deceiving the user
> > by making the buffer menu incorrect.
> > 
> > The user will promptly seek out such buffers — but they disappeared,
> > giving the distressing wrong impression that they were killed anyway.
> 
> Thanks for reporting this.  Does the patch below give good results?
> 
> diff --git a/lisp/buff-menu.el b/lisp/buff-menu.el
> index bc1288a..95746b3 100644
> --- a/lisp/buff-menu.el
> +++ b/lisp/buff-menu.el
> @@ -476,9 +476,10 @@ Buffer-menu-execute
>  			 (tabulated-list-set-col 2 " " t))
>  		     (error (warn "Error saving %s" buffer))))
>  		 (if delete
> -		     (unless (eq buffer (current-buffer))
> -		       (kill-buffer buffer)
> -		       (tabulated-list-delete-entry))
> +		     (if (and (not (eq buffer (current-buffer)))
> +                              (kill-buffer buffer))
> +                         (tabulated-list-delete-entry)
> +                       (forward-line 1))
>  		   (forward-line 1)))))))))
>  
>  (defun Buffer-menu-select ()
> 
> 
> 
> 

bug#33669: 26.1; Buffer-menu-mode bad UX

[Eli Zaretskii]

Ping!  Ping!  Devon, could you please try the proposed fix?

> Date: Sat, 22 Dec 2018 12:50:10 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 33669@debbugs.gnu.org
> 
> Ping!  Devon, could you please try the proposed fix?
> 
> > Date: Sat, 08 Dec 2018 13:07:42 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: 33669@debbugs.gnu.org
> > 
> > > From: Devon Sean McCullough <Emacs-Hacker2018@jovi.net>
> > > Date: Sat, 8 Dec 2018 10:06:48 +0800
> > > 
> > > When Buffer-menu-execute asks
> > > 
> > > 	Buffer foo modified; kill anyway? (yes or no)
> > > 
> > > and the reply is no,
> > > that line is removed anyway, deceiving the user
> > > by making the buffer menu incorrect.
> > > 
> > > The user will promptly seek out such buffers — but they disappeared,
> > > giving the distressing wrong impression that they were killed anyway.
> > 
> > Thanks for reporting this.  Does the patch below give good results?
> > 
> > diff --git a/lisp/buff-menu.el b/lisp/buff-menu.el
> > index bc1288a..95746b3 100644
> > --- a/lisp/buff-menu.el
> > +++ b/lisp/buff-menu.el
> > @@ -476,9 +476,10 @@ Buffer-menu-execute
> >  			 (tabulated-list-set-col 2 " " t))
> >  		     (error (warn "Error saving %s" buffer))))
> >  		 (if delete
> > -		     (unless (eq buffer (current-buffer))
> > -		       (kill-buffer buffer)
> > -		       (tabulated-list-delete-entry))
> > +		     (if (and (not (eq buffer (current-buffer)))
> > +                              (kill-buffer buffer))
> > +                         (tabulated-list-delete-entry)
> > +                       (forward-line 1))
> >  		   (forward-line 1)))))))))
> >  
> >  (defun Buffer-menu-select ()
> > 
> > 
> > 
> > 
> 
> 
> 
> 

bug#33669: 26.1; Buffer-menu-mode bad UX

[Eli Zaretskii]

> From: Devon Sean McCullough <Emacs-Hacker2018@jovi.net>
> Date: Sat, 5 Jan 2019 10:36:22 -0500
> 
> > On Jan 5, 2019, at 3:44 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Ping!  Ping!  Devon, could you please try the proposed fix?
> 
> The original and patched versions both look problematic.  Instead, I had good results with
> 
> --- old/buff-menu.el	2019-01-05 10:26:45.000000000 -0500
> +++ new/buff-menu.el	2019-01-05 10:27:23.000000000 -0500
> @@ -475,10 +475,10 @@
>  			   (save-buffer))
>  			 (tabulated-list-set-col 2 " " t))
>  		     (error (warn "Error saving %s" buffer))))
> -		 (if delete
> -		     (unless (eq buffer (current-buffer))
> -		       (kill-buffer buffer)
> -		       (tabulated-list-delete-entry))
> +		 (if (and delete
> +			  (not (eq buffer (current-buffer)))
> +                          (kill-buffer buffer))
> +                     (tabulated-list-delete-entry)
>  		   (forward-line 1)))))))))
>  
>  (defun Buffer-menu-select ()
> 

Thanks, pushed to the emacs-26 branch.