diamondburned / gotk4

Autogenerated GTK4 bindings for Go
GNU Affero General Public License v3.0
515 stars 20 forks source link

Casting *glib.Object to non-gtk object requires extra casting step #86

Open MJacred opened 1 year ago

MJacred commented 1 year ago

Specs

Problem

what I want to achieve:

EDIT: ok, the list model should give *gio.File objects in this case, so no issue here. Though there are probably other cases? if not, this issue can be closed. (code is still untested, as refactoring from gotk3->gotk4 is work in progress)

problem

steps

Solution

  1. is this the wrong approach? or
  2. can return type of existing Cast*() funcs be changed to interface{}? or
  3. adding alternative Cast*() funcs that return interface{}? (or sth. along the lines)
diamondburned commented 1 year ago

now I need to do value.(interface{}).(string) to get my string, because string does not implement this interface

This is really weird and probably doesn't work as well as you expect it to, because string doesn't implement Objector, so it'll crash right in the function.

Are you sure this isn't a GValue or something like that?

MJacred commented 1 year ago

Are you sure this isn't a GValue or something like that?

see EDIT in my description -> it is a GValue (placement was not that good, I guess)

Though the primitives are also valid GValues. So it might pose a problem later, if there's a case where the list consists of primitives (haven't found a case, yet)

Closing issue for now?

diamondburned commented 1 year ago

Yes, though this is one of those scenarios where it's extremely important to defer to the official GTK API documentation:

According to Gtk.FileChooser.get_files, the type of each object in the returned list is a GFile, not a string! You'll need to do f.CastObject().(gio.Filer) to get the GFile in the most correct way possible (there's no guarantee that the returned type is anything concrete, but it must implement the Filer interface).

Admittedly, the method should be Item(n uint) glib.Objector, not Item(n uint) *glib.Object. In fact, since Go has generics now, we can even have a ListStore[Filer] type. Since the whole package has no stable guarantees at the moment, I might eventually get to changing it to work better.

For now, feel free to close this issue.

MJacred commented 1 year ago

Sorry, I should have quoted it directly from my original description (I noticed that it returns *gio.File objects due to the docs and amended it right away):

EDIT: ok, the list model should give *gio.File objects in this case, so no issue here. Though there are probably other cases? if not, this issue can be closed. (code is still untested, as refactoring from gotk3->gotk4 is work in progress)


Admittedly, the method should be Item(n uint) glib.Objector, not Item(n uint) *glib.Object

From what I can see, glib.Objector is never returned from any method, or taken as parameter (except in the new() funcs). The same goes for e.g. gio.ListModeller.

Isn't that why there's func (v *Object) Cast*() Objector? At least if you get an object from the API.

But what about the other way around? As these funcs don't take an interface as input, what is the best approach to pass in an object as parameter? glib.BaseObject()?

Do you intend to change this that the interface shall be taken as input parameter and return value?

diamondburned commented 1 year ago

From what I can see, glib.Objector is never returned from any method, or taken as parameter (except in the new() funcs). The same goes for e.g. gio.ListModeller.

That's definitely weird in hindsight. I tried digging through git blame and wasn't able to find a reasonable explanation as to why I did that.

Isn't that why there's func (v *Object) Cast*() Objector? At least if you get an object from the API.

No Cast() is implementation detail at best and broken at worst.

Again, it's important to emphasize that the function doesn't necessarily return a *gio.File, although it may. What it (and the generator) actually does is traversing the type tree:

A few things to note here:

FWIW I think it's intentional that the generator generates a ListModel by itself which just returns a GObject with no hint as to what the actual type may be (until later on), so there's not much that it can do. Still, returning an Objector seems to be the better idea here, it'll only add some cost to do type conversion.

MJacred commented 1 year ago

Again, it's important to emphasize that the function doesn't necessarily return a *gio.File, although it may […]

Ah, yes, indeed. Thanks for the clarification.


After a simple search I found ~80 instances where Objector should have been returned/taken.

There are similar cases for other types where the interface version should be returned I reckon, such as for func (self *DropDown) Model() *gio.ListModel.

I guess the affected interfaces need to be identified one by one?

diamondburned commented 1 year ago

Hmm, in an ideal world, I think we'll generate a type ListModel[T Objector] struct, but I'm not sure how feasible that actually is. I think maybe making it all return Objector is good enougn. Yet another special case in the code generator.

MJacred commented 1 year ago

Code generators are black magic either way.

Shall I update issue title & description to reflect the issue of "use interface instead of struct for param/return type, if necessary" or close this one and create new issue?

diamondburned commented 1 year ago

I think this issue is fine as it is.

MJacred commented 1 year ago

Ok, then I'll leave title and description as-is and we'll leave the issue open until the struct type is replaced by the interface type