Terminal Posted May 19, 2023 Share Posted May 19, 2023 Hello, I noticed today when I was using my Workflow that leverages various Window Management automation tasks that argv input handling can cause a error in the a Window does not have focus. What is happening is that when you click on the Desktop and don't focus a window, and then trigger a workflow that using something like `half-right` or any other present, the `argv` that is passed to the window-bounds.js doesn't have the correct amount of arguments and you end up getting a `../window-bounds.js: execution error: Error: Error: Invalid index. (-1719)` The fix is fairly straight forward (in fact I did it locally) and that is to first check argv length at the beginning of run to be >= 3 and then in any of the conditionals that accept 3+ more argv validate the length again before accessing Link to comment
vitor Posted May 19, 2023 Share Posted May 19, 2023 Checking for the number of arguments in several places increases the amount of code and surface area for problems but doesn’t offer a benefit and doesn’t address the situation, it hides the cause. The problem isn’t the number of arguments, it’s that there’s no window to move. And there’s no window specifically because your frontmost app (the Finder) has no open windows at the time. In other words, it’s not about the Desktop. Thus a path here could be to check if a window was successfully obtained and show a helpful error message if not. However, due to type of object returned we only know if we were successful when we try access its properties. Which comes with a performance cost. We could have it into a try clause, but it’s a sizeable piece of code which uses the Objective-C bridge so keeping it simple matters. In sum, we we need to return an error in that situation anyway, because it is indeed an error. At best the message could be a bit more instructive, but that’s a tiny improvement for a niche (and understandable) situation which comes at either a cost of code comprehension or execution speed, neither of which are good tradeoffs because you want this to be as fast as possible and maintainable. I do have ideas for a revamp of this code to make it faster, and if the plan is workable the messaging would be taken care of in the same swoop. No promises or ETA as more exploration is in order and it requires futzing with some low-level APIs. It’s important that it continues to work, and it does now. Terminal 1 Link to comment
Terminal Posted May 19, 2023 Author Share Posted May 19, 2023 12 minutes ago, vitor said: Checking for the number of arguments in several places increases the amount of code and surface area for problems but doesn’t offer a benefit and doesn’t address the situation, it hides the cause. The problem isn’t the number of arguments, it’s that there’s no window to move. And there’s no window specifically because your frontmost app (the Finder) has no open windows at the time. In other words, it’s not about the Desktop. Thus a path here could be to check if a window was successfully obtained and show a helpful error message if not. However, due to type of object returned we only know if we were successful when we try access its properties. Which comes with a performance cost. We could have it into a try clause, but it’s a sizeable piece of code which uses the Objective-C bridge so keeping it simple matters. In sum, we we need to return an error in that situation anyway, because it is indeed an error. At best the message could be a bit more instructive, but that’s a tiny improvement for a niche (and understandable) situation which comes at either a cost of code comprehension or execution speed, neither of which are good tradeoffs because you want this to be as fast as possible and maintainable. I do have ideas for a revamp of this code to make it faster, and if the plan is workable the messaging would be taken care of in the same swoop. No promises or ETA as more exploration is in order and it requires futzing with some low-level APIs. It’s important that it continues to work, and it does now. Yes, it just took me a minute to see why it wasn't working. For now, I've added a check in my workflow to ensure there is a top most window Link to comment
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now