Fix more bad interactions with evil visual

[real-or-random]

Follow up to #1205.

The first commit is a tweak to #1205. The other commits fix additional cases.

I hope this should fix any real functionality problems with evil-mode's visual selection. The remaining issues will then be issues when displaying the transient menu (#909). But let's first fix functionality.

[real-or-random]

This forces rebuilding the entire transient after every key press.

If you don't like the overhead introduced by this, then an alternative is to call (transient--stack-pop) (transient--env-apply #'transient--refresh-transient)) (still, instead of the (run-at-time ...). This will also refresh the main transient prefix. But it relies on transient internals. Please let me know if you prefer this solution.

[karthink]

I'm aware of :refresh-suffixes and chose to avoid it in gptel-menu because of performance issues. I use it in some other transient menus, like gptel-tools, that have to be fully dynamic to function.

Why do you need to enforce a menu-wide change here?

There are actually many calls to transient-setup or equivalent in specific infixes of gptel-menu, not just the ones on line 548 and 1053 you've removed.

[real-or-random]

There are actually many calls to transient-setup or equivalent in specific infixes of gptel-menu, not just the ones on line 548 and 1053 you've removed.

Indeed. It took me a while to figure out what is special about the calls on lines 548 and 1053 (and why they need a timer): on these lines, the active transient is not gptel-menu but gptel--preset. So when you call transient-setup there without a timer, you refresh gptel--preset but not the parent gptel-menu.

I don't think transient offers an official way to refresh the parent. I tried calling (transient-setup 'gptel-menu) but that didn't work.

If you're concerned about performance, then there are two options:

• As I said above, (transient--stack-pop) (transient--env-apply #'transient--refresh-transient) does the job at the cost of relying on a transient internal.
• Or, let's just drop the third commit (and perhaps also the second)
  and keep the timers here. As I wrote below, removing these is not essential.

[real-or-random]

Hm, after taking a step, I'm not convinced anymore that the second and the third commit actually do fix anything.

They avoid cases in which we get the region wrong, i.e., the user has created a visual selection, but evil hasn't had a chance to set the region to this selection). All those cases appear when displaying the transient. However, this is not just about wrong info displayed to the user (as in #909) but they actually affect functionality: if the code displaying the transient believes that there's no region, then all the choices related to a region won't even appear (e.g., "r Rewrite" or "-r Add region to context").

The problem now is that the display code gets the region wrong only when called from the post-command-hook or from a timer (1), but not when called directly by the user (2). So the second and the third commit here potentially avoid issues for case 2 but not for case 1. For case 1, we'll anyway need a bit of evil-specific code to solve these issues entirely, even if we get rid of the timers.

What does this mean?

• I believe we still want the first commit here because it cleans up a bit what I did in gptel-rewrite: Fix issues with evil's visual selection mode #1205.
• I leave it to you if you want the second and the third commit (with or without the alternative version touched upon in the thread above). I believe these are still improvements because they get rid of the timer hacks. This makes the code certainly cleaner, but it also risks breaking whatnot. What do you think?
• I still need to PR my evil-specific changes that will address the display issues in all cases, i.e., When using evil mode, "Send selection" goes away in transient menu on any menu choices #909 but also the missing menu items mentioned above.

[real-or-random]

• I still need to PR my evil-specific changes that will address the display issues in all cases, i.e., When using evil mode, "Send selection" goes away in transient menu on any menu choices #909 but also the missing menu items mentioned above.

See #1236.

[karthink]

1. I'm not comfortable enabling :refresh-suffixes on gptel-menu. Why is this necessary to fix evil-mode compatibility?
2. Does this need to be merged before gptel-transient: Fix display issues with evil visual selections #1236 or are they independent?

[real-or-random]

Sorry, I tried to explain what everything does, but I see that there's a lot of text in this PR, and it's not super clear since also my understanding of things has evolved over time.

Let me try to explain again, and more clearly.

2. Does this need to be merged before gptel-transient: Fix display issues with evil visual selections #1236 or are they independent?

They are independent. In terms of fixed user-facing evil-mode issues, #1236 entirely subsumes this one because it addresses the problem at a different layer. My suggestion is to go ahead and merge #1236 if you're fine with the changes there. This should resolve all user-facing issues with evil-mode, so getting #1236 in should be the main priority in this "evil project".

---

Assuming #1236 gets in, all this PR here does is clean up the code by getting rid of the remaining (run-at-time 0 ...) hacks. After #1236, they shouldn't make any trouble specifically for evil users. Still, the hacks are not very clean because additional code can run before the timer fires (e.g., what if the selected window has changed before the timer fires?). But since they seem to do the job in practice, getting rid of them is merely desirable but not essential.

1. I'm not comfortable enabling :refresh-suffixes on gptel-menu. Why is this necessary to fix evil-mode compatibility?

Each commit in this PR removes one run-at-time call. The third commit does so at the cost of adding :refresh-suffixes.

Why does it need :refresh-suffixes? The reason is what we've discussed in the conversation above. This addresses refreshing the gptel-menu transient in this specific case:

1. Open gptel-menu
2. Press @ to open the gptel--preset transient and press some character to apply a preset
3. Now you end up back in the gptel-menu transient and should see the options refreshed according to the applied preset.

The current code calls (run-at-time 0 nil #'transient-setup) to perform the refresh. The reason why this works is that when the timer fires, the active transient is gptel-menu, so that transient gets refreshed.

However, if we remove the timer and simply call transient-setup, then at the time of the call, the active transient will still be gptel--preset. So gptel--preset gets refreshed instead of gptel-menu.

What we want to do here is to refresh the parent. But there does not seem to be an official way of doing this. The only options I could come up with:

• Set :refresh-suffixes. That forces a refresh every time, but I can understand that you're not comfortable with that approach.
• Run (transient--stack-pop) (transient--env-apply #'transient--refresh-transient) instead of (transient-setup). But that's not great either since it relies on transient internals. So this would merely replace the timer hack with another (probably worse) hack.

---

Here's my suggestion on how to move forward:

• You merge gptel-transient: Fix display issues with evil visual selections #1236 if you're happy with the changes there.
• I update this PR to drop the third commit, so we leave this single timer hack in place. Or better, let me replace the commit with one that only adds an explanatory comment above the (run-at-time 0 ... line.
• Independently of that, I can ask upstream if there's a clean way to refresh the parent transient. Edit: See Is there a way to refresh the "parent" transient? magit/transient#430.