all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
@ 2018-06-23 16:22 João Távora
  2018-06-24 13:37 ` Noam Postavsky
  0 siblings, 1 reply; 8+ messages in thread
From: João Távora @ 2018-06-23 16:22 UTC (permalink / raw)
  To: 31951

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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-23 16:22 bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate João Távora
@ 2018-06-24 13:37 ` Noam Postavsky
  2018-06-24 20:22   ` João Távora
  0 siblings, 1 reply; 8+ messages in thread
From: Noam Postavsky @ 2018-06-24 13:37 UTC (permalink / raw)
  To: João Távora; +Cc: 31951

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)






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-24 13:37 ` Noam Postavsky
@ 2018-06-24 20:22   ` João Távora
  2018-06-24 20:46     ` Noam Postavsky
  0 siblings, 1 reply; 8+ messages in thread
From: João Távora @ 2018-06-24 20:22 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31951

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






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-24 20:22   ` João Távora
@ 2018-06-24 20:46     ` Noam Postavsky
  2018-06-25 11:07       ` João Távora
  0 siblings, 1 reply; 8+ messages in thread
From: Noam Postavsky @ 2018-06-24 20:46 UTC (permalink / raw)
  To: João Távora; +Cc: 31951

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-24 20:46     ` Noam Postavsky
@ 2018-06-25 11:07       ` João Távora
  2018-06-26  1:53         ` Noam Postavsky
  0 siblings, 1 reply; 8+ messages in thread
From: João Távora @ 2018-06-25 11:07 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31951

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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-25 11:07       ` João Távora
@ 2018-06-26  1:53         ` Noam Postavsky
  2018-06-26 19:27           ` João Távora
  2018-06-27 13:20           ` João Távora
  0 siblings, 2 replies; 8+ messages in thread
From: Noam Postavsky @ 2018-06-26  1:53 UTC (permalink / raw)
  To: João Távora; +Cc: 31951

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






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-26  1:53         ` Noam Postavsky
@ 2018-06-26 19:27           ` João Távora
  2018-06-27 13:20           ` João Távora
  1 sibling, 0 replies; 8+ messages in thread
From: João Távora @ 2018-06-26 19:27 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31951

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate
  2018-06-26  1:53         ` Noam Postavsky
  2018-06-26 19:27           ` João Távora
@ 2018-06-27 13:20           ` João Távora
  1 sibling, 0 replies; 8+ messages in thread
From: João Távora @ 2018-06-27 13:20 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 31951, 31951-done

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






^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-06-27 13:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-23 16:22 bug#31951: 27.0.50; [PATCH] server-save-buffers-kill-terminal should respect save-some-buffers-default-predicate João Távora
2018-06-24 13:37 ` Noam Postavsky
2018-06-24 20:22   ` João Távora
2018-06-24 20:46     ` Noam Postavsky
2018-06-25 11:07       ` João Távora
2018-06-26  1:53         ` Noam Postavsky
2018-06-26 19:27           ` João Távora
2018-06-27 13:20           ` João Távora

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.