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

Support named args for dispmethod #486

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

junkmd
Copy link
Collaborator

@junkmd junkmd commented Jun 10, 2023

#371

I was trying to implement the #400 functionality.
However, I realized that it is misleading to have a function annotation that allows named arguments to be accepted, since a dispmethod called via IDispatch.Invoke will raise an error when a named (with default value) argument is passed at runtime.

So, I improved the process of instantiating DISPPARAMS so that it can also accept named arguments in dispmethod.
Using _argspec passed to IDispatch.Invoke, it completes default values for optional arguments that were not passed, to ensure that the passed argument names are correct.

This added functionality is only triggered when named arguments are passed to dispmethod.
Same as before, when just only positional arguments are passed, the VARIANT array is assigned to DISPPARAMS.rgvarg in the reverse order.
Therefore, this change does not break backward compatibility.
In the case that an element of argspec is not what the newly added processing expects, and an error is raised when passing named arguments, it can be avoided by passing only positional arguments.

The bound_named_property has also been improved to allow passing named arguments.

As for testing, I used Excel to verify that it is possible to pass named arguments to a dispmethod.
I have Excel in my development environment, so I can run this test and make sure it passes.
However, AppVeyor CI does not have Excel, so I am not able to fully guarantee End2End operation with CI.
Instead, I generously cover the test of DISPPARAMS instantiation process, which can be executed independent of Excel and the environment.
commethod (and stdmethod) were originally designed to accept named arguments, but there were no tests to do so explicitly, so I added them.

If there is a dispmethod in an environment-independent library that can easily provide fixtures, I will try to test with that object as well, so I am looking for feedback.

junkmd added a commit to junkmd/pywinauto that referenced this pull request Jun 10, 2023
@junkmd junkmd marked this pull request as ready for review June 11, 2023 04:11
@junkmd
Copy link
Collaborator Author

junkmd commented Jun 11, 2023

@vasily-v-ryabov

Please review this.

Not to break backwards compatibility, but since this is more of a "feature addition to allow named arguments to be passed" than a "bug fix for not being able to pass named arguments", and since there are many lines of codebase changed.

If we decide not to incorporate this into the 1.3.0 like #475, I would move #400 and this from the 1.3.0 milestone to next version milestone.

@junkmd junkmd added this to the 1.3.0 milestone Nov 9, 2023
@junkmd junkmd removed this from the 1.3.0 milestone Jan 6, 2024
@junkmd junkmd changed the base branch from drop_py2 to main February 5, 2024 02:22
to help calling `IDispatch.Invoke`.

The processing order for assigning variables to fields in `DISPPARAMS`
is the same as before 62ce303.
The change in processing order should have no effect, but after
committing 62ce303, I reconsidered trying to keep the previous
processing order as much as possible.
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.

None yet

1 participant