GNU bug report logs - #65719
[PATCH] Pass restart argument to 'kill-emacs-query-functions'

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 65719 in the body.
You can then email your comments to 65719 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: me <at> elken.dev
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Pass restart argument to 'kill-emacs-query-functions'
Date: Sun, 03 Sep 2023 20:28:34 +0100

[Message part 1 (text/plain, inline)]

Hi all,

Small change here to pass the restart status to 
'kill-emacs-query-functions' in 'save-buffers-kill-emacs' so any 
functions there can react to being restarted, sample use-case this may 
be useful is if one is using a package like perspective.el to handle 
buffer/window management and when calling `M-x restart-emacs` you WANT 
your last session to be persisted, but just starting Emacs normally you 
don't.

There are other potential use-cases for this too, so hopefully this 
patch is well-received.

Thanks all,

Ellis (@lkn)

[0001-Add-restart-argument-to-kill-emacs-query-functions.patch (text/x-diff, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: me <at> elken.dev, 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sun, 3 Sep 2023 18:01:28 -0700

me--- via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> Small change here to pass the restart status to
> 'kill-emacs-query-functions' in 'save-buffers-kill-emacs' so any
> functions there can react to being restarted, sample use-case this may
> be useful is if one is using a package like perspective.el to handle
> buffer/window management and when calling `M-x restart-emacs` you WANT
> your last session to be persisted, but just starting Emacs normally you
> don't.
>
> There are other potential use-cases for this too, so hopefully this
> patch is well-received.

Thanks.  The feature sounds useful, and the use-case makes sense.  But
isn't the change backwards-incompatible?  Can we find a way to do this
without any breaking changes?

One idea would be to simply introduce a new variable that takes the
argument, while making the old one obsolete.  That would allow people to
slowly migrate over to the new one, without everything suddenly breaking
under their feet.

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

From: Elken <me <at> elken.dev>
To: Stefan Kangas <stefankangas <at> gmail.com>, 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to 'kill-emacs-query-functions'
Date: Mon, 04 Sep 2023 06:04:25 +0100

[Message part 1 (text/plain, inline)]

Hi Stefan, 

Thanks for the positive feedback. 

I'm not sure that's the case, I'll do more testing locally. If that is the case, then I can introduce 'restart-emacs-query-functions' instead when 'restart' is t for the same outcome, which may actually be a preferable change anyway. 

Thanks, 

Ellis (@lkn)

[Message part 2 (text/html, inline)]

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

From: me <at> elken.dev
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Mon, 04 Sep 2023 06:26:28 +0100

[Message part 1 (text/plain, inline)]

> One idea would be to simply introduce a new variable that takes the
> argument, while making the old one obsolete.  That would allow people 
> to
> slowly migrate over to the new one, without everything suddenly 
> breaking
> under their feet.

How does this patch sound? I think it's the best of both worlds :)

[0001-Introduce-restart-emacs-query-functions.patch (text/x-diff, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: me <at> elken.dev
Cc: 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Fri, 8 Sep 2023 04:37:53 -0700

me <at> elken.dev writes:

>> One idea would be to simply introduce a new variable that takes the
>> argument, while making the old one obsolete.  That would allow people
>> to slowly migrate over to the new one, without everything suddenly
>> breaking under their feet.
>
> How does this patch sound? I think it's the best of both worlds :)

LGTM, but let's see if anyone else has any comments.

> From 7d6f5c259cefe65b5e021dacee18966acc6bc120 Mon Sep 17 00:00:00 2001
> From: Ellis Kenyo <me <at> elken.dev>
> Date: Mon, 4 Sep 2023 06:20:33 +0100
> Subject: [PATCH] Introduce 'restart-emacs-query-functions'
>
> * etc/NEWS: Update NEWS file.
>
> * lisp/files.el (save-buffers-kill-emacs): Add new option
> 'restart-emacs-query-functions' for functions that should be called
> when 'restart' is passed.
> ---
>  etc/NEWS      |  7 +++++++
>  lisp/files.el | 13 +++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index bbf4b67fe34..95e6248c08a 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -776,6 +776,13 @@ the current project.
>  The look of the key prompt in the project switcher has been changed
>  slightly.  To get the previous one, set this option to 'brackets'.
>
> +** Files
> +
> +*** New user option 'restart-emacs-query-functions'.
> +Equivalent to 'kill-emacs-query-functions', this hook is called when
> +'save-buffers-kill-emacs' is called with the 'restart' flag; if
> +invoked from say 'M-x restart-emacs'.
> +
>  
>  * Incompatible Lisp Changes in Emacs 30.1
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 4188615e490..7849d6b7c81 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -8142,6 +8142,15 @@ defcustom kill-emacs-query-functions
>    :version "26.1"
>    :group 'convenience)
>
> +(defcustom restart-emacs-query-functions nil
> +  "Functions to call with no arguments to query about restarting
> +Emacs. If any of these functions returns nil, restarting Emacs is
> +canceled. `save-buffers-kill-emacs' calls these functions when passed
> +`restart'.  See also `kill-emacs-hook'."
> +  :type 'hook
> +  :version "30.1"
> +  :group 'convenience)
> +
>  (defcustom confirm-kill-emacs nil
>    "How to ask for confirmation when leaving Emacs.
>  If nil, the default, don't ask at all.  If the value is non-nil, it should
> @@ -8227,6 +8236,10 @@ defun save-buffers-kill-emacs
>                              (yes-or-no-p "Active processes exist; kill them and exit anyway? "))
>                          (when (window-live-p window)
>                            (quit-restore-window window 'kill)))))))))
> +     ;; Only call these when restarting
> +     (when restart
> +       (run-hook-with-args-until-failure 'restart-emacs-query-functions))
> +
>       ;; Query the user for other things, perhaps.
>       (run-hook-with-args-until-failure 'kill-emacs-query-functions)
>       (or (null confirm)
> --
> 2.41.0

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: me <at> elken.dev
Cc: 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sun, 15 Oct 2023 07:32:34 -0700

me <at> elken.dev writes:

>> One idea would be to simply introduce a new variable that takes the
>> argument, while making the old one obsolete.  That would allow people
>> to
>> slowly migrate over to the new one, without everything suddenly
>> breaking
>> under their feet.
>
> How does this patch sound? I think it's the best of both worlds :)

Thanks, sounds good to me.

Before it can go in, I think it should also be added to the user manual,
next to where `kill-emacs-query-functions' is documented.  And add the
correct "+++" tag to the NEWS entry.

Thanks in advance.

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

From: Ellis Kenyo <me <at> elken.dev>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Mon, 12 Feb 2024 14:54:37 +0000

Apologies for letting this one slip, I must have forgotten to reply.

Some time has passed since so I'll create a new patch in the next few 
hours including that manual change and mail it in :)

Sorry for the delay!

Thanks,

Ellis

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

From: Ellis Kenyo <me <at> elken.dev>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 65719 <at> debbugs.gnu.org
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Mon, 12 Feb 2024 15:29:50 +0000

[Message part 1 (text/plain, inline)]

One patch hot and fresh out the oven :)

Hope we can get this one in!

Thanks,

Ellis

[0001-Introduce-restart-emacs-query-functions.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ellis Kenyo <me <at> elken.dev>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Mon, 12 Feb 2024 18:53:28 +0200

> Cc: 65719 <at> debbugs.gnu.org
> Date: Mon, 12 Feb 2024 15:29:50 +0000
> From:  Ellis Kenyo via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> From 7b18b01b434003e6f25fe8b71d510464b8ac0afd Mon Sep 17 00:00:00 2001
> From: Ellis Kenyo <me <at> elken.dev>
> Date: Mon, 12 Feb 2024 15:25:25 +0000
> Subject: [PATCH] Introduce 'restart-emacs-query-functions'
> 
> * etc/NEWS: Update NEWS file.
> 
> * lisp/files.el (save-buffers-kill-emacs): Add new option
> 'restart-emacs-query-functions' for functions that should be called
> when 'restart' is passed.

Thanks, but can you tell in what cases kill-emacs-query-functions
(which will be called anyway in this case) is not enough?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Elken <me <at> elken.dev>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Mon, 12 Feb 2024 19:35:58 +0200

[Please use Reply All to reply, to keep everybody CC'ed.]

> Date: Mon, 12 Feb 2024 17:21:04 +0000
> From: Elken <me <at> elken.dev>
> 
> The use case that raised it for me was loading perspective workspaces when I restart in place (due to a
> configuration change or other reason) but I don't want to load the workspaces every time I start Emacs.

And why cannot you use kill-emacs-query-functions for this purpose?

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

From: Elken <me <at> elken.dev>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to 'kill-emacs-query-functions'
Date: Mon, 12 Feb 2024 17:52:24 +0000

[Message part 1 (text/plain, inline)]

> but I don't want to load the workspaces every time I start Emacs.

:)

[Message part 2 (text/html, inline)]

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

From: me <at> elken.dev
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Thu, 02 Jan 2025 14:07:33 +0000

On 2024-02-12 17:35, Eli Zaretskii wrote:
> [Please use Reply All to reply, to keep everybody CC'ed.]
> 
>> Date: Mon, 12 Feb 2024 17:21:04 +0000
>> From: Elken <me <at> elken.dev>
>> 
>> The use case that raised it for me was loading perspective workspaces 
>> when I restart in place (due to a
>> configuration change or other reason) but I don't want to load the 
>> workspaces every time I start Emacs.
> 
> And why cannot you use kill-emacs-query-functions for this purpose?

Bumping this patch again as it would be nice to have in. To answer the 
question more clearly (I thought I did before, my apologies) using 
kill-emacs-query-functions would technically work but also add another 
prompt every time I quit or restart. It makes sense to have a clear 
separation of quitting for good and restarting in place.

I also noticed this morning that `kill-emacs` and `restart-emacs` don't 
handle the prefix argument for the hooks. Would you accept either 
another patch for that or an amendment to this patch? The current patch 
I imagine will need to be rebased anyway, so whichever is easier for 
you.

Again apologies for letting this one slip so long.

Thanks all,

Ellis

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> elken.dev
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sat, 04 Jan 2025 15:09:17 +0200

> Date: Thu, 02 Jan 2025 14:07:33 +0000
> From: me <at> elken.dev
> Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> 
> On 2024-02-12 17:35, Eli Zaretskii wrote:
> > [Please use Reply All to reply, to keep everybody CC'ed.]
> > 
> >> Date: Mon, 12 Feb 2024 17:21:04 +0000
> >> From: Elken <me <at> elken.dev>
> >> 
> >> The use case that raised it for me was loading perspective workspaces 
> >> when I restart in place (due to a
> >> configuration change or other reason) but I don't want to load the 
> >> workspaces every time I start Emacs.
> > 
> > And why cannot you use kill-emacs-query-functions for this purpose?
> 
> Bumping this patch again as it would be nice to have in. To answer the 
> question more clearly (I thought I did before, my apologies) using 
> kill-emacs-query-functions would technically work but also add another 
> prompt every time I quit or restart. It makes sense to have a clear 
> separation of quitting for good and restarting in place.

Sorry, I still don't follow.  What does your
kill-emacs-query-functions hook do when passed the RESTART argument
that is different from RESTART being nil (i.e. killing Emacs without
restarting it)?  And what does "loading workspaces" mean in this
context?

> I also noticed this morning that `kill-emacs` and `restart-emacs` don't 
> handle the prefix argument for the hooks.

What would these hooks do with the prefix argument?

In any case, the prefix argument is available via current-prefix-arg.

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

From: me <at> elken.dev
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sat, 04 Jan 2025 13:34:23 +0000

> Sorry, I still don't follow.  What does your
> kill-emacs-query-functions hook do when passed the RESTART argument
> that is different from RESTART being nil (i.e. killing Emacs without
> restarting it)?

The point is differentiating between a quick "Oh I just need to clear 
out some memory or reset the state of my Emacs, but I want to carry on 
working as I was" and "I'm done with Emacs now, next time I start it I 
want it to be a clean slate". Adding another hook was the cleanest 
option I could come up with.

> And what does "loading workspaces" mean in this context?

Maybe I should refer to them as perspectives instead, but this package: 
https://github.com/nex3/perspective-el. An arrangement of buffers and 
frames categorised by some condition (in my case project.el project) so 
each perspective has its own "tab" with all the buffers that belong to 
the perspective and a specific window arrangement. Basically, I want all 
the open buffers and window arrangement remembered.

When I close down Emacs proper, I don't want it to load the exact 
arrangement when I then re-open it next time. However, when I call 
`restart-emacs`, I DO want it to load the previous arrangement of 
buffers, windows and tabs.

> In any case, the prefix argument is available via current-prefix-arg.

Yes you're right, I realised after I sent it....

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> elken.dev
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sat, 04 Jan 2025 16:14:17 +0200

> Date: Sat, 04 Jan 2025 13:34:23 +0000
> From: me <at> elken.dev
> Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> 
> > Sorry, I still don't follow.  What does your
> > kill-emacs-query-functions hook do when passed the RESTART argument
> > that is different from RESTART being nil (i.e. killing Emacs without
> > restarting it)?
> 
> The point is differentiating between a quick "Oh I just need to clear 
> out some memory or reset the state of my Emacs, but I want to carry on 
> working as I was" and "I'm done with Emacs now, next time I start it I 
> want it to be a clean slate". Adding another hook was the cleanest 
> option I could come up with.

It's a strange (and maybe even dangerous) distinction.

But in any case, I think a cleaner solution would be to bind a
variable inside restart-emacs, and then your hook could examine it.
Alternatively, ...

> > In any case, the prefix argument is available via current-prefix-arg.
> 
> Yes you're right, I realised after I sent it....

...you could invoke restart-emacs with a prefix argument, and examine
that in your kill-emacs-query-functions hook.  AFAICT, this should
work without any changes.

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

From: me <at> elken.dev
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Tue, 07 Jan 2025 15:54:42 +0000

> It's a strange (and maybe even dangerous) distinction.

I'm afraid I don't see this, could you elaborate?

> But in any case, I think a cleaner solution would be to bind a
> variable inside restart-emacs, and then your hook could examine it.
> Alternatively, ...
> 
>> > In any case, the prefix argument is available via current-prefix-arg.
>> 
>> Yes you're right, I realised after I sent it....
> 
> ...you could invoke restart-emacs with a prefix argument, and examine
> that in your kill-emacs-query-functions hook.  AFAICT, this should
> work without any changes.

I'm afraid I also don't follow here, how is that cleaner than 
introducing
a specific hook? Rather than needing to specifically modify a function
to work with the prefix argument, you can just add it to the hook.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> elken.dev
Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Tue, 07 Jan 2025 18:38:04 +0200

> Date: Tue, 07 Jan 2025 15:54:42 +0000
> From: me <at> elken.dev
> Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> 
> > It's a strange (and maybe even dangerous) distinction.
> 
> I'm afraid I don't see this, could you elaborate?
> 
> > But in any case, I think a cleaner solution would be to bind a
> > variable inside restart-emacs, and then your hook could examine it.
> > Alternatively, ...
> > 
> >> > In any case, the prefix argument is available via current-prefix-arg.
> >> 
> >> Yes you're right, I realised after I sent it....
> > 
> > ...you could invoke restart-emacs with a prefix argument, and examine
> > that in your kill-emacs-query-functions hook.  AFAICT, this should
> > work without any changes.
> 
> I'm afraid I also don't follow here, how is that cleaner than 
> introducing
> a specific hook? Rather than needing to specifically modify a function
> to work with the prefix argument, you can just add it to the hook.

It isn't necessarily cleaner, but it is a local solution for what I
consider a local and very specialized problem.  There's no reason to
expect Emacs to have an OOTB solution for every possible niche use
case that some user needs to handle.  Adding a hook means we all have
to pay the price in documenting it, maintaining it, etc.

Each request for adding a new feature should have a solid enough basis
before we must consider it for addition.  FWIW, I don't yet see why
this particular request qualifies.

Message #58 received at 65719-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> elken.dev
Cc: 65719-done <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#65719: [PATCH] Pass restart argument to
 'kill-emacs-query-functions'
Date: Sat, 01 Feb 2025 14:05:38 +0200

> Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> Date: Tue, 07 Jan 2025 18:38:04 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > Date: Tue, 07 Jan 2025 15:54:42 +0000
> > From: me <at> elken.dev
> > Cc: 65719 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> > 
> > > It's a strange (and maybe even dangerous) distinction.
> > 
> > I'm afraid I don't see this, could you elaborate?
> > 
> > > But in any case, I think a cleaner solution would be to bind a
> > > variable inside restart-emacs, and then your hook could examine it.
> > > Alternatively, ...
> > > 
> > >> > In any case, the prefix argument is available via current-prefix-arg.
> > >> 
> > >> Yes you're right, I realised after I sent it....
> > > 
> > > ...you could invoke restart-emacs with a prefix argument, and examine
> > > that in your kill-emacs-query-functions hook.  AFAICT, this should
> > > work without any changes.
> > 
> > I'm afraid I also don't follow here, how is that cleaner than 
> > introducing
> > a specific hook? Rather than needing to specifically modify a function
> > to work with the prefix argument, you can just add it to the hook.
> 
> It isn't necessarily cleaner, but it is a local solution for what I
> consider a local and very specialized problem.  There's no reason to
> expect Emacs to have an OOTB solution for every possible niche use
> case that some user needs to handle.  Adding a hook means we all have
> to pay the price in documenting it, maintaining it, etc.
> 
> Each request for adding a new feature should have a solid enough basis
> before we must consider it for addition.  FWIW, I don't yet see why
> this particular request qualifies.

No further comments, so I'm closing this bug.

Previous Next

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