Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unset argument behavior on various functions. #353

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

EtorixDev
Copy link
Contributor

This PR covers two related changes.


The first pertains to the WinTitle, WinText, ExcludeTitle, ExcludeText keys in WinWait. Both the v1 and v2 docs for WinWait specify that "At least one of these is required." As far as I can tell this is a unique requirement of this function.

The problem is that for the purposes of this function AHK treats empty strings as valid input, which means a call to WinWait with blank or missing arguments (which default to blank) and no timeout will result in an infinitely stuck script.

My solution is to raise a ValueError in the engine code to catch this ahead of time. Alternatively, a fix could be implemented in the daemon files by using the unset keyword in v2, however v1 does not support this and would instead require (to the best of my limited knowledge) 16 if statements to cover every combination of empty strings being excluded.


The second pertains to functions that make use of controls, namely AHKControlClick, AHKControlGetText, AHKControlGetPos, and AHKControlSend. These functions accept optional control arguments. A control can be a string, integer, or an object. Objects clearly aren't supported due to the encoding method, but the same goes for integers.

Integers become strings during encoding and while AHK can mostly deal with stringified numbers during operations like math, as input for a control it instead interprets it as a string. So if you use the HWND of a window, you're using "12345", not 12345, and the window fails to be found.

For v2 I have fixed this by changing the instances of ctrl := args[1] to ctrl := IsNumber(args[1]) ? Number(args[1]) : args[1] which uses the v2 specific IsNumber function to check if the number is numeric, and if so, type cast it, otherwise it stays as a string.

For v1, no changes are necessary. The documentation for the functions specify that to use a HWND you would instead use the WinText field with a ahk_id prefix.

Additionally, AHKControlClick specifically has a bug that is already mitigated in AHKControlSend, and that is the handling of ctrl when unset. In v1, a blank or omitted ctrl delegates it to the target window's topmost control, so no changes are needed.

However in v2 this only occurs for an omitted control, making a blank control valid input. In AHKControlSend you check if ctrl is blank and if so, call the underlying function without it. AHKControlClick is missing this. My proposed fix (for both, modifying the current fix) is to instead use the v2 specific unset keyword by passing the underlying function ctrl || unset which is equivalent to not providing the value at all if a blank input is provided.

@spyoungtech
Copy link
Owner

Thank you for the detailed report and fixes, I'll take a look at getting this merged soon. Thanks for your patience on this.

@spyoungtech spyoungtech changed the base branch from main to gh-353 December 20, 2024 09:08
@spyoungtech
Copy link
Owner

Pulling this into a separate branch so Pre-commit can try to apply needed changes.

@spyoungtech spyoungtech merged commit a01980a into spyoungtech:gh-353 Dec 20, 2024
1 check failed
spyoungtech added a commit that referenced this pull request Dec 20, 2024
Fix unset argument behavior on various functions
@spyoungtech
Copy link
Owner

Merged to main in #359 -- released in version 1.8.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants