jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

Gtk::TreeStore#model returns the C pointer instead of Crystal wrapper #54

Closed hugopl closed 4 years ago

hugopl commented 4 years ago

~This may be related to the last fixes on signals, this example shows that the first signal parameter is a union type instead of a Crystal gobject wrapper.~

require "gobject/gtk/autorun"

builder = Gtk::Builder.new_from_file("#{__DIR__}/../lib/gobject/samples/gtk_tree_view.glade")
builder.connect_signals

# Insert something into the model
model = Gtk::TreeStore.cast(builder["tree_model"])
root = Gtk::TreeIter.new
model.append(root, nil)
model.set(root, [0], GObject::Value.new("Root"), 1)

# view
view = Gtk::TreeView.cast(builder["tree_view"])
view.on_row_activated do |view, path, column|
  model = view.model.not_nil!

  puts "model type:  #{typeof(model)}"
  puts "path type:   #{typeof(path)}"
  puts "column type: #{typeof(column)}"
  if model.is_a?(Gtk::TreeStore)
    puts "Crystal friendly object"
  elsif model.is_a?(Pointer(LibGtk::TreeModel))
    puts "Got the C pointer!?"
  else
    abort("caos")
  end
end

# Show main view.
main_window = Gtk::Window.cast(builder["main_window"])
main_window.show_all

Output here after I double click in the tree view row:

model type:  (Gtk::TreeStore | Pointer(LibGtk::TreeModel))
path type:   Gtk::TreePath
column type: Gtk::TreeViewColumn
Got the C pointer!?

Tested on archlinux, crystal 0.34, crystal-gobject at 4f1e6c61cd603e9236c236758df3240183e2e2ad.

hugopl commented 4 years ago

Ooops, I made a confusion... the signals seems ok, the problem is with view.model return type. sorry, should I close this bug and create another one?

jhass commented 4 years ago

It's fine, maybe just retitle it :)

jhass commented 4 years ago

Soo... returning a wrapper for interfaces, which Gtk::TreeModel is one, simply wasn't implemented yet :D

The little bit weird typeof result probably is a type interference bug (or maybe not since the block is captured?) caused by reusing the model variable in the block. typeof(view.model) showed just the pointer type and Nil.

hugopl commented 4 years ago

nice! great work! I'm doing a pet project in Crystal/GTK, so I may submit more issues as I found.

Also... should I submit PRs "Crystalyzing" some APIs (as I need them for my pet project :-P)? Since the GTK API is too verbose due to C nature itself, and we don't need to repeat this verbosity.

jhass commented 4 years ago

Also... should I submit PRs "Crystalyzing" some APIs (as I need them for my pet project :-P)? Since the GTK API is too verbose due to C nature itself, and we don't need to repeat this verbosity.

Yes, definitely. I started this project because I wanted a libnotify binding, so for that I already did that. Just put them under src/gtk (or whatever the respective library) for now. Eventually when they're big enough they should move into their own shard of course.

If you recognize any patterns, it's also totally a possibility to try to handle them in the generator. Like it already replaces get_foo with just foo and set_foo with just the self arg and one paramater with foo=.

hugopl commented 4 years ago

cool, I'm currently doing these "crystalization" in my project itself by re-opening the classes, once I feel they are ok I submit a PR or a issue describing a pattern.

First thing I think is about to replace GValue by a Union type, I remember when I was working on PySide (Python-Qt bindings) and one of the goals was to make QVariant (Qt's GValue) transparent to users and the result was nice, however I still need to gather a bit more expertise on GTK to analyse the side effects of such suggestion.

jhass commented 4 years ago

The GValue implementation certainly is a bit wonky still, largely because there are things I don't understand about it. Like is INT always 32bit or platform dependent? What's the difference between INTERFACE and OBJECT? Can we always just use POINTER and store an opaque one for reference types? Etc.

jhass commented 4 years ago

Alright, I spend some time improving GObejct::Value. It now acts as union wrapper in the vein of JSON::Any/YAML::Any. It's still not handling 100% of the possible values, mainly because I'm pretty clueless on boxed, variant, param and interface.

I also made the generator detect a single GValue out parameter and have it return a GObject::Value instead. Along the way most wrapper arguments gained type restrictions.

hugopl commented 4 years ago

Awesome!

I think a similar pattern could be done withGtk::TreeIter, it's mostly used as a out parameter. However in the case of Gtk::TreeIter would be nice to keep the C-like versions using out parameters available just in case people want to re-use the Gtk::TreeIter while appending hundred of rows to a model to avoid some allocations.

in my pet project I do things like:

module Gtk
  class TreeView
    def value(path : TreePath, column : Int32) : GObject::Value
      view_model = model.not_nil!
      iter = Gtk::TreeIter.new
      view_model.iter(iter, path)
      value = GObject::Value.new
      view_model.value(iter, column, value)
      value
    end
  end
end

to to simplify things, and a similar approach for ListStore#append.

OTOH there's no much Gtk::TreeIter use cases, and it's not too generic/wide used as GValue, so I would be in doubt about put such behavior in the generator or let all this be hand-made API modifications.

jhass commented 4 years ago

OTOH there's no much Gtk::TreeIter use cases, and it's not too generic/wide used as GValue, so I would be in doubt about put such behavior in the generator or let all this be hand-made API modifications.

Yes exactly. I think a handmade API would be more appropriate here, possibly using macros to iterate through the generated methods or keep the boring part down otherwise.

jhass commented 4 years ago

In other news master cleans up the value : Enumerbale, n_values : Int style APIs now to just compute n_values in the wrapper. I figured out that'll be a generic enough pattern to be worth it.