Bug 34824

Also, am interested in feedback from the reviewer regarding reviewing in cgit compared to in gitweb?

For those like me that just don't know, what's the context of this change ? What flexibility do we gain ?

(In reply to comment #2)
> For those like me that just don't know, what's the context of this change ?
> What flexibility do we gain ?

I've written a "meta porter" which is a porter for Salut in that it maintains many client-to-client porters and hides all the details of setting up connections on-the-go.

By making the wocky porter an interface, we can make the existing C2S porter and my new meta porter have the same API, which is nice and less hacky when adding to the WockySession.

+ * wocky-c2s-porter.c - Source for WockyC2SPorter
+ * Copyright (C) 2009 Collabora Ltd.

This seems unlikely.

- * Copyright (C) 2009 Collabora Ltd.
- * @author Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
+ * Copyright (C) 2011 Collabora Ltd.

This is also unlikely. It should presumably be 2009–2011.

+  guint (*register_handler_from_stanza) (

I don't really like the naming here. Of course, it's my fault because I called the original function register_handler_from. Maybe we should call the stanza variants _by_stanza or similar? I'm not overly hung up on this.

Why is register_handler_from_server() on the generic interface? The link-local porter has no such concept, so I wonder if that should be a method on WockyC2SPorter.

+const gchar * wocky_porter_get_full_jid (WockyPorter *self);
+const gchar * wocky_porter_get_bare_jid (WockyPorter *self);
+const gchar * wocky_porter_get_resource (WockyPorter *self);

I suppose these are less contentious, although link-local JIDs don't generally have resources.

 static void
 wocky_porter_default_init (WockyPorterInterface *iface)
 {
+  GType iface_type = G_TYPE_FROM_INTERFACE (iface);
+  static gboolean initialized = FALSE;
+  GParamSpec *spec;
+
+  if (initialized)
+    return;

Should probably use g_once_init_enter()?

(In reply to comment #4)
> This seems unlikely.
[...]
> This is also unlikely. It should presumably be 2009–2011.

Fixed.

> +  guint (*register_handler_from_stanza) (
> 
> I don't really like the naming here. Of course, it's my fault because I called
> the original function register_handler_from. Maybe we should call the stanza
> variants _by_stanza or similar? I'm not overly hung up on this.

_by_stanza sounds better. Shall we change _va to _by_va at the same time? I guess blah_va is more common. Done.

> Why is register_handler_from_server() on the generic interface? The link-local
> porter has no such concept, so I wonder if that should be a method on
> WockyC2SPorter.
> 
> +const gchar * wocky_porter_get_full_jid (WockyPorter *self);
> +const gchar * wocky_porter_get_bare_jid (WockyPorter *self);
> +const gchar * wocky_porter_get_resource (WockyPorter *self);
> 
> I suppose these are less contentious, although link-local JIDs don't generally
> have resources.

I wondered what to do about the from_server and the get_full_jid, bare_jid and resource functions but decided on just leaving them in the interface because full jids, bare jids, resources and servers all exist in the huge majority of XMPP -- non-link-local. If  we were to throw them into the c2s-porter directly then implement some other porter which has something to do with normal XMPP then we'll have more identical porter-specific functions. Admittedly, at that point they could be added to the interface.

Hmm, I'm just not sure anymore. What do you think?

> Should probably use g_once_init_enter()?

OK, done.

(branch updated and pushed)

  * To match stanzas from any sender, see
- * wocky_porter_register_handler_from_anyone(). To match stanzas sent by the
- * server, see wocky_porter_register_handler_from_server().
+ * wocky_porter_register_handler_from_anyone().

I think it'd be good to explicitly say “If you're using a WockyC2S porter, you can match stanzas sent by the server using wocky_c2s_porter_…”. Otherwise it will be easy to miss that it exists.

> Shall we change _va to _by_va at the same time? I
> guess blah_va is more common.

The _va suffix doesn't bother me as much. It's standard, and doesn't look as weird in English.

> I wondered what to do about the from_server and the get_full_jid, bare_jid and
> resource functions but decided on just leaving them in the interface because
> full jids, bare jids, resources and servers all exist in the huge majority of
> XMPP -- non-link-local. If  we were to throw them into the c2s-porter directly
> then implement some other porter which has something to do with normal XMPP
> then we'll have more identical porter-specific functions. Admittedly, at that
> point they could be added to the interface.

I think it's fine to leave them on the interface. In theory you could have a resource on link-local, though it's unlikely… code dealing with link-local will just have to cope with get_resource possibly returning NULL. As long as it returns NULL rather than crashing …

(In reply to comment #7)
>   * To match stanzas from any sender, see
> - * wocky_porter_register_handler_from_anyone(). To match stanzas sent by the
> - * server, see wocky_porter_register_handler_from_server().
> + * wocky_porter_register_handler_from_anyone().
> 
> I think it'd be good to explicitly say “If you're using a WockyC2S porter, you
> can match stanzas sent by the server using wocky_c2s_porter_…”. Otherwise it
> will be easy to miss that it exists.

Done.

Merged, thanks.