GNOME Bugzilla – Bug 685963
Add ContentDirectory FeatureList state variable XML parser
Last modified: 2019-02-22 05:57:46 UTC
Add ContentDirectory FeatureList state variable XML parser: * Add the parser * Update documentation * Add a test case
Created attachment 226257 [details] [review] Add the FeatureList XML parser
Created attachment 226258 [details] [review] Update documentation
Created attachment 226259 [details] [review] Add a test case
Review of attachment 226257 [details] [review]: ::: libgupnp-av/gupnp-feature-list-parser.c @@ +142,3 @@ +gboolean +gupnp_feature_list_parser_parse_text (GUPnPFeatureListParser *parser, + const char *text, indentation is off @@ +181,3 @@ + name = xml_util_get_attribute_content(element, "name"); + version = xml_util_get_attribute_content(element, + "version"); Missing space before "(" ::: libgupnp-av/gupnp-feature-list-parser.h @@ +62,3 @@ + void (* feature_available) (GUPnPFeatureListParser *parser, + GUPnPFeature *feature); +} GUPnPFeatureListParserClass; I know you're just following the other parsers here, but I don't think this API is particularly nice. It's not really async anyway and feature lists aren't that long, so I think it's easier, also for the consumers, to just have some _parse() function that returns the list of features. ::: libgupnp-av/gupnp-feature.h @@ +64,3 @@ + +const char * +gupnp_feature_get_name (GUPnPFeature *object); nitpick: Shouldn't that be "feature" instead of "object"?
Review of attachment 226259 [details] [review]: Can you make that a real test that is run on make check?
Created attachment 226544 [details] [review] Add the FeatureList XML parser
Created attachment 226545 [details] [review] Add a test case
Hi, I updated two of the patches according to your comments: * Fix spaces * The parser returns a list now * The test case is run on make check Regis
Pushed with minor introspection annotation fixes.