gir.core
gir.core copied to clipboard
GInterface properties
Hello @badcel, @firox263 ,
I think we have to review the way to handle GInterface properties. I'm currently working on the widget catalog samples, I'm seeing that for now the generator is creating empty GInterfaces. I think we can at least generate an inner Native
class which will contain the list of property names of that interface, so for example in the Box
class we will have:
public static readonly Property<Orientation> OrientationProperty = Property<Orientation>.Register<Box>(
Orientable.Native.OrientationProperty,
nameof(Orientation),
(o) => o.Orientation,
(o, v) => o.Orientation = v
);
Also, since the Orientable
interface for example can be used by many classes, I think it will be better to have the property descriptor in the interface, but not the properties.
public partial interface Orientable
{
#region Properties
public static readonly Property<Orientation> OrientationProperty = Property<Orientation>.Register<Orientable>(
Native.OrientationProperty,
nameof(Orientation),
(o) => o.Orientation,
(o, v) => o.Orientation = v
);
Orientation Orientation { get; set; }
#endregion
#region Native
public static class Native
{
public const string OrientationProperty = "orientation";
}
#endregion
}
So in the implementations like Box
we have:
public partial class Box : Orientable
{
public Orientation Orientation
{
get => GetProperty(Orientable.OrientationProperty);
set => SetProperty(Orientable.OrientationProperty, value);
}
}
Like that we will not rewrite property descriptors in every implementations (what seems strange for me...).
What do you think ?
@na2axl :I think we can at least generate an inner Native class which will contain the list of property names of that interface
Yes you can do this as we have the same for classes already :+1:. The Native
class should probably be protected
or internal
.
I think it will be better to have the property descriptor in the interface
This is the big question at first I intended to do this in the interface PR but then skipped over it, because for classes the property descriptors are public. And if we implemet an interface in the classic way, like we decided in #99, then the classes would have the public properties of the interface, too. But if they have the public properties, they should have the public property descriptors of the interface, too. But if we use the default interface implementation feature for the interface property descriptors, those are not visible on the class. This is the reason why i skipped it, it is just not consistent.
@na2axl what are your thoughts on this?
We could consider adding them as a temporary workaround as long as the generator is not rewritten #100, too.
But if we use the default interface implementation feature for the interface property descriptors, those are not visible on the class. This is the reason why i skipped it, it is just not consistent.
@badcel I think this is not really necessary, if we implement descriptors in interfaces we can always use them anywhere else, even when doing something like:
var box = new Box(Orientation.Vertical, 16);
Orientable.OrientationProperty.Set(box, Orientation.Horizontal); // This will work because Box is an Orientable
Or in object initialization/bindings:
var box = new Box(Orientation.Vertical, 16)
{
// We have not properties indexers yet, but it's planned ;-)
[Orientable.OrientationProperty] = Orientation.Horizontal,
};
Also this can make sense to users because in the real life the orientation
property is defined in the Orientable
GInterface, so it's a bit normal to see the descriptor of that property in our Orientable
interface.
@na2axl Yeah I think it is matter of preference. I think I would prefer the solution which is better from an API point of view, not from a code generation point of view.
For myself I have currently no strong opinion on this. For me both solutions are okay (implementing property descriptor in class vs implementing in interface).
I think it is a little bit easier to find the descriptor, if it is on the class for the user. We would need no additional documentation for this. If they are on interfaces it should probably be clarified in the documentation.
Otherwise, what about generating the descriptors for both?
I think I would prefer the solution which is better from an API point of view, not from a code generation point of view.
Ok so we are on the same side 🙂. From an API point of view, I think implementing GInterface properties in C# interfaces will have the same behavior of Attached Properties in XAML-based languages (it's another concept of the DependencyProperty
family). An attached property is just a property which is declared in a class and not used by this one, but instead by another classes. For example we have the Grid
widget, which have the descriptors of ColumnSpan
and RowSpan
properties, but those properties will never be used by the Grid
itself but instead by its children. So in XAML we will have something like:
<Grid Columns="2" Rows="2">
<Image Source="test.png" Grid.Column="0" Grid.Row="0" Grid.RowSpan="2" />
<Label Weight="Bold" Color="Red" Grid.Column="0" Grid.Row="1">My Image</Label>
<Button Click="OnDeleteButtonClick" Grid.Column="1" Grid.Row="1">Delete</Button>
</Grid>
And even in C# we can have something like:
Grid.ColumnProperty.Set(label, 1);
Grid.ColumnProperty.Set(button, 0);
So in other frameworks it's already normal to see something like that from an API point of view.
I think it is a little bit easier to find the descriptor, if it is on the class for the user. We would need no additional documentation for this. If they are on interfaces it should probably be clarified in the documentation.
Yes you are right, the way we choose must be documented. But we also don't have to forget that peoples coming here will have at least a little knowledge on GTK and his GNOME friends, so from my point of view if they know that the orientation property is defined in the GtkOrientable interface it will surely not a surprise to see the C# descriptor of that property in an Orientable
interface.
Okay, so let's do it. I think we will see how we like the feature if we use it more in practice. :+1: