gtkd-developers / GtkD

GtkD is a D binding and OO wrapper of GTK+ originally created by Antonio Monteiro
http://gtkd.org
Other
322 stars 71 forks source link

Enhancement: Have Container.getChildren() return Widget[] instead of ListG #138

Closed gnunn1 closed 8 years ago

gnunn1 commented 9 years ago

I needed to get a list of the widgets held within the container and naively tried converting the ListG to Widget[] which failed miserably. After looking at the source code for Container, I can see why it failed since ListG contains a list of GtkWidget* instead of Widget. I wrote the following function to get the list of children as Widget[] and it seems to work, would make sense to replace the Container.getChildren with a similar method as I think the philosophy in GtkD is to generally avoid making developers work with GtkWidget* directly.

Widget[] getChildren(Container container) {
    ListG list = container.getChildren();

    Widget[] result = new Widget[list.length()];
    size_t count;

    while(list !is null && count < list.length) {
        result[count] = ObjectG.getDObject!(Widget)(cast(GtkWidget*) list.data);
        list = list.next();
        count++;
    }
    return result;
}

If it makes sense and you want a pull request, I'm happy to do so.

MikeWey commented 9 years ago

Changing the return type of a function would be a breaking change, and the Lists have a toArray method:

container.gerChildren().toArray!Widget();
gnunn1 commented 9 years ago

Thanks, unfortunately ListG.toArray directly to Widget didn't work for me since in this case because ListG holds GtkWidget* rather then Widget. For me the issue is whether it's reasonable for someone working with GtkD to have know how to cast GtkWidget* to a Widget via the line:

ObjectG.getDObject!(Widget)(cast(GtkWidget*) list.data)

It just strikes me as being non-intuitive but maybe that's because I'm coming from a primarily Java background (no pointer/structs) versus a C background and am pretty new in GTK. I do understand this is a breaking change, maybe an alternative method like getWidgetChildren()? If you don't agree, no worries just close this as a will not fix.

MikeWey commented 9 years ago

ListG.toArray should detect the C type of the Widget, so passing it shouldn't be necessary. If this isn't the case it might be a bug, but the unittest seems to pass.

gnunn1 commented 9 years ago

I built a reproducer and of course it works fine in that context, I must have been screwing something up in my original code, sorry about that.

One question on this though, looking at the code to ListG, the toArray always creates a new wrapper object around the struct. Would it be better if if this code used the ObjectG.getDObject instead to grab the existing wrapper, if there is one, then creating a whole new one?

MikeWey commented 9 years ago

It should indeed be using ObjectG.getDObject.

gnunn1 commented 9 years ago

OK, found the problem with my code and it is because the ListG!toArray generates a new reference and my code was sloppy expecting the references to match up and Seg Faulting on a null reference when it didn't. Ah the joys of learning a new thing. If you fix the ListG (or do I need to do a pull request?) to use ObjectG.getDObject. it will indeed to fix my problem.

Just out of curiosity, if you want to know if two widgets are equal (i.e wrap the same GTK3 widget), is the best practice to compare the pointer returned from getWidgetStruct or getStruct? I assume if the pointers are the same the GTK3 widget is the same even if different D objects are wrapping them?

MikeWey commented 9 years ago

I've switched it to getDObject in commit 7562e5bc1d12c4834e7ebdf3c42211babc426c05.

getWidgetStruct and getStruct return the same pointer but getStruct is protected so you would be using getWidgetStruct.

gnunn1 commented 8 years ago

Thanks Mike.

russel commented 6 years ago

@gnunn1 Do you have a reference to an example of doing this comparison of a widget reference to a widget held in a container? I find myself needing to do the same.

I have posted a question on the GtkD forum: https://forum.gtkd.org/groups/GtkD/thread/787/

@MikeWey

gnunn1 commented 6 years ago

I replied on the forum, hope it helps.