Danp2 / au3WebDriver

Web Driver UDF for AutoIt
MIT License
108 stars 23 forks source link

Let _WD_Window accept unformatted handles #429

Closed seadoggie01 closed 1 year ago

seadoggie01 commented 1 year ago

Pull request

Proposed changes

_WD_Window($sSession, "handles") returns an array of handles. This lets _WD_Window accept its own return values directly instead of having to remember to type '{"handle":"' & $aHandles[1] & '"}' each time

Checklist

Put an x in the boxes that apply. If you're unsure about any of them, don't hesitate to ask. We are here to help!
This is simply a reminder of what we are going to look for before merging your code.

Types of changes

Please check x the type of change your PR introduces:

What is the current behavior?

_WD_Window needs formatted handles

What is the new behavior?

_WD_Window attempts to format unformatted handles

Additional context

N/A

System under test

Please complete the following information.

Danp2 commented 1 year ago

Hi @seadoggie01,

Thanks for the submission. Since we already have __WD_JsonElement, it would make sense to me to add __WD_JsonHandle. This would encapsulate all of your changes into a single function so that you could write the following to confirm a properly formatted string --

$sOption = __WD_JsonHandle($sOption)

Thoughts?

Dan

seadoggie01 commented 1 year ago

That sounds great! Would that be in addition to editing the _WD_Window function?

Danp2 commented 1 year ago

That sounds great! Would that be in addition to editing the _WD_Window function?

Yes. You would add the new __WD_JsonHandle function to wd_core.au3. Then you would replace your current proposed changes to _WD_Window with the single line of code from my last post (each of your new If statements would get replaced with this line of code).

seadoggie01 commented 1 year ago

I've left the conditional in the __WD_JsonHandle since it would break scripts that already add {"handle":

Danp2 commented 1 year ago

I've left the conditional in the __WD_JsonHandle since it would break scripts that already add {"handle":

Yes, that's what I was expecting you to do. 👍

mlipok commented 1 year ago

little late but... Should we do the same here: https://github.com/seadoggie01/WebDriver/blob/26eb680fd85ac83934b2644d2c2d20e8469c41db/wd_core.au3#L590-L594

Danp2 commented 1 year ago

@mlipok I don't believe this is needed here, but I'd like the hear your reasoning why you believe it is needed.