Hi, what is framed mode?

Anyway, I'm not sure if this is a good idea. According to the docstring, save-window-excursion affects the selected frame, but in your case a different frame appears to be affected, which suggests some behavior we don't understand. Also, it generally warns against using it:

Execute BODY, then restore previous window configuration.
This macro saves the window configuration on the selected frame,
executes BODY, then calls ‘set-window-configuration’ to restore
the saved window configuration.  The return value is the last
form in BODY.  The window configuration is also restored if BODY
exits nonlocally.

BEWARE: Most uses of this macro introduce bugs.
E.g. it should not be used to try and prevent some code from opening
a new window, since that window may sometimes appear in another frame,
in which case ‘save-window-excursion’ cannot help.

To fix the problem you're reporting in the best way probably requires a more complete understanding of the cause and possible solutions.

Hi, what is framed mode?

Wasn't sure what you call the opposite of activities-tabs-mode, so I just called it "framed mode".

save-window-excursion affects the selected frame, but in your case a different frame appears to be affected

Apologize that it wasn't clear. I am actually referring to selected frame being altered as the problem. Just showing side-by-side before and after from that screen demo in the issue in image below: the left side is before the activities-save-all timer, and the right is after that timer runs.

Notice in the aligned images, on the right-side the minibuffer top input line (orange) shifts 2 lines down. And if just leave idle, every 5 seconds it will continue to shrink until it is 1-2 lines in height!

(Aside: Yes, the other (non-selected) frame jumps a bit, but only for a very brief moment until it refreshes. But the issue is more with selected frame adjusting minibuffer and not resetting.)

Also, it generally warns against using it

How I read that warning, it is saying is that bugs from it will cause users window configurations to be messed up when doing things like opening a new window sometimes being in another frame. In this case, since it's only wrapping the save-all operation which does not do any other window changes, etc. the warning is heeded and it would not have issues noted. In this scenario, it is fixing the an issue with window configuration being messed up by another process.

Hope that makes it clearer what I was trying to say earlier.

Thanks for the clarifications.

Notice in the aligned images, on the right-side the minibuffer top input line (orange) shifts 2 lines down. And if just leave idle, every 5 seconds it will continue to shrink until it is 1-2 lines in height!

This is peculiar. Nothing in Activities itself resizes windows that way. Do you know what code is doing it? Does it happen in a default Emacs config? Does it happen with the latest versions of Consult, et al?

As you may have noticed, we have the option activities-anti-save-predicates which has in its default value active-minibuffer-window, which means that if a minibuffer window is open, the activity's state won't be saved. But Activities still switches to each active activity to see if it should be saved. Since the minibuffer should exist across activities, maybe that check should be done at a higher level and preclude even switching activities to see if they should be saved.

I'm not strictly opposed to using save-window-excursion, but I think we should try to find out the root cause first.

Looks like what happens my situation, when timer runs all activities are saved, which does a select-frame to switch to the other activity. Since I have default minibuffer-follows-selected-frame is t the other frame inherits the active minibuffer when that frame is selected for save. But in the other frame, it sizes the minibuffer for it's frame - likely because it doesn't have the same elements, or buffer text, or etc. This size then follows-selected-frame back to original activity frame, which causes shrinkage.

I had tried doing a let binding of minibuffer-follows-selected-frame but docstring says only top-level value is used.

The save-window-excursion macro simply holds onto the size of minibuffer while it switched between activities frames, and restores once done.

we have the option activities-anti-save-predicates which has in its default value active-minibuffer-window

Oh, no I did not notice that. Yes, maybe moving to higher level may make sense. Was there a reason you added active-minibuffer-window originally to activities-anti-save-predicates? I.e. did you notice something else happening, or just good practice?

So... with all that said, I started thinking about what I said above with: "But in the other frame, it sizes the minibuffer for it's frame - likely because it doesn't have the same elements, or buffer text, or etc." I remembered I had added a couple of years ago ability to resize vertico minibuffer. See: https://github.com/minad/vertico/wiki#adjust-number-of-visible-candidates-when-buffer-is-resized

It does look like this is also to blame. I went a bit different from using (1- height) from that vertico wiki helper to difference of 5, because the 1- caused the minibuffer to grow wildly. I also noticed that minibuffer shrunk by exactly 2 lines with activities-save-all. I decided to change my difference of 5 to then just be 3, and now doesn't shrink. Go figure. But also with 3 I can't manually drag and resize the minibuffer anymore, which was the point of it.

tl;dr

Very long story short (too long probably), I need to leave my vertico resize as (- height 5) in order to resize manually minibuffer when needed. Not sure I want to turn off minibuffer-follows-selected-frame. I'm okay advising activities-save-all myself with (for others that may come across):

      (advice-add #'activities-save-all :around
                  (defun my/activities-save-all-dont-adjust-minibuffer (orig &rest r)
                    (save-window-excursion
                      (apply orig r)))))

There may be some other feature out there that may also react to frame changes like this. Now at least aware if something else pops up down the road.

Definitely good closing this PR, if you are. Appreciate the time here and with activities!

Mulling over more, better would be just to inhibit-redisplay when saving all since save-window-excursion has usage warning.

Updated the PR.

Feel free to accept or close PR.

Thanks!

Mulling over more, better would be just to inhibit-redisplay when saving all since save-window-excursion has usage warning.

That sounds nice, but its docstring says that it's for internal purposes, and it's not documented in the manual, so I'm not sure that it's okay for us to use. It should probably be researched a bit first, e.g. by looking for mentions on emacs-devel.

As well, I'm not certain that redisplay is even happening, because I don't see any flickering (whereas I did in other circumstances), and the Lisp code isn't stopping between activities, and we're not calling redisplay ourselves--so I don't know when redisplay would even happen.

Finally, AFAICT redisplay isn't necessary for adjusting a window configuration, so I don't know if inhibiting it would prevent the problem reported here.

Thank you for allowing the detailed back and forth! I definitely appreciate your insight in trying to create and maintain many popular packages with best long-term maintainability.

That sounds nice, but its docstring says that it's for internal purposes, and it's not documented in the manual, so I'm not sure that it's okay for us to use. It should probably be researched a bit first, e.g. by looking for mentions on emacs-devel.

There's only 16 references to inhibit-redisplay in emacs-devel, so not many. :) In emacs-bugs the most related is this thread: https://yhetil.org/emacs-bugs/[email protected]/T/#efb26d264c4fe1c68f936bb9d959684f2037781e6 where mentioned: Display mini-window resized even when there are several frames is similar to this, and the ultimate fix was to use inhibit-redisplay to prevent the resizing. More is found in emacs Changelog.3:

2019-02-02  Martin Rudalics  <[email protected]>

        Fix bugs caused by running window change functions during redisplay

        * src/xdisp.c (redisplay_internal): Run window change
        functions before updating the display so changes induced by
        these functions can get caught by redisplay (Bug#34138).
        * src/window.c (run_window_change_functions): Bind
        Qinhibit_redisplay to avoid that the minibuffer window gets
        resized while running window change functions (Bug#34179,
        Bug#34260).

As well, I'm not certain that redisplay is even happening, because I don't see any flickering (whereas I did in other circumstances), and the Lisp code isn't stopping between activities, and we're not calling redisplay ourselves--so I don't know when redisplay would even happen.

Finally, AFAICT redisplay isn't necessary for adjusting a window configuration, so I don't know if inhibiting it would prevent the problem reported here.

Seems to happen during select-frame being called from function chain: activities-save-all -> activities-save +> activities-with activity -> select-frame (and then) activities-save +> activities-state -> activities--window-state -> with-selected-frame. Not 100% and you probably know better, but it looks like selected frame change causes run_window_change_functions (c code) go through, which at top of that specbind (Qinhibit_redisplay, Qt); happens.

Very long story short (sorry!), setting inhibit_redisplay t definitely causes problem not not [<edit] to happen (I also tested).

However, in further digging, it looks like edebug-debugger explicitly won't debug if inhibit_redisplay is non-nil. Which is bad if having issues with saving, especially persisting to disk and get error. Or just trying to debug it in general.

So I do think save-window-excursion is better, but what I've done in latest iteration is to take the expanded save-window-excursion macro and only save and restore the current frame's window-configuration if there is an active-minibuffer, to make it more targeted fix. It also does not stop edebug.

Let me know if that answers (to the best of my abilities :)) some of your questions, and helps set context.

My only hesitation about binding inhibit-redisplay now is that the example you cited does so in a C file rather than a Lisp one. I'd still like to get confirmation from a maintainer that it's okay to use it in this way from Lisp code. It seems like it would be the best solution if it's okay to use it. Would you be willing to ask about that on emacs-devel?

Very long story short (sorry!), setting inhibit_redisplay t definitely causes problem not not to happen (I also tested).

I guess that double-negative is a typo, but I have to ask to be sure. :)

