[Bug 27740] WockyNodeTree and friends

Mon Apr 19 21:06:17 CEST 2010

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

--- Comment #3 from Sjoerd Simons <sjoerd at luon.net> 2010-04-19 12:06:17 PDT ---
(In reply to comment #1)
> +
> +/* Slightly evil macro that tests that two stanzas are equal without regarding
> + * their id, except that if one has an id and the other does not this is not
> + * considered a difference. It modifies the stanzas because I am lazy.
>   */
> -#define test_assert_stanzas_equal(s1, s2) \
> +#define test_assert_stanzas_equal_no_id(s1, s2) \
> 
> The change here is adding "without regarding their id", and that's not true.
> The bit after the comma explains exactly how IDs are treated. So I don't think
> this improves the comment.

revert that change, the function confused me more then i thought :)

> +wocky_xmpp_node_new (const char *name, const gchar *ns)
>  {
>    WockyXmppNode *result = g_slice_new0 (WockyXmppNode);
> 
>    result->name = g_strdup (name);
> +  result->ns = (ns != NULL) ? g_quark_from_string (ns) : 0;
> 
> g_quark_from_string() is NULL-safe.

fixed

> @@ -0,0 +1,85 @@
> +/*
> + * wocky-node-tree.c - Source for WockyNodeTree
> + * Copyright (C) 2006 Collabora Ltd.
> 
> + * wocky-node-tree.h - Header for WockyNodeTree
> + * Copyright (C) 2006,2010 Collabora Ltd.
> 
> I find that very hard to believe.

dates updated to be ranges

> 6af7e30332 adds:
> 
> +WockyNodeTree *wocky_node_tree_build (WockyNodeBuildTag first_tag,
> +    ...) G_GNUC_NULL_TERMINATED;
> +
> +WockyNodeTree * wocky_node_tree_build_va (va_list ap);
> +
> 
> but they're not implemented anywhere afaict.

yup nuked them

> +void
> +wocky_xmpp_node_add_build (WockyXmppNode *node,
> +    WockyNodeBuildTag first_tag,
> +    ...)
> +{
> +  va_list ap;
> +
> +  va_start (ap, first_tag);
> +  wocky_xmpp_node_add_build_va (node, ap);
> +  va_end (ap);
> +}
> 
> 
> Doesn't this mean you're not passing first_tag to _build_va?

yeah it's broken, fixed and test added

> +/**
> + * wocky_node_add_build:
> + * @node: The node under which to add a new subtree
> + * @first_tag: The build tag for the first node
> + * @Varargs: the description of the stanza to build,
> + *  terminated with %NULL
> + *
> + * Add a node subtree to an existing parent node
> 
> There should be a full stop at the end of this sentence.

. added

> + * Example:
> + * <example><programlisting>
> + * wocky_node_add_build (node,
> + *    '(', "body",
> + *        '$', "Telepathy rocks!",
> + *    ')',
> + *   NULL);
> + * </programlisting></example>
> + *
> + * <!-- -->* adds the following under the given node
> + * <body>
> + *    Telepathy rocks!
> + * </body>
> + * *<!-- -->/
> + * </programlisting></example>
> + *
> + */
> 
> You close the <programlisting> and <example> twice. You can use |[ ... ]| as an
> abbreviation. I think the listing should end at the end of the C code, and the
> XML could be in its own block. <programlisting language="xml"> ?

slightly tweaked, also tweaked the doc generation as this didn't even end up
there

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