http://git.collabora.co.uk/?p=user/sjoerd/wocky.git;a=shortlog;h=refs/heads/wocky-node-tree-updated-api
this is a branch which i will review
I wonder whether WockyNodeTree:top-node stealing the thing you give it (rather than copying it) will confuse bindings? + DEBUG ("Stanza without a namespace !?, using dummy namespace"); When will this happen? + * <example><programlisting> + * wocky_node_tree_new ("html", "http://www.w3.org/1999/xhtml", + * "alice@<!-- -->collabora.co.uk", "bob@<!-- -->collabora.co.uk", + * '(', "html", ':', "http://www.w3.org/1999/xhtml", + * '(', "body", '@', "textcolor", "red", + * '$', "Wocky wooo", + * ')' + * NULL); + * </programlisting></example> Following in the glorious tradition of getting these examples wrong, you have not closed the html tag. + * This function may only be called in non-streaming mode. ... + g_return_if_fail (!writer->priv->stream_mode); I think the non-streaming mode should be a subclass. Having methods that may or may not blow up is kind of weird. + destination = wocky_node_tree_new ("mainlain europe", mainland. can you really have spaces in tag names?
(In reply to comment #2) > I wonder whether WockyNodeTree:top-node stealing the thing you give it (rather > than copying it) will confuse bindings? > > + DEBUG ("Stanza without a namespace !?, using dummy namespace"); > > When will this happen? In non-streaming mode when the top node doesn't have a namespace > + * <example><programlisting> > + * wocky_node_tree_new ("html", "http://www.w3.org/1999/xhtml", > + * "alice@<!-- -->collabora.co.uk", "bob@<!-- -->collabora.co.uk", > + * '(', "html", ':', "http://www.w3.org/1999/xhtml", > + * '(', "body", '@', "textcolor", "red", > + * '$', "Wocky wooo", > + * ')' > + * NULL); > + * </programlisting></example> > > Following in the glorious tradition of getting these examples wrong, you have > not closed the html tag. fixed! > + * This function may only be called in non-streaming mode. > ... > + g_return_if_fail (!writer->priv->stream_mode); > > I think the non-streaming mode should be a subclass. Having methods that may or > may not blow up is kind of weird. Yes, but it would mean loads of boilerplate for not much win.. In all but very special cases non-streaming mode and serializing node-trees is kind of weird, so while i agree with you i'd prefer keeping it this way. > + destination = wocky_node_tree_new ("mainlain europe", > > mainland. can you really have spaces in tag names? You can't.. fixed now. In the glorious future wocky should check this and yell at you if needed.
(In reply to comment #3) > (In reply to comment #2) > > I wonder whether WockyNodeTree:top-node stealing the thing you give it (rather > > than copying it) will confuse bindings? > > > > + DEBUG ("Stanza without a namespace !?, using dummy namespace"); > > > > When will this happen? > > In non-streaming mode when the top node doesn't have a namespace Could you document that in the code? The "!?" makes it look more worrying than it actually is, probably. > > + * This function may only be called in non-streaming mode. > > ... > > + g_return_if_fail (!writer->priv->stream_mode); > > > > I think the non-streaming mode should be a subclass. Having methods that may or > > may not blow up is kind of weird. > > Yes, but it would mean loads of boilerplate for not much win.. In all but very > special cases non-streaming mode and serializing node-trees is kind of weird, > so while i agree with you i'd prefer keeping it this way. I think making it a subclass would make the base writer's API less weird. But okay, if you insist. Otherwise ++
merged
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.