Thanks for looking into these things.

My only hesitation about binding inhibit-redisplay now is ...

I agree, should not use, but I was saying more for problem it causes with edebug, among others.

You may have missed this part in my overly verbose reply:

I do think save-window-excursion is better, but what I've done in latest iteration is to take the expanded save-window-excursion macro and only save and restore the current frame's window-configuration if there is an active-minibuffer, to make it more targeted fix. It also does not stop edebug.

You can see this in the updated commit attached to PR. More targeted and common.

Very long story short (sorry!), setting inhibit_redisplay t definitely causes problem not not to happen (I also tested).

I guess that double-negative is a typo, but I have to ask to be sure. :)

I edited and struck out the second "not". It was not not intended. :)

Thanks for your patience in working with me on this. I have a lot going on, so when I revisit this issue, it's easy for me to overlook or forget details.

I mentioned earlier the possibility of having a global "anti-save" predicate list, which would prevent the saving of any activities, and that we could check for an active minibuffer in it. It seems to me that we probably shouldn't be saving any activities if there's an active minibuffer, anyway; and a solution like that would be more general and seem less hacky than this workaround of saving and restoring the window config if a minibuffer was open at the time we started saving. Do you (still) agree?

By the way, for future reference, it's not a good idea to make PRs from a fork's master branch. It makes it harder to rebase and such if upstream diverges, and it presents a problem if a contributor ends up with multiple outstanding PRs. (You don't need to fix this now, especially since we may use a different solution. This is just FYI.)

It seems to me that we probably shouldn't be saving any activities if there's an active minibuffer, anyway; and a solution like that would be more general and seem less hacky than this workaround of saving and restoring the window config if a minibuffer was open at the time we started saving. Do you (still) agree?

Yes, makes perfect sense.

I mentioned earlier the possibility of having a global "anti-save" predicate list, which would prevent the saving of any activities, and that we could check for an active minibuffer in it.

Since I know you have a lot going on, would you take assistance on this - or you already have a plan?

By the way, for future reference, it's not a good idea to make PRs from a fork's master branch.

Not sure what I was thinking... I do know better.

Okay to close this PR now?

Thanks again!

@jeff-phil Sure, feel free to send a PR about the new predicate list.

BTW, forgive me if I asked already, but have you done FSF copyright assignment for Emacs?

@jeff-phil Sure, feel free to send a PR about the new predicate list.

[thumbs-up!]

BTW, forgive me if I asked already, but have you done FSF copyright assignment for Emacs?

Yes, I am complete on FSF copyright assignment.

Yes, I am complete on FSF copyright assignment.

Ok, however I'm not allowed to take anyone's word for it (not that I doubt you). I have to get confirmation from the Emacs maintainers (or find you in the list of existing contributors). Can I find commits by you in emacs.git? If so, under what name/email?