window_action: use appscript to close all windows #55
Conversation
|
Looks like a start although I am a bit worried about apps that are not scriptable or have broken scripting interfaces. Closing all windows in an app is not something I do very often, and I didn't realize you implemented this with different syntax to the typical knausj "noun verb" so I'd not been using any of these commands. I notice you have a long timeout here. Is this because closing is synchronous in some apps which will do things like prompt to save changes? (Note that the timeout is in seconds — maybe you thought it was milliseconds? https://appscript.sourceforge.io/py-appscript/doc/appscript-manual/11_applicationcommands.html) I'd recommend you don't bother waiting for a response to the event more than a couple hundred milliseconds, so this doesn't hang the thread in Talon. You should get a different exception (immediately) if the app’s scripting dictionary doesn't support window closing, for example: |
|
The 10s was pretty arbitrary. I only included it because appscript included a timeout by default, and I was worried about the possibility of it hanging indefinitely. I can set it to something lower like 3-5s, although I think it can genuinely take a while for an application to close all of its windows if there are a lot of them and it's not the most performant UI system (for example, intellij). Do you know if this timeout is for just sending the message, or for the entire action to run? If it's the former, then only waiting for 300ms make sense; if it's the latter, I think it's reasonable to wait a few seconds if it really needs that time to finish the work. I'm guessing if we hit the timeout, it won't interrupt the process of closing the windows (?), but it will cause this code to try to proceed to accessibility, which is not necessary.
Ah, I didn't realize the versions in the repo were verb-noun, I thought I had checked in noun-verb versions. I can add those. I personally find these pretty useful! |
|
I think you can set the timeout very short. You don't care about a successful response or subsequent actions, only a failed response to allow you to fall back to accessibility. Yes it should not interrupt the process of closing the windows! |
|
That's fine; I'm just wondering if there's a case where the scripting call would succeed, but legitimately take a few seconds, and we might time out before it finishes. |
|
The timeout is only on the client side. The server (application) isn't even aware of the timeout — so nothing to worry about. |
|
Got it. So something like 0.3? |
|
Yup! Just enough to ensure that you can get an error response — should be fine, particularly when the fallback will still work! |
phillco
force-pushed
the
phil/2023-06-18/2aca
branch
from
aba26b0 to
9801b4e
Compare
|
Done. I also refactored slightly and made the |
In #55 it was pointed out that the existing commands used verb-noun, which is less standard. Also added the missing `<user.running_applications> window {user.window_actions}`, so you can target (close or minimize) the most recent window of another application. More situational, but I don't see a reason not to fill out the matrix. The fullscreen logic needs a little work after this. Filed #57
I noticed that stuff like "window close all" in applications like Finder was really slow and flaky, and would often result in several windows being left open if you had a lot open (probably because they technically treat tabs as windows).
I was poking around with AppleScript and noticed that it's actually much faster and more reliable to just close all of the windows that way, ie with:
It fixes the Finder issue, and even in applications previously behaved well, there is an improvement (all windows close immediately, rather than a cascade of closures as it iterates through them). So let's use that!
I didn't do much to improve the "current window" and "other windows" use cases, although it probably not much work. I just wanted to knock this out quickly.