fix broken GValue setters and make setter methods public

[osa1]

I suggest paying attention to warnings printed by GTK, in my experience they almost always indicate bugs.

[osa1]

I wasn't aware of GValue::init() so I added init() calls in setters. But now thinking about it, I say let's remove GValue::init. Setting the type and then using type-specific method is just redundant work. e.g. this is too painful:

let mut value = gtk::GValue:new(gtk::g_type_enum::Boolean).unwrap();
value.init(...);
value.set_boolean(true);

We can skip init(). Maybe we can even have this:

GValue::new_boolean(...);
GValue::new_string(...);

etc.

How does that sound?

[GuillaumeGomez]

I did this like this for a reason but I can't remember which one... GTK+ did split initialization and settting like I did. Take a look at this example. You're supposed to be able to change the value hold by the GValue type. If you create a constructor for each type, you're breaking this.

[osa1]

If all we want to provide is wrapper for the C API, then we should keep 3-step initialization. On the other hand, if we want to provide good abstractions we can have 1-step initialization. I think we can even parameterize GValue on Rust types, like this:

pub struct GValue<V: GValuePublic> { ... }

[osa1]

Anyhow, I'll revert my first commit and make setters/getters public. Rest can wait for discussions I think.

[osa1]

I updated the patch.

[osa1]

Added implementation of CellRendererToggle.

[jeremyletang]

hi,

Can you set back the function you put public as private ? We want the only way to use the GValue is using set or get method.

We don't want to provide an exact wrapper to the C API, and try to make abstraction as much as we can. But in the case of GValue we cannot make initialisation in 1-step, We cannot parametrize the struct with the inner type as the inner type can change during runtime. So we can reduce the initialisations steps by adding a get_type function into the trait GValuePrivate and retrieve the type from the value at the moment that the function set is called.

[osa1]

Can you set back the function you put public as private ?

Can you point me the specific line? Which function are you talking about?

[GuillaumeGomez]

The ones that are implemented in the GValuePublic trait. All the linked functions are supposed to remain private.

[osa1]

Guys, sorry, I'm having trouble understanding this. Can you please point me the specific lines? I don't think I did any changes in GValuePublic. I only changed implementation of GValue and I think that was necessary because otherwise there's no way to set value of a GValue.

[GuillaumeGomez]

Okay, let's take an example. It will be more easier to understand I hope. For example this line calls the get_int method from gvalue. So when you'll use the i32 type with gvalue, the trait will be use.

The rust-guideline says (I make it very short here) that it's better to only have one way to do one stuff. So in this case, since there is the i32 type does have the GValuePublic trait, there is no need to keep the get_int and set_int methods public.

I hope my explanations helped you. Don't hesitate to ask if you're still not sure about something.

[jeremyletang]

@osa1 see this line, the function set method from the trait GValuePrivate which is implemented by all possible value for a GValue

[osa1]

Thanks for the help. I didn't see the generic set<T: GValuePublic> method. I removed the commit from pull request.

Note that there are still some public setter methods that were previously pub, like set_object, set_pointer, set_enum etc. Why are those public?

[jeremyletang]

yes, there is some public setters, but we are not sure how we want to handle them safely for the moment, so they stay public.

Anyway, I'll merge your changes. thanks !

[jeremyletang]

oups, it's seems that this cannot be merge for the moment.

[osa1]

I rebased the patch.