[Bug 29298] review Conn.I.ClientTypes

[bugzilla-daemon at freedesktop.org]

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

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |r-, minor changes

--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk> 2010-08-29 15:04:23 PDT ---
+static gboolean
+dummy_caps_set_predicate (const GabbleCapabilitySet *set,
+    gconstpointer user_data)
+{
+  return TRUE;
+}

Why not just make gabble_presence_pick_resource_by_caps() accept a NULL
predicate?

Why can't the method and contact attribute implementations share code?

+  /* Find all identity nodes in the return. */

Do you mean ‘result’?

+      const gchar *type;
+
+      /* Now get the client type */
+      if ((type = wocky_node_get_attribute (identity, "type")) == NULL)
+        continue;

This is perverse. Assign directly to type when it's defined?

+              WockyNode *query = wocky_node_tree_get_top_node
(cached_query_rep
+              GPtrArray *types;
+
+              if ((types = client_types_from_message (handle, query)) != NULL)

Ditto.

+gboolean
+gabble_presence_cache_disco_in_progress (GabblePresenceCache *cache,
+    TpHandle handle,
+    const gchar *resource)

I think maybe some of the Jingle code could do with being made to use this
function?


       /* trump existing status & message if it's more present
        * or has the same presence and a higher priority */
       if (r->status > presence->status ||
+          (r->status == presence->status && r->last_activity > activity) ||
           (r->status == presence->status && r->priority > prio))

You should update the comment.


+      if (!tp_strdiff (type, "bot"))
+        res->client_type |= GABBLE_CLIENT_TYPE_BOT;
+
+      if (!tp_strdiff (type, "console"))
+        res->client_type |= GABBLE_CLIENT_TYPE_CONSOLE;
+
+      if (!tp_strdiff (type, "game"))
+        res->client_type |= GABBLE_CLIENT_TYPE_GAME;
+
+      if (!tp_strdiff (type, "handheld"))
+        res->client_type |= GABBLE_CLIENT_TYPE_HANDHELD;
+
+      if (!tp_strdiff (type, "pc"))
+        res->client_type |= GABBLE_CLIENT_TYPE_PC;
+
+      if (!tp_strdiff (type, "phone"))
+        res->client_type |= GABBLE_CLIENT_TYPE_PHONE;
+
+      if (!tp_strdiff (type, "web"))
+        res->client_type |= GABBLE_CLIENT_TYPE_WEB;
+
+      if (!tp_strdiff (type, "sms"))
+        res->client_type |= GABBLE_CLIENT_TYPE_SMS;

If you generate a GEnum for the GabbleClientType enum, you can use
wocky_enum_from_nick() to remove all this code. Oh hmm, I guess it'd be
GFlags... but something similar presumably applies. The next function which
does the reverse is harder because the flags aren't mutually exclusive.
Something similar should be possible... get the GFlagsClass and iterate its
->values array.

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