-
Notifications
You must be signed in to change notification settings - Fork 807
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 named arguments in win32com.client.dynamic.Dispatch #2150
base: main
Are you sure you want to change the base?
Conversation
If any `Missing` argument value is encountered, argument processing stopped and any later arguments are not processed. This causes us to silently ignore named arguments that are later in the argument order. The value of pythoncom.Empty is chosen to be consistent with the genpy code.
This has the same problem I mentioned in #2149 - back in the day this would cause some objects to break. There was no single answer I could find that worked everywhere so I erred on the side of b/w compat. Sadly I'm not sure how true that is today, but also aren't really in a position to test and find out. |
The current state is unfortunately rather broken and has repeatedly been causing Issues for our customers over the last few years. Last week we again got a support request saying something like: "I use someObj.functionA(doTheThing=true) of your application, but it stopped doing The Thing. It hasn't worked all week. It worked last week, but it just stopped working around noon on Monday". The fact that only one of the I would argue that fixing that inconsistency is worth the backwards compatibility risk. Also, the
And what's described in the literature:
|
Sadly this is true in various other cases too, particularly when trying to index items - eg, The general solution/advice there is to use
I can agree with that in general, but the question is still who exactly breaks, and does that influence our choice? ie, it's one thing for the docs to say what they say, but it's another thing entirely if one choice makes working with (say) Office worse. I'm not saying it will, but really just saying I'm a little reluctant to make such changes blind to what the actual consequences are to heavily used COM objects. It might also be the case that the fact we (say) made things worse for Office simply points at other issues we should address at the same time, which would mean there's no need to choose one over the other. |
Yes, that obviously solves the issue, but only after somebody has successfully noticed it and identified the cause.
Anything that's implemented in C++ using the usual MS Toolchain shouldn't break, because calling DispInvoke() fills in the missing parameters in exactly the same way. And given that EnsureDispatch/makepy have been behaving this way for 21 years now, I would be rather hopeful that somebody would have reported any serious breakage with a popular application. Clearly, various people are using e.g. Word with EnsureDispatch: [1] [2] [3]. Is there anything that would make you more confident that this is safe? I fixed and ran |
Problem
Given an IDispatch interface with a function
The following code misbehaves:
Instead of passing the value of
arg2
, it is silently ignored. So the behavior is identical tod.MyFun()
.Caused By
The dynamic client uses
pythoncom.Missing
as its default value for non-given arguments.But if any
Missing
argument value is encountered, argument processing is stopped and any later arguments are not processed. This causes us to silently ignore named arguments that are later in the argument order:pywin32/com/win32com/src/PyIDispatch.cpp
Lines 369 to 375 in f7d0a79
Context
This was already fixed by 6a3be4b for genpy-generated interfaces, but the dynamic interface was seemingly forgotten back then. This somewhat worsens the issue because now the two interfaces have different behavior.
Fix
See PR.
The value of
pythoncom.Empty
is chosen to be consistent with the genpy code. Usingpythoncom.ArgNotFound
seems more correct to me, but that should probably then be changed in both implementations and is a separate issue.