GNU bug report logs - #31951
27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate

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 31951 in the body.
You can then email your comments to 31951 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: João Távora <joaotavora <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Sat, 23 Jun 2018 17:22:48 +0100

Hi,

If the Emacs client was started without an explicit list of buffers to
edit, save-some-buffers is called with t for PRED (save all buffers),
but that was before save-some-buffers-default-predicate existed.

I don't see any reason why save-some-buffers-default-predicate shouldn't
be respected in server-save-buffers-kill-terminal (of course if ARG is
non-nil, we do pass t so that the previous behaviour remains).

Trivial patch attached,
João

diff --git a/lisp/server.el b/lisp/server.el
index ff03cbe622..ac14ef314e 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1670,9 +1670,9 @@ server-save-buffers-kill-terminal
 	     ;; and offer to save all buffers.  Otherwise, offer to
 	     ;; save only the buffers belonging to the client.
 	     (save-some-buffers
-	      arg (if buffers
-		      (lambda () (memq (current-buffer) buffers))
-		    t))
+	      arg (and buffers
+		       (lambda () (memq (current-buffer) buffers))
+                       (and arg t)))
 	     (server-delete-client proc)))
 	  (t (error "Invalid client frame")))))

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Sun, 24 Jun 2018 09:37:26 -0400

João Távora <joaotavora <at> gmail.com> writes:

> If the Emacs client was started without an explicit list of buffers to
> edit, save-some-buffers is called with t for PRED (save all buffers),
> but that was before save-some-buffers-default-predicate existed.
>
> I don't see any reason why save-some-buffers-default-predicate shouldn't
> be respected in server-save-buffers-kill-terminal (of course if ARG is
> non-nil, we do pass t so that the previous behaviour remains).

>	     ;; If client is bufferless, emulate a normal Emacs exit
>	     ;; and offer to save all buffers.  Otherwise, offer to
>	     ;; save only the buffers belonging to the client.
>  	     (save-some-buffers
> -	      arg (if buffers
> -		      (lambda () (memq (current-buffer) buffers))
> -		    t))
> +	      arg (and buffers
> +		       (lambda () (memq (current-buffer) buffers))
> +                       (and arg t)))

I think you meant

+	      arg (if buffers
+		      (lambda () (memq (current-buffer) buffers))
+                   (and arg t))

But I'm not sure if this change makes sense, seeing as
save-buffers-kill-emacs also ignores
save-some-buffers-default-predicate:

    (defun save-buffers-kill-emacs (&optional arg)
      [...]
      ;; Don't use save-some-buffers-default-predicate, because we want
      ;; to ask about all the buffers before killing Emacs.
      (save-some-buffers arg t)

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

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Sun, 24 Jun 2018 21:22:48 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> If the Emacs client was started without an explicit list of buffers to
>> edit, save-some-buffers is called with t for PRED (save all buffers),
>> but that was before save-some-buffers-default-predicate existed.
>>
>> I don't see any reason why save-some-buffers-default-predicate shouldn't
>> be respected in server-save-buffers-kill-terminal (of course if ARG is
>> non-nil, we do pass t so that the previous behaviour remains).

> I think you meant
>
> +	      arg (if buffers
> +		      (lambda () (memq (current-buffer) buffers))
> +                   (and arg t))

Yes, of course.

> But I'm not sure if this change makes sense, seeing as
> save-buffers-kill-emacs also ignores
> save-some-buffers-default-predicate:
>
>     (defun save-buffers-kill-emacs (&optional arg)
>       [...]
>       ;; Don't use save-some-buffers-default-predicate, because we want
>       ;; to ask about all the buffers before killing Emacs.
>       (save-some-buffers arg t)

Right, but thats because, when killing Emacs, it is really the last
chance to save those buffers before they're potentially gone for good.

In contrast, almost since server-save-buffers-kill-terminal's inception,
it has had a provision for saving only a subset of buffers, by guessing
that the user's intention was to edit only the buffers passed to the
emacsclient command.  This, of course, may very well not be true, but
it's a good thing in my opinion.  So I think it's natural that it also
respects save-some-buffers-default-predicate, which is also designed to
loosen these conditions, but doesn't guess, because it requires
customization by the user.

Alternatively, we could have some
save-some-buffers-default-strict-predicate for the PRED=t situation, but
I think that's overenginneering it.  Of course I'm open to other ideas.
How would you make quitting a client terminal not bother you about those
pesky ChangeLog buffers?

João

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Sun, 24 Jun 2018 16:46:02 -0400

João Távora <joaotavora <at> gmail.com> writes:

>>     (defun save-buffers-kill-emacs (&optional arg)
>>       [...]
>>       ;; Don't use save-some-buffers-default-predicate, because we want
>>       ;; to ask about all the buffers before killing Emacs.
>>       (save-some-buffers arg t)
>
> Right, but thats because, when killing Emacs, it is really the last
> chance to save those buffers before they're potentially gone for good.

Oh, I got mixed up by the comment in server-save-buffers-kill-terminal:

	     ;; If client is bufferless, emulate a normal Emacs exit
	     ;; and offer to save all buffers.  Otherwise, offer to
	     ;; save only the buffers belonging to the client.

So I'd say your change is sensible, we should just update that comment
to explain the differences in the "emulation".

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

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Mon, 25 Jun 2018 12:07:26 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>>>     (defun save-buffers-kill-emacs (&optional arg)
>>>       [...]
>>>       ;; Don't use save-some-buffers-default-predicate, because we want
>>>       ;; to ask about all the buffers before killing Emacs.
>>>       (save-some-buffers arg t)
>>
>> Right, but thats because, when killing Emacs, it is really the last
>> chance to save those buffers before they're potentially gone for good.
>
> Oh, I got mixed up by the comment in server-save-buffers-kill-terminal:
>
> 	     ;; If client is bufferless, emulate a normal Emacs exit
> 	     ;; and offer to save all buffers.  Otherwise, offer to
> 	     ;; save only the buffers belonging to the client.
>
> So I'd say your change is sensible, we should just update that comment
> to explain the differences in the "emulation".

Hmmmm OK, what about:

diff --git a/lisp/server.el b/lisp/server.el
index ff03cbe622..33e88409ca 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1666,13 +1666,16 @@ server-save-buffers-kill-terminal
 	     (save-buffers-kill-emacs arg)))
 	  ((processp proc)
 	   (let ((buffers (process-get proc 'buffers)))
-	     ;; If client is bufferless, emulate a normal Emacs exit
-	     ;; and offer to save all buffers.  Otherwise, offer to
-	     ;; save only the buffers belonging to the client.
+             ;; If the client has buffers, offer to save only those
+             ;; buffers.  Otherwise, this is similar to a normal Emacs
+             ;; exit (where we offer to save all buffers) with the
+             ;; exception that, if ARG is nil, then passing nil as the
+             ;; PRED argument to `save-some-buffers' ensures
+             ;; `save-some-buffers-default-predicate' is respected.
 	     (save-some-buffers
 	      arg (if buffers
 		      (lambda () (memq (current-buffer) buffers))
-		    t))
+                    (and arg t)))
 	     (server-delete-client proc)))
 	  (t (error "Invalid client frame")))))


