-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Replace builders with generated traits on ObjectBuilder #1417
base: main
Are you sure you want to change the base?
Conversation
17e6a81
to
37eb9a1
Compare
module_name, analysis.name | ||
"\tpub use super::{}::{}BuilderExt;", | ||
module_name, | ||
analysis.builder_trait_prefix() |
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.
Can't this return the actual builder trait name instead?
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.
Which one? I had to do something like this because otherwise it gets the same conflicts as with the normal trait names (e.g. ObjectExt
WindowExt
ApplicationExt
, etc)
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.
Looks generally good to me, nice cleanup :)
Can you create a PR to show the changes before we merge this here?
I think something needs to be done in gtk-rs-core about the missing |
Sorry meant gtk3-rs and gtk4-rs, those seem to be the only places using it |
37eb9a1
to
1485bdd
Compare
@jf2048 @bilelmoussaoui What's the plan/status here? |
It is a real pain to modify that wrapper macro to do what we need to do here |
Alternate solution to #1391. Tested with gtk4 it removes around 40,000 lines of code.
I had to remove
builder_postprocess
too. For that I would like to somehow come up with a way to fold the setup intoFromGlibPtr
andFromValue
.