Hi,

I fixed this bug and pushed changes my public repository on github[1](it is at devel branch).
My commit is here[2].


[1] - git@github.com:sdemircan/empathy.git
[2] - https://github.com/sdemircan/empathy/commit/58976fa8379f93e731eb152802f20d72ad853775

Thanks for your contribution. A few review comments:

- You can also remove the code that updates phone_icon from update_presence_msg().

- You can call update_phone_icon() from empathy_roster_contact_constructed() like all other update_*() functions.

- Change notification is a bit tricky, because empathy_individual_get_client_types() returns the clients types of the most available tp_contact. So you need to monitor which of the individual's tp_contact is the most available, then monitor the client types of that tp_contact.

- You should not declare variables in the middle of a code block. We do old-school C ;-)

(In reply to comment #2)
> - Change notification is a bit tricky, because
> empathy_individual_get_client_types() returns the clients types of the most
> available tp_contact. So you need to monitor which of the individual's
> tp_contact is the most available, then monitor the client types of that
> tp_contact.

Quickly discussed this with Guillaume, and we think the best is to add a client-types property in FolksPresenceDetails[1]. Then FolksIndividual will be responsible to expose the client-types of its most available persona, and emit change notification.

[1] http://telepathy.freedesktop.org/doc/folks/c/FolksPresenceDetails.html#FolksPresenceDetails.properties

Xavier, that approach seems reasonable. There's a model from Telepathy we can follow here, right? Ie, is there some well-known set of strings for PC, mobile phone, tablet (is that separate?), etc.?

Created attachment 238646 [details] [review]
Patch

i have added client_types property to both Folks.PresenceDetails and impelemented in Folks.Individual and Tpf.Persona.

Travis: telepathy spec defines the possible client types: http://telepathy.freedesktop.org/spec/Connection_Interface_Client_Types.html

Serhat: the patch looks good to me at first glance, but I'll let travis do the proper review since he knows Folks better than myself. Thanks for your work!

Review of attachment 238646 [details] [review]:

Pretty complete. Just a few niggles (mostly documentation issues). One omission is that this.client_types needs to be set to an empty array in Tpf.Persona.from_cache() (around line 1230 of tpf-persona.vala). Thanks.

::: backends/telepathy/lib/tpf-persona.vala
@@ +255,3 @@
+   * The Persona's client types.
+   *
+   * See {@link Folks.PresenceDetails.presence_message}.

Do you mean “See {@link Folks.PresenceDetails.client_types}.”?

This needs a “@since UNRELEASED” line.

@@ +1275,3 @@
+      var contact = (Contact?) this._contact.get ();
+      assert (contact != null); /* should never be called while cached */
+      this.client_types = contact.get_client_types();

Need a space before the ‘()’.

::: folks/individual.vala
@@ +241,3 @@
+  
+  /**
+   * {@inheritDoc}

This needs a “@since UNRELEASED” line.

@@ +1603,3 @@
               this.presence_message != presence_message ||
+              this.presence_status != presence_status ||
+              this.client_types!= client_types)

Missing space before the ‘!=’.

::: folks/presence-details.vala
@@ +103,2 @@
   /**
+   * The contact's client types.

This needs more description of what a ‘client type’ is, and what values one could expect to see in this property.

It also needs a “@since UNRELEASED” line.

Review of attachment 238646 [details] [review]:

This should add to tests/telepathy/individual-properties.vala to check for client_type as it does for presence_status. You'll need to add to libtp-test-contactlist (in tests/lib/telepathy/contactlist/) as necessary. See the way it deals with presence.

Please also update NEWS to mention the new API (see farther down in the NEWS file for the format we report them in).

And, thanks for this work. It will be handy!

Review of attachment 238646 [details] [review]:

This should add to tests/telepathy/individual-properties.vala to check for client_type as it does for presence_status. You'll need to add to libtp-test-contactlist (in tests/lib/telepathy/contactlist/) as necessary. See the way it deals with presence.

Please also update NEWS to mention the new API (see farther down in the NEWS file for the format we report them in).

And, thanks for this work. It will be handy!

Serhat, could you please make the changes above and re-submit for review?

Thanks in advance.

Whoops, I didn't mean to re-purpose this for the Folks portion. I've cloned this as bug #699865 to track that dependency.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/empathy/issues/600.