[Bug 26249] add GObject-Introspection and Vala bindings

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=26249

--- Comment #7 from Danielle Madeley <danielle.madeley at collabora.co.uk> 2010-04-13 22:17:39 PDT ---
(In reply to comment #6)
> Useful references: http://live.gnome.org/GObjectIntrospection/Annotations and
> http://live.gnome.org/GObjectIntrospection/WritingBindingableAPIs
> 
> Please say in NEWS in this branch that gtk-doc 1.14 is strongly recommended (or
> even just increase the dependency). I fixed a bug that would make it truncate
> annotated doc-comments at the first newline, which broke much of our
> documentation; 1.14 has that fix.

I bumped the dependency. Seems like the right choice.

> > +       sed -re 's/gpointer self/TpProxy *self/' < proxy.h > proxy-introspectable.h
> 
> Why are you using --regexp-extended? This looks unnecessary (and probably not
> portable); just use sed -e 's/.../'.

Habit. Fixed.

> Please put the temporary proxy-introspectable.h in _gen so it's git-ignored.

Fixed.

> > +TelepathyGLib-1.0.gir: $(INTROSPECTION_SCANNER) libtelepathy-glib.la
> 
> Shouldn't this be called TelepathyGLib-0.12.gir?

I don't know TBH. It has a separate version field inside it. The 1.0 seems to
refer to an ABI version?

> > 2138  * tp_account_get_current_presence:
> > 2139  * @account: a #TpAccount
> > 2140  * @status: (out) (transfer none): return location for the current status
> > 2141  * @status_message: (out) (transfer none): return location for the current status message
> 
> These strings are (transfer full).

Fixed. Whoops.

> >     * TpChannel::group-members-changed:
> 
> Perhaps (skip) this one, and just bind group-members-changed-detailed?
> 
> > JS example that retrieves the statuses of valid accounts
> 
> Please put this in tests/manual/ or something (if it's intended as a manual
> regression test), or examples/client/ (if it's good example code).

They're not really intended as either at the moment. I'm not entirely sure if
we should merge them or not. I suppose I should write proper regression tests
instead.

> Is it good JS style to use semicolons or not? You seem to have omitted the
> semicolon on the import of TelepathyGLib. We should probably pick a Javascript
> coding convention (i.e. steal the one from either GJS or Seed) and document it
> on /Style on the wiki (a simple reference like "for Javascript, use <a ...>the
> GJS coding conventions</a>" would be perfect).

This was a mistake that for some reason the interpreter didn't pick up. Fixed.

There doesn't seem to be a strong convention yet. Maybe gnome-shell has a
document we can borrow though.

> You seem to be passing null as user_data to all the async functions in JS.
> Could you make this unnecessary by annotating the user-data as (closure)?

Yes probably. I copied this basic format from gio-cat.js in the gjs examples.

> >   * tp_account_update_parameters_finish:
> ...
> > + * @reconnect_required: (out) (transfer none): a #GStrv to fill with
> > + *  properties that need a reconnect to take effect
> 
> This GStrv is (transfer full).

Whoops. I really should have read the source code, instead of the docstring. We
should probably make ownership explicit in the docstring. Fixed.

> Should the C type of this parameter be GStrv *reconnect_required? Would that
> produce better bindings by default?

I've forced the type in the annotation for now.

> >  * tp_account_manager_get_most_available_presence:
> >  * @manager: a #TpAccountManager
> >- * @status: a string to fill with the actual status
> >- * @message: a string to fill with the actual status message
> >+ * @status: (out) (transfer none): a string to fill with the actual status
> >+ * @message: (out) (transfer none) :a string to fill with the actual status
> >+ *  message
> 
> The two strings are (transfer full). Also, the second colon in @message should
> be swapped with the preceding space.

Fixed.

> >   * tp_account_get_requested_presence:
> >   * @account: a #TpAccount
> > - * @status: return location for the requested status
> > - * @status_message: return location for the requested status message
> > + * @status: (out) (transfer none): return location for the requested status
> > + * @status_message: (out) (transfer none): return location for the requested
> > + *  status message
> 
> The strings are (transfer full).

Fixed.

> >  * tp_asv_size:
> 
> I think this could be (skip)ped? It's only there because g_hash_table_size
> isn't const-correct.

Perhaps. Need to look into dicts more in general.

> > - * tp_asv_take_object_path:
> > + * tp_asv_take_object_path: (skip)
> >   * @asv: a #GHashTable created with tp_asv_new()
> >   * @key: string key
> > - * @value: value
> > + * @value: (transfer full): value
> 
> I don't think it makes sense to use (transfer full) here? It's really a
> hypothetical new transfer type, (transfer steal) perhaps, which can't be bound,
> and the function is (skip)ped anyway.

That's not meant to be there. Fixed.

> > 1822  * tp_asv_get_strv:
> ...
> > 1836  * Returns: (transfer none): the %NULL-terminated string-array value of @key,
> > 1837  *  or %NULL
> 
> I think this should be annotated as (type Strv), or (array zero-terminated=1)
> (element-type utf8), or however you spell that?

Fixed. (type GLib.Strv)

> >   * tp_asv_set_strv:
> ...
> > + * @value: (in) (transfer none): a %NULL-terminated string array
> 
> Would it be better if we used "GStrv value" as the C declaration?

Also fixed.

> > +          try {
> > +            let params = account.get_parameters();
> > +            print("params = " + params);
> > +            Tp.asv_dump(params);
> 
> I'd be happier about the quality of our introspection if this iterated over the
> array printing the GValues, however you do that in Javascript (some sort of
> equivalent of g_strdup_value_contents).

I haven't yet found a way to iterate through a hash table yet. Certainly this
works:

  print("$$ account = " + params["account"]);
  if (params["account"] == "danielle.madeley at collabora.co.uk")
    print("wooooh");

Which implies that our tp_asv_get_* functions are actually unrequired (need to
build a test for all data types I suppose).

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.