[Bug 24146] Move data forms code to Wocky

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=24146





--- Comment #4 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk>  2009-09-29 04:42:45 PST ---
(In reply to comment #3)
> Partial review up to 8fc141547d96a1ca69:
> 
> wocky_data_forms_field and wocky_data_forms_field_option should be
> camel-cased.

done

> extract_value_list() doesn't NULL-terminate the returned value. This
> suggests that that code path isn't tested. :)

Fixed in the Wocky branch (and tested!) :)


> +      DEBUG ("Wrong field type for '%s': %s", type_str, var);
> 
> It's not a wrong type, it's an invalid type.

fixed.

> +          DEBUG ("No option provided for '%s'", var);
> 
> Options.

fixed.

> Why does wocky_data_forms_field_new() take ownership of the 'options'
> list, but not of (eg) the default value?

No good reason; fixed.


> +      if (value== NULL)
> 
> Style: add a space.

already fixed.

> extract_search_keys_from_froms(): The last word should be "forms" :)

fixed.

> +          "server doesn't support any known search key");
> 
> keys plural.

fixed.

> +  /* list will be reveresed */
> 
> reversed.

fixed.

> +        if (!wocky_strdiff (value, "true"))
> +          return wocky_g_value_slice_new_boolean (TRUE);
> +        else if (!wocky_strdiff (value, "false"))
> +          return wocky_g_value_slice_new_boolean (FALSE);
> +        else
> +          DEBUG ("Invalid boolean value: %s", value);
> +        break;
> 
> From the footnote assigned to the boolean type in XEP-0004 §3.3:
> 
> > 10. In accordance with Section 3.2.2.1 of XML Schema Part 2:
> > Datatypes, the allowable lexical representations for the xs:boolean
> > datatype are the strings "0" and "false" for the concept 'false' and
> > the strings "1" and "true" for the concept 'true'; implementations
> > MUST support both styles of lexical representation.
> 
> Amusingly, this branch produces "0" and "1", but only accepts "false"
> and "true". :)

Oooops, nice catch. Fixed. :)

> The name of wocky_data_forms_submit() seems a bit misleading. It
> doesn't actually submit the form. Obviously it doing so would be
> strange... Maybe it's worth the misleading name for brevity's sake?
> 

Don't know. IMHO the current name isn't that bad but I'm open to suggestions.


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