Bug 33901

http://git.collabora.co.uk/?p=user/cassidy/telepathy-glib;a=shortlog;h=refs/heads/support-room-list-33901

You should also return a list of known servers, by scanning all the channel classes for requests with Server in Fixed. I don't know if Gabble implements it, but it sure should.

Assuming Gabble does this, then we can say something like “if with_server is set to %TRUE, but known_servers is empty, requests which do not specify Server can be expected to fail”. This is only applicable when calling this function on the caps from a running connection in Gabble's case I guess.

+      for (j = 0; allowed_properties[j] != NULL; j++)
+        {
+          if (!tp_strdiff (allowed_properties[j],
+                TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER))
+            server = TRUE;
+        }

This is secret code for:

       server = tp_strv_contains (allowed_properties, TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER);

+ * Return whether this protocol or connection can perform rooms
+ * listing.

“Returns whether or not this protocol or connection supports listing rooms.”


+ * Return whether this protocol or connection can perform rooms
+ * listing.
+ *
+ * Returns: %TRUE if a channel request containing RoomList as ChannelType,
+ * HandleTypeNone as TargetHandleType can be expected to work,
+ * %FALSE otherwise.

I actually think these should be swapped around. The body should say something like:

  Discovers whether this protocol or connection supports listing rooms. Specifically, if this function returns %TRUE, a room list channel can be requested as follows:

  |[ ... code for requesting a channel ... ]|

  If with_server is set to %TRUE, a list of rooms on a particular server can be requested as follows:

  |[ ... ]|

Or, thinking aloud, maybe we should have:

enum {
  ROOM_LISTING_SUPPORTED = 1,
  ROOM_LISTING_SERVER_SUPPORTED = 2,
  ROOM_LISTING_SERVER_REQUIRED = 4
} RoomListingSupportFlags;

as the return value, rather than a proliferation of non-self-documenting booleans? I'm not sure this helps much.

(In reply to comment #2)
> You should also return a list of known servers, by scanning all the channel
> classes for requests with Server in Fixed. I don't know if Gabble implements
> it, but it sure should.

I tried implementing this in Gabble (bug #34071) but I'm not sure that's
actually possible. :\

> +      for (j = 0; allowed_properties[j] != NULL; j++)
> +        {
> +          if (!tp_strdiff (allowed_properties[j],
> +                TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER))
> +            server = TRUE;
> +        }
> 
> This is secret code for:
> 
>        server = tp_strv_contains (allowed_properties,
> TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER);

Oh nice, changed.

> + * Return whether this protocol or connection can perform rooms
> + * listing.
> 
> “Returns whether or not this protocol or connection supports listing rooms.”

fixed.

> 
> + * Return whether this protocol or connection can perform rooms
> + * listing.
> + *
> + * Returns: %TRUE if a channel request containing RoomList as ChannelType,
> + * HandleTypeNone as TargetHandleType can be expected to work,
> + * %FALSE otherwise.
> 
> I actually think these should be swapped around. The body should say something
> like:
> 
>   Discovers whether this protocol or connection supports listing rooms.
> Specifically, if this function returns %TRUE, a room list channel can be
> requested as follows:
> 
>   |[ ... code for requesting a channel ... ]|
> 
>   If with_server is set to %TRUE, a list of rooms on a particular server can be
> requested as follows:
> 
>   |[ ... ]|

done.

(In reply to comment #3)
> Or, thinking aloud, maybe we should have:
> 
> enum {
>   ROOM_LISTING_SUPPORTED = 1,
>   ROOM_LISTING_SERVER_SUPPORTED = 2,
>   ROOM_LISTING_SERVER_REQUIRED = 4
> } RoomListingSupportFlags;
> 
> as the return value, rather than a proliferation of non-self-documenting
> booleans? I'm not sure this helps much.

This could be nice, but shouldn't ROOM_LISTING_SUPPORTED be "included" in
ROOM_LISTING_SERVER_SUPPORTED and ROOM_LISTING_SERVER_REQUIRED?

(In reply to comment #4)
> (In reply to comment #2)
> > You should also return a list of known servers, by scanning all the channel
> > classes for requests with Server in Fixed. I don't know if Gabble implements
> > it, but it sure should.
> 
> I tried implementing this in Gabble (bug #34071) but I'm not sure that's
> actually possible. :\

According the discussion on bug #34071 I suspect this should be done as a separated API.

+ *     TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER: G_TYPE_STRING,

Example no compile.

+ * /\* Same code as above but with request defined using: *\/

Does that work?

+ * tp_account_channel_request_create_channel_async (req, NULL, NULL,

Shouldn't the example use create_and_handle_channel probably? This is a nit-pick though.

+ * If with_server is set to %TRUE, a list of rooms on a particular server can be

@with_server.

(In reply to comment #6)
> + *     TP_PROP_CHANNEL_TYPE_ROOM_LIST_SERVER: G_TYPE_STRING,
> 
> Example no compile.

Oops fixed (amend)

> + * /\* Same code as above but with request defined using: *\/
> 
> Does that work?

Well, the conference Server doesn't exist (I took it from the XEP); I can put
a real one if you prefer.

> + * tp_account_channel_request_create_channel_async (req, NULL, NULL,
> 
> Shouldn't the example use create_and_handle_channel probably? This is a
> nit-pick though.

You're right; fixed.

> + * If with_server is set to %TRUE, a list of rooms on a particular server can
> be
> 
> @with_server.

fixed.

gogogo.

Merged to master; will be in 0.13.14