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

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

[João Távora]

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

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

[Noam Postavsky]

João Távora <joaotavora@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)

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

[João Távora]

Noam Postavsky <npostavs@gmail.com> writes:

> João Távora <joaotavora@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

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

[Noam Postavsky]

João Távora <joaotavora@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".

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

[João Távora]

Noam Postavsky <npostavs@gmail.com> writes:

> João Távora <joaotavora@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

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

[Noam Postavsky]

João Távora <joaotavora@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")))))
 

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

[João Távora]

Noam Postavsky <npostavs@gmail.com> writes:

> João Távora <joaotavora@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

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

[João Távora]

Noam Postavsky <npostavs@gmail.com> writes:

> João Távora <joaotavora@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