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

Add support for ext_trait in properties macro #1149

Merged
merged 1 commit into from Aug 11, 2023

Conversation

ranfdev
Copy link
Member

@ranfdev ranfdev commented Jul 30, 2023

closes #1059. Currently the user must always specify the name of the ext_trait.
Note: the ext trait is inevitably defined in the same imp module where #[derive(Properties)] is called.

Sadly this feature won't support adding other methods generated by a signal macro, because macros can't share data and the entire trait definition must happen in one macro.

@bilelmoussaoui bilelmoussaoui added the needs-backport PR needs backporting to the current stable branch label Aug 6, 2023
use super::*;

#[derive(Properties, Default)]
#[properties(wrapper_type = super::Author, ext_trait = AuthorExt)]
Copy link
Member

Choose a reason for hiding this comment

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

It seems strange to me that this trait is defined and implemented by this macro. Usually you want to have more things in the Ext trait than just property setters/getters

Copy link
Member

Choose a reason for hiding this comment

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

I think that is fine, as long as the trait is named something like AuthorPropertiesExt, so you can also have a similar trait for auto generated signals in the future for example.

Copy link
Member

Choose a reason for hiding this comment

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

And that could also be a super-trait of the actual Ext trait. Maybe we should make that clearer in the docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now the trait defaults to MyTypePropertiesExt. I didn't specify anything about a possible super-trait MyTypeExt, because we don't have any macro to build that trait yet. The user can implement it as they wish.

@sdroege sdroege merged commit a993413 into gtk-rs:master Aug 11, 2023
47 checks passed
@bilelmoussaoui bilelmoussaoui added backported PR was backported to the current stable branch and removed needs-backport PR needs backporting to the current stable branch labels Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported PR was backported to the current stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support generating a trait with Properties macro
3 participants