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

Stop building and alias winxpgui + fix ietoolbar demo #2217

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Mar 19, 2024

As mentioned in https://mail.python.org/pipermail/python-win32/2024-March/014879.html

there's no longer a need given we no longer support XP at all. However,
for backwards compatibility we can't really drop the names - IOW, these
could all technically be rolled into a single .pyd, but from winxpgui import foo must continue to work somehow for all foo it worked for in
the past.

I went the very simple (and fast to "compile" 😉 ) route of having a pure python module that re-exports what we need.

>>> import winxpgui
>>> import win32gui
>>> import win32console
>>> winxpgui.GetConsoleWindow is win32console.GetConsoleWindow
True
>>> winxpgui.SetActiveWindow is win32gui.SetActiveWindow
True

To remove pywin32's own usage of winxpgui in code, I had to touch the ietoolbar demo, which I had to fix to test.

As for Internet Explorer, technically there's a mode in Edge to allow
some sites to run pages in "IE mode" (that thing really won't die,
will it?), but that's still through the Edge application.
The iebutton and ietoolbar demos technically do install an addon that
I can see and enable in "Internet Settings", but idk if they can
really do anything (and as such, if the demos are still valid).

In that case I see no reason to not consider them valid.

I could remove it now, or leave it for a follow-up PR to keep this well-contained

// @pyswig int, <o PyRECT>|GetWindowRgnBox|Returns the bounding box for a window's region
// @rdesc Returns type of region and rectangle coordinates in device units
int_regiontype GetWindowRgnBox(
HWND hWnd, // @pyparm <o PyHANDLE>|hWnd||Handle to a window that has a window region. (see <om win32gui.SetWindowRgn>)
RECT *OUTPUT);
// @comm Only available in winxpgui
#endif

Copy link
Collaborator Author

@Avasam Avasam Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be working perfectly fine for me:

>>> import win32gui
>>> gdi = win32gui.CreateRectRgnIndirect((0,0,1000,1000))
>>> window = win32gui.GetForegroundWindow()
>>> win32gui.SetWindowRgn(window, gdi, True)
>>> win32gui.GetWindowRgnBox(window)
[2, (0, 0, 1000, 1000)]

(even avoiding winxpgui to prove that it's working in this new build)

Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, but my main concern is users who upgrade without uninstalling - depending on the PYTHONPATH order, there seems a good chance upgrading users might still find themselves loading the old winxpguidmodule.pyd. I'm not in a position to test that currently - are you able to?

@Avasam
Copy link
Collaborator Author

Avasam commented Apr 11, 2024

If both the old .pyd and .py are found in the same repository, it is indeed likely importing the .pyd will take precedence.

Is that an issue? Most symbols were the exact same, built from win32gui. I guess we'd want to avoid a fresh install / full re-install from possibly causing some behavior change.
I'm not totally certain how to best handle that. I could "compile" the .py file with Cython or mypyc. to ensure it overrides the old winxpgui.pyd. Should still be faster and simpler.

Edit: or learn to do the exact same re-exports I'm doing, but in C (could maybe shortcut it using cythonize) and build the very simple module.

@mhammond
Copy link
Owner

Is that an issue?

Hrm, maybe not. I guess a risk is that later pywintypes.dll ends up changing symbols, meaning an old winxpgui fails to load. OTOH though, that seems an edge case so maybe it's fine.

@mhammond
Copy link
Owner

Ok, let's YOLO this given the benefit of killing this code is real and that risk I mentioned above is both theoretical and temporary

@Avasam
Copy link
Collaborator Author

Avasam commented Apr 11, 2024

If you're good with it, I'm good with it. It removes so much complexity and speeds up build times.

@Avasam Avasam changed the title Stop building and alias winxpgui Stop building and alias winxpgui + fix ietoolbar demo Apr 11, 2024
@Avasam Avasam merged commit 7711a57 into mhammond:main Apr 11, 2024
27 checks passed
@Avasam Avasam deleted the remove-winxpgui branch April 11, 2024 16:30
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