I tried to make it terser, but couldn't.  If the indentation looks off,
it's not, it's because I untabified the changed regions.

If nobody complains I'll push this in a few days' time.

João

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Mon, 25 Jun 2018 21:53:45 -0400

João Távora <joaotavora <at> gmail.com> writes:

> I tried to make it terser, but couldn't.

Hmm, maybe just concentrate on the save-some-buffers-default-predicate
thing.  I think the original comment pretty much redundantly repeats
what the doc string (and the code itself) say.

--- i/lisp/server.el
+++ w/lisp/server.el
@@ -1639,13 +1639,15 @@ server-save-buffers-kill-terminal
 	     (save-buffers-kill-emacs arg)))
 	  ((processp proc)
 	   (let ((buffers (process-get proc 'buffers)))
-	     ;; If client is bufferless, emulate a normal Emacs exit
-	     ;; and offer to save all buffers.  Otherwise, offer to
-	     ;; save only the buffers belonging to the client.
 	     (save-some-buffers
 	      arg (if buffers
-		      (lambda () (memq (current-buffer) buffers))
-		    t))
+                      ;; Only files from emacsclient file list.
+                      (lambda () (memq (current-buffer) buffers))
+                    ;; No emacsclient file list: don't override
+                    ;; `save-some-buffers-default-predicate' (unless
+                    ;; ARGS is non-nil), since we're not killing
+                    ;; Emacs (unlike `save-buffers-kill-emacs').
+                    (and arg t)))
 	     (server-delete-client proc)))
 	  (t (error "Invalid client frame")))))
 

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

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Tue, 26 Jun 2018 20:27:26 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> I tried to make it terser, but couldn't.
>
> Hmm, maybe just concentrate on the save-some-buffers-default-predicate
> thing.  I think the original comment pretty much redundantly repeats
> what the doc string (and the code itself) say.

Heh, I don't think your version is much better (or much worse).  But
since I'm a fan of sharing responsibility, let's go with yours :-)

João

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

From: João Távora <joaotavora <at> gmail.com>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31951-done <at> debbugs.gnu.org, 31951 <at> debbugs.gnu.org
Subject: Re: bug#31951: 27.0.50;
 [PATCH] server-save-buffers-kill-terminal should respect
 save-some-buffers-default-predicate
Date: Wed, 27 Jun 2018 14:20:54 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>
>> I tried to make it terser, but couldn't.
>
> Hmm, maybe just concentrate on the save-some-buffers-default-predicate
> thing.  I think the original comment pretty much redundantly repeats
> what the doc string (and the code itself) say.
>
> --- i/lisp/server.el
> +++ w/lisp/server.el
> @@ -1639,13 +1639,15 @@ server-save-buffers-kill-terminal
>  	     (save-buffers-kill-emacs arg)))
>  	  ((processp proc)
>  	   (let ((buffers (process-get proc 'buffers)))
> -	     ;; If client is bufferless, emulate a normal Emacs exit
> -	     ;; and offer to save all buffers.  Otherwise, offer to
> -	     ;; save only the buffers belonging to the client.
>  	     (save-some-buffers
>  	      arg (if buffers
> -		      (lambda () (memq (current-buffer) buffers))
> -		    t))
> +                      ;; Only files from emacsclient file list.
> +                      (lambda () (memq (current-buffer) buffers))
> +                    ;; No emacsclient file list: don't override
> +                    ;; `save-some-buffers-default-predicate' (unless
> +                    ;; ARGS is non-nil), since we're not killing
> +                    ;; Emacs (unlike `save-buffers-kill-emacs').
> +                    (and arg t)))
>  	     (server-delete-client proc)))
>  	  (t (error "Invalid client frame")))))
>  


Pushed as ce54573dacaeb234ac006b71cbaafe1c543515f1.

Thanks,
João

Previous Next

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