mhammond / pywin32

Python for Windows (pywin32) Extensions
5.01k stars 792 forks source link

Extend PyWin_SetAPIError #2188

Closed CristiFati closed 6 months ago

CristiFati commented 7 months ago

Split this out of #2183.

This change by itself is much more significant than the other one, so it should be stand alone (rather than a prerequisite).

So far, anything is unchanged functionally (all the places that call it). If this behavior (returning None on non error, instead of raising PyWinExc_ApiError(0, "No error message is available")) becomes the default, the (new) default argument value should be changed. But (although I don't know what the module backward compatibility / deprecation policy is), I'd just deprecate it for now:

mhammond commented 6 months ago

TBH I'm not sure the right thing to do is have PyWin_SetAPIError not set an error - wouldn't it be clearer and cleaner, if slightly more verbose, to just have each call site check the code and explicitly return None there?

The question about how to change those call-sites without breaking b/w compat is a good one though and one I need to think through a little more. I do like the idea of a deprecation warnning for a few releases - in which case some helper might still make sense - I just don't think it should be inside PyWin_SetAPIError

CristiFati commented 6 months ago

Sorry for the late reply, currently on vacation with sporadic access / time.

Moved the functionality in a different (wrapper) function.