Skip to content

Conversation

carlosmn
Copy link
Contributor

@carlosmn carlosmn commented Jul 4, 2025

Given a struct like

#[derive(Copy, Default, Clone, Debug, PartialEq, Eq, glib::Enum)]
#[enum_type(name = "SimpleEnum")]
pub enum SimpleEnum {
    #[default]
    One,
    Two,
}

you currently have to specify the default value as part of the param spec builder, e.g.

            #[property(get, set, builder(SimpleEnum::One))]
            fenum: RefCell<SimpleEnum>,

With this PR, we allow for that to change to

            #[property(get, set, default)]
            fenum: RefCell<SimpleEnum>,

as discussed in #930. This started with what @ranfdev wrote though it changed a little bit.

It's still a bit messier than I was hoping as you still have a few different ways of setting the default value, you can have

#[property(default)]
#[property(builder(Type::Default))]

which is fine for compat and because not every enum has a built-in default, but you could also do things like

#[property(builder(Type::Default).default_value(Type::OtherDefault))]
#[property(default, builder().default_value(Type::OtherDefault))]

though you shouldn't and probably wouldn't because why would you. It comes out of the generic nature of this and it's probably not worth catching this and erroring out, but it doesn't make the whole thing easier to understand.

The main thing here are Enums but I also added it for chars because they do have a default value in Rust. This might however not make as much sense in glib? I'm not sure if there's an actually-correct answer there.

As I mentioned in #930 (comment) I don't think you can make this an actual blanket implementation because you need to know a lot about the types and the builder function, which is arbitrary. But covering enums sounds like it's 90% of the use-case, and other libraries can still implement it themselves.

carlosmn added 4 commits July 2, 2025 09:47
…fault

This will be used by the `Properties` macro to use a default value as defined by
the type's own `Default` implementation.

As a first blanket implementation we set it for enums.
`char` has a default in Rust so we can implement in there though it might not be
relevant in all contexts. The enum implementation covers any enum that
implements `Default`.
When naked (rather than `default = 1`), this specifies that we should use the
param spec builder defined via `HasParamSpecDefaulted` as the default value.

This is particularly relevant for enums, for which we did a blanket
implementation in a previous commit.
@sdroege
Copy link
Member

sdroege commented Jul 7, 2025

I think this generally makes sense. thanks! @bilelmoussaoui ?

@carlosmn carlosmn marked this pull request as ready for review July 7, 2025 10:50
@bilelmoussaoui
Copy link
Member

I think this generally makes sense. thanks! @bilelmoussaoui ?

lgtm to me as well!

@sdroege sdroege merged commit 635d5cd into gtk-rs:main Jul 14, 2025
48 checks passed
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.

3 participants