activities.el icon indicating copy to clipboard operation
activities.el copied to clipboard

Reverting an activity should either remove the last state or immediately save

Open jdtsmith opened this issue 1 year ago • 8 comments

I tend to use revert on an activity when it has "drifted" and the buffers shown have changed. The new annotations PR #83 revealed that the last state remains set even after revert.

This is surprising because that state is no longer reachable while the activity is active, AFAIU, and it will eventually be overwritten when save-all is run during idle time. So it seems sensible either to remove the last state, or immediately save the activity, after it gets reverted.

jdtsmith avatar Apr 23 '24 22:04 jdtsmith

Wow. I have been having lots of friction and thought I was going crazy using activities. I believe this is why.

I had started manually calling activities-save-all periodically out of paranoia.

ParetoOptimalDev avatar Jun 14 '24 15:06 ParetoOptimalDev

I don't think this is necessary, or even necessarily beneficial.

  • If a user calls activities-revert and then wishes he hadn't, he could, e.g. kill the frame/tab before its state is automatically saved, and then activities-resume the activity again to return to the old last-saved state.
    • You may argue that this "undo" feature is non-obvious, and I would agree, yet it works.
  • The only downside I can see is that if a user called activities-revert and then killed the frame/tab before the activity's state was automatically saved (which may not even be the case in the future, if I can hook Activities into killing a frame/tab so it can be saved automatically), if he then called activities-resume on it, it would be restored to the old last-saved state instead of the default state.
    • But that is trivially solved by calling activities-revert again; no harm is done.

So I see a potentially significant downside to doing this, and no significant downside to not doing it.

What do you think? Thanks.

alphapapa avatar Jun 14 '24 18:06 alphapapa

Wow. I have been having lots of friction and thought I was going crazy using activities. I believe this is why.

Please be specific about the problematic behavior you have observed.

I had started manually calling activities-save-all periodically out of paranoia.

Why? That is what activities-mode does. That's why it exists and is recommended by default.

alphapapa avatar Jun 14 '24 18:06 alphapapa

If a user calls activities-revert and then wishes he hadn't, he could, e.g. kill the frame/tab before its state is automatically saved, and then activities-resume the activity again to return to the old last-saved state. You may argue that this "undo" feature is non-obvious, and I would agree, yet it works.

Indeed I would, also dependent on activities-mode-idle-frequency. Compared to the confusion of when you revert the activity, but the details of the last state remain in place, I come down in favor of "saving on change". Revert is certainly a change. Besides, we have better tools for "I just changed the window state, but I wish I didn't", e.g. winner-undo, which works wonderfully with activities.

The only downside I can see is that if a user called activities-revert and then killed the frame/tab before the activity's state was automatically saved (which may not even be the case in the future, if I can hook Activities into killing a frame/tab so it can be saved automatically), if he then called activities-resume on it, it would be restored to the old last-saved state instead of the default state.

Yes this would be confusing, sort of the inverse of the "undo trick" you propose. The other related confusion comes from annotation information being out of date until the idle time save occurs. Nothing obvious happened, but the state changed. I know we haven't yet merged the annotation PR, but it was what revealed the situation to me.

Here's the analogy I think people will reach for: choosing revert file in some app. If the contents of the "file" (here, an activities state, buffers and so on) change after a revert command, but the associated metadata like character count do not (e.g. number of buffers, last saved date, etc.) most I believe would conclude this is a bug, especially if it then mysteriously updates at a later time.

One possibility to preserve both behaviors and retain the possibility of your "undo" trick (though IMO it's of questionable value) is not to delete last or save, but instead add a current-type flag in each activity which can point to either default or last. Revert sets it to default, and presumably save would update it to last, if things have changed. I think in my usage all this buys you is 5s of "intermediate state", but for people who disable auto-saving, this could perhaps be valuable.

jdtsmith avatar Jun 14 '24 19:06 jdtsmith

One possibility to preserve both behaviors and retain the possibility of your "undo" trick (though IMO it's of questionable value) is not to delete last or save, but instead add a current-type flag in each activity which can point to either default or last. Revert sets it to default, and presumably save would update it to last, if things have changed. I think in my usage all this buys you is 5s of "intermediate state", but for people who disable auto-saving, this could perhaps be valuable.

I'd prefer not to do that, as it would mean additional complexity in several places.

I don't know. I think this isn't a very big deal, and the inconsistency, as it were, also provides some flexibility. As you said, until you looked at the annotation, which examines the internal data structure, you didn't even notice it.

It is regrettable that there is an inconsistency in the annotation, and maybe that's reason enough to do what you say. OTOH I'm not sure that it is.

From another perspective, there is no inconsistency in the annotation, because it reflects the reality of the data structure. The inconsistency then is just a matter of how the command works internally vs. what it implies to the user. But I've already covered why I'm not sure that is a problem.

Would it be okay with you if we "slept on" this issue for a while?

alphapapa avatar Jun 14 '24 22:06 alphapapa

Of course! Easy to add an :after advice if needed.

jdtsmith avatar Jun 15 '24 01:06 jdtsmith

Please be specific about the problematic behavior you have observed.

I finally ran into an instance.

I resumed activity "today" consisting of "Dashboard" and "household" org-ql agenda views.

I want this activity to now only show the "Agenda-like" org-ql view so I:

  • M-x org-ql-view Agenda-like RET
  • C-x 0 (delete all other windows except for Agenda-like
  • suspend the today activity (assuming this will save the state as docs say "Its last state is saved, and its frames, windows, and tabs are closed.")
  • It's last state was a fullscreen Agenda-like org-ql view
  • resume activity "today"

Everything is good and works as expected to this point. I work a bit and the activity drifts away from it's intended purpose. I revert the activity expecting a full screen "Agenda-like" org-ql view.

Instead I get the old state consisting of "Dashboard" and "household" org-ql agenda views, despite the activity having resumed correctly.

ParetoOptimalDev avatar Jun 19 '24 22:06 ParetoOptimalDev

Aside from the race condition discussed above, the last note from @ParetoOptimalDev sounds like a bit of usage confusion. Revert will always return to the originally stored "default state" at the time of creation of the activity. To overwrite the default state, you must redefine your existing activity again (use activities-define to redefine it). Suspend is intended to save the latest state. Resume is intended to resume that latest suspended state.

shipmints avatar Jun 20 '24 20:06 shipmints