Save activity when frame/tab is deleted/closed

[mbatson]

Closes #29

Hi, I wanted this functionality and got it working in my own init.el, then saw #29 was open, so figured I may as well propose a proper patch for the actual project. Please note I'm a newbie at elisp and an amateur programmer, so please don't hesitate to close this PR if the proposed changes are just not done in a good enough way and it's more trouble than it's worth to fix up, or even if this behaviour just isn't desired anymore! Also I note that #29 is currently low-priority in the milestones, so no hurry to get to this PR!

This change has the following effects:

• When user does not have activities-tabs-mode enabled and they delete a frame that has an activity open in a manner that calls the hook delete-frame-functions, the activities state is saved before the frame is deleted.
• When user does have activities-tabs-mode enabled, and they close a tab in a way that calls tab-bar-tab-pre-close-functions, the activities state is saved before the tab is closed.
• When user has activities-tabs-mode enabled, and they delete a frame that contains tabs with activities open in them, the state of each activity open in those tabs is saved before the frame is deleted.

I've taken activities-mode--killing-emacs as a model for the two new functions activities-mode--deleting-frame and activities-tabs-mode--closing-tab.

A couple things I was unsure about:

• Should this behaviour be toggled by a user option?
• The implemented behaviour respects activities-kill-buffers, but let me know if it would be preferred that it didn't.
• To handle the situation where a user is using tabs, but deletes the frame holding those tabs, I found the most straightforward way to handle it was just adding the logic in the deleting-frame function in activities.el. An alternative could be to move that logic into activities-tabs.el to keep the handling of tabs more separated, but I wasn't sure about this as then both modes will need to add/remove functions to delete-frame-functions, and I was concerned it could lead to a messy situation where the same functions like activities-save and activities--kill-buffers are being called multiple times by different functions on the same tabs/frames.

P.S. Thanks very much for the great package! This is exactly what I've been looking for in terms of being able to pick up different projects/activities where I last left off in a manner that gets out of the way and doesn't require manual fussing around with state.

Edit: Forgot to mention, I don't yet have copyright assignment for Emacs contributions, but have requested the form.

[alphapapa]

Hi Matthew,

Thanks, this looks pretty good to me. I wouldn't have known that you're new to Elisp if you hadn't said so. :)

In general, this is probably a good idea, since it closes the gap between the timed saving and manually killing a frame or tab. For myself, I don't mind the gap, but I'm sure that some users do, so we should probably implement this.

Should this behaviour be toggled by a user option?

I doubt that an option is needed, because it seems like the correct behavior to implement, and it should be harmless (some users might even perceive this as fixing a bug).

The implemented behaviour respects activities-kill-buffers, but let me know if it would be preferred that it didn't.

We probably need an option for that, though, because killing a frame/tab is not the same as killing an activity, and Activities is intended not to interfere with actions outside its realm, so killing a frame/tab probably shouldn't kill the associated buffers by default.

To handle the situation where a user is using tabs, but deletes the frame holding those tabs, I found the most straightforward way to handle it was just adding the logic in the deleting-frame function in activities.el. An alternative could be to move that logic into activities-tabs.el to keep the handling of tabs more separated, but I wasn't sure about this as then both modes will need to add/remove functions to delete-frame-functions, and I was concerned it could lead to a messy situation where the same functions like activities-save and activities--kill-buffers are being called multiple times by different functions on the same tabs/frames.

That's a tough one. I'd prefer to keep the conditional logic more separated into the two libraries/modes, in general. But sometimes the best solution is not clear, and sometimes there's no clean way to do it. To be frank, at the moment I don't have the energy to think about those details, as it's a Friday evening and it's been a long week. :) Maybe we could both think about this part for a while longer.

P.S. Thanks very much for the great package! This is exactly what I've been looking for in terms of being able to pick up different projects/activities where I last left off in a manner that gets out of the way and doesn't require manual fussing around with state.

Thanks, I'm glad to hear that. Sounds like it's fulfilling its purpose as intended. :)

Edit: Forgot to mention, I don't yet have copyright assignment for Emacs contributions, but have requested the form.

Great, thanks. I'm required to ask the Emacs maintainers for confirmation directly, so when you receive confirmation of your assignment from the FSF secretary, please send me the name and email address you used to sign the forms, which I can then ask the maintainers about.

[mbatson]

Thanks, this looks pretty good to me. I wouldn't have known that you're new to Elisp if you hadn't said so. :)

Thanks! To clarify, I've been slowly building up familiarity with elisp over a few years configuring and hacking away at things in my init file, so not totally new, but new to writing code meant to be used and read by other people, so it's nice to hear I'm on the right track and not doing things in some unnatural/poor-style manner haha :)

I doubt that an option is needed, because it seems like the correct behavior to implement, and it should be harmless (some users might even perceive this as fixing a bug).

That makes sense and was my thinking too.

The implemented behaviour respects activities-kill-buffers, but let me know if it would be preferred that it didn't.

We probably need an option for that, though, because killing a frame/tab is not the same as killing an activity, and Activities is intended not to interfere with actions outside its realm, so killing a frame/tab probably shouldn't kill the associated buffers by default.

That's fair. In my own use of activities, I think of killing the frame/tab as just another way of suspending an activity, so I want it to behave exactly the same as if I'd called activities-suspend. But it's true that other users may not want/expect these things to be 100% conflated. Unless I hear from you otherwise, I'm happy to push a new commit here for review with an implementation of such an option soon.

To handle the situation where a user is using tabs, but deletes the frame holding those tabs, I found the most straightforward way to handle it was just adding the logic in the deleting-frame function in activities.el. An alternative could be to move that logic into activities-tabs.el to keep the handling of tabs more separated, but I wasn't sure about this as then both modes will need to add/remove functions to delete-frame-functions, and I was concerned it could lead to a messy situation where the same functions like activities-save and activities--kill-buffers are being called multiple times by different functions on the same tabs/frames.

That's a tough one. I'd prefer to keep the conditional logic more separated into the two libraries/modes, in general. But sometimes the best solution is not clear, and sometimes there's no clean way to do it. To be frank, at the moment I don't have the energy to think about those details, as it's a Friday evening and it's been a long week. :) Maybe we could both think about this part for a while longer.

No worries! Enjoy your evening and weekend! In the meantime, if I have time over the next few days I can post some code here with an example of the other way I was considering implementing it, with all logic referring to activities-tabs-mode contained in activities-tabs.el.

Great, thanks. I'm required to ask the Emacs maintainers for confirmation directly, so when you receive confirmation of your assignment from the FSF secretary, please send me the name and email address you used to sign the forms, which I can then ask the maintainers about.

Will do!

[mbatson]

I've pushed a proposed implementation of a user option for whether to kill buffers when deleting/closing a frame/tab.

And as promised, to make it easier to compare and contrast approaches, here's an alternative implementation of this PR with the logic related to tab-closing separated out into activities-tabs.el for consideration:

https://github.com/alphapapa/activities.el/compare/master...mbatson:activities.el:save-on-frame-tab-close-alt

While it requires a bit more fussing around with adding/removing the appropriate hooks, it actually turned out to be cleaner than I'd anticipated, and gave an easy opportunity for removing some code duplication as well.

[mbatson]

I've completed copyright assignment and sent you an email with the details.