-
Notifications
You must be signed in to change notification settings - Fork 1.3k
_tkinter pt. 2 #5640
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
_tkinter pt. 2 #5640
Conversation
b6f0d3b
to
471d304
Compare
The existing tk/tcl libraries seem to use inwelling to generate bindings https://github.com/oooutlk/tcltk/blob/main/tcl/src/lib.rs#L150-L159 If we switch to bindings, how do you think we go similar with those approach to avoid common pitfalls and find a shared point with them in future? |
I took a look a while back, before writing custom crates. It seems Currently I have a few problems with this approach.
I blacklist these macros here: https://github.com/arihant2math/tkinter/blob/d9874a8446cd32dfcd3e71b8e891329f304d2a3b/tk-sys/build.rs#L113 and re-implement them here: https://github.com/arihant2math/tkinter/blob/2957564c74fbb4f25b967f6ac1e2289ed04fd0c0/tk-sys/src/lib.rs#L7 Seeing that they crash the linker if included, unless patched, ... I doubt these libraries are handling memory management well.
I think with bindings we will be able to mirror the cpython implementation well enough that most issues should be avoided. I think the current set of tk libraries aren't ready for something as broad as tkinter. |
Also I would ideally have contributed to https://github.com/MisterEggnog/tcltk-rs, but it seems to be unmaintained and the author seems to be inactive. |
Thank you so much for explanation |
2de72c4
to
5c4df62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good.
Are you going to individually maintain tk bindings? Otherwise could you consider to transfer the repository to RustPython org?
I don't have too much of a use for them, so I'll send a transfer request after I clean it up and add basic CI. |
5c4df62
to
be45915
Compare
Also since I'm not an admin of the RustPython org, I'd have to transfer it to you first, and you can transfer it to the org. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this headache driving work
Switches to custom bindgen crates and does some implementation work (
GetVar
is still broken however).Also ...
building on macos/linux is broken, but that can be fixed on those cratesmacos/linux require pkg-config now.