After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 685963 - Add ContentDirectory FeatureList state variable XML parser
Add ContentDirectory FeatureList state variable XML parser
Status: RESOLVED FIXED
Product: gupnp-av
Classification: Other
Component: General
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: GUPnP Maintainers
GUPnP Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-10-11 12:53 UTC by regis.merlino
Modified: 2019-02-22 05:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add the FeatureList XML parser (22.77 KB, patch)
2012-10-11 12:55 UTC, regis.merlino
needs-work Details | Review
Update documentation (2.52 KB, patch)
2012-10-11 12:56 UTC, regis.merlino
committed Details | Review
Add a test case (4.42 KB, patch)
2012-10-11 12:56 UTC, regis.merlino
needs-work Details | Review
Add the FeatureList XML parser (21.83 KB, patch)
2012-10-16 13:51 UTC, regis.merlino
committed Details | Review
Add a test case (4.84 KB, patch)
2012-10-16 13:52 UTC, regis.merlino
committed Details | Review

Description regis.merlino 2012-10-11 12:53:50 UTC
Add ContentDirectory FeatureList state variable XML parser:

* Add the parser
* Update documentation
* Add a test case
Comment 1 regis.merlino 2012-10-11 12:55:21 UTC
Created attachment 226257 [details] [review]
Add the FeatureList XML parser
Comment 2 regis.merlino 2012-10-11 12:56:09 UTC
Created attachment 226258 [details] [review]
Update documentation
Comment 3 regis.merlino 2012-10-11 12:56:38 UTC
Created attachment 226259 [details] [review]
Add a test case
Comment 4 Jens Georg 2012-10-16 09:59:04 UTC
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"?
Comment 5 Jens Georg 2012-10-16 10:01:58 UTC
Review of attachment 226259 [details] [review]:

Can you make that a real test that is run on make check?
Comment 6 regis.merlino 2012-10-16 13:51:58 UTC
Created attachment 226544 [details] [review]
Add the FeatureList XML parser
Comment 7 regis.merlino 2012-10-16 13:52:45 UTC
Created attachment 226545 [details] [review]
Add a test case
Comment 8 regis.merlino 2012-10-16 13:56:10 UTC
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
Comment 9 Jens Georg 2012-10-24 18:33:03 UTC
Pushed with minor introspection annotation fixes.