Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

WIP: macro for *Impl, *ImplExt, IsSubclassable boilerplate #235

Closed
wants to merge 1 commit into from

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Dec 23, 2020

Just a prototype, but this seems like it could be helpful, in the absence of full gir-based generation. Not sure if this method will work for all implementations.

@@ -64,6 +64,7 @@ glib = { path = "../glib" }
gdk = { path = "../gdk" }
gdk-pixbuf = { path = "../gdk-pixbuf" }
pango = { path = "../pango" }
paste = "1.0"
Copy link
Member

Choose a reason for hiding this comment

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

Add a hidden re-export of paste to glib and use that in the macro. It's not very nice if using a macro requires users to add other dependencies :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this was just a quick hack for testing before trying to re-export it correctly

@sdroege
Copy link
Member

sdroege commented Dec 24, 2020

The only part that I found that wouldn't work for all cases is the override_vfuncs part. See e.g. https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/blob/master/gstreamer-base/src/subclass/base_transform.rs (note how transform, transform_ip and transform_ip_passthrough are handled).

Otherwise this should help to reduce quite a bit of boring boilerplate.

@bilelmoussaoui
Copy link
Member

It would be nice to have something similar for IsImplementable

@ids1024
Copy link
Contributor Author

ids1024 commented Dec 24, 2020

The only part that I found that wouldn't work for all cases is the override_vfuncs part.

Perhaps the macro could have a variant that adds it, and one that doesn't, since it seems a standard implementation would handle most cases.

The other issue is that it potentially just makes the subclassing code potentially even more arcane, since to properly understand it one would have to understand the implementation of the macro as well as the other code where it's used. But I think it's probably better overall (still theoretically as a "temporary" solution before gir generation can be sorted out).

@sdroege
Copy link
Member

sdroege commented Dec 24, 2020

Perhaps the macro could have a variant that adds it, and one that doesn't, since it seems a standard implementation would handle most cases.

Yeah that could become configurable in some way :)

The other issue is that it potentially just makes the subclassing code potentially even more arcane, since to properly understand it one would have to understand the implementation of the macro as well as the other code where it's used. But I think it's probably better overall (still theoretically as a "temporary" solution before gir generation can be sorted out).

Not worse than glib::wrapper! IMHO, and it reduces the boilerplate a lot. I think it's fine, and if it confuses someone then that's mostly a matter of lacking documentation.

@ids1024
Copy link
Contributor Author

ids1024 commented Dec 24, 2020

Not worse than glib::wrapper! IMHO, and it reduces the boilerplate a lot. I think it's fine, and if it confuses someone then that's mostly a matter of lacking documentation.

It does define two traits with paste! and require the code using the macro to understand just as well how the traits involved work, so it does a fairly bad job at hiding how it works, as macros go.

But currently I think even the people who understand how subclassing works in gtk-rs are afraid to implement it for new types due to the Lovecraftian level of boilerplate, so it probably can't be worse... 😆

@ids1024
Copy link
Contributor Author

ids1024 commented Dec 24, 2020

Or, I guess if someone is trying to implement IsSubclassable and related boilerplate now, they'll copy from another implementation. After this macro, they'll do the same, but it will just happen what they copy has a macro call, is shorter, and requires fewer changes.

So no worse.

@sdroege
Copy link
Member

sdroege commented Dec 24, 2020

But currently I think even the people who understand how subclassing works in gtk-rs are afraid to implement it for new types due to the Lovecraftian level of boilerplate

Not really, check the gtk4-rs repository. It's just that nobody who understands how it works cares enough about gtk3 to add more things there, and in GStreamer I've already added it for all the things that make sense to be subclassed so... :)

So no worse.

Definitely better, IMHO!

@ids1024 ids1024 force-pushed the issubclassable branch 3 times, most recently from 6112ba9 to ebe878e Compare December 24, 2020 18:06
Just a prototype, but this seems like it could be helpful, in the
absence of full gir-based generation. Not sure if this method will work
for all implementations.
@sdroege
Copy link
Member

sdroege commented Dec 25, 2020

It would be nice to have something similar for IsImplementable

If you're looking for a complicated case for that one: https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/blob/01b16fe6c0a55cb40cfb87113fe2d165c91078e0/gstreamer/src/subclass/uri_handler.rs#L18-41 . Note also that not all the functions are not &self

@ids1024 My plan is to move to associated constants for a few of the class methods we have as part of #31 . Similar to how that does it already for properties, same will be done for signals and I think for GTK it would make sense to make part of the template machinery like that (and in GStreamer pad templates, element factory metadata, etc.).

Can you see a way how that could be covered here? Or maybe it's fine to not cover it as the number of classes/interfaces where such a thing is needed is going to be relatively small.

@GuillaumeGomez
Copy link
Member

Needs to be re-opened on gtk-rs-core.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants