GNOME Bugzilla – Bug 431722
icalvalue_new_from_string_with_error() cannot parse ical GEO property
Last modified: 2007-05-18 19:40:51 UTC
the ical property GEO used to carry (latitude, longitude) cannot be parsed by the function icalvalue_new_from_string_with_error(), from libical. This further leads to libecal not capable of saving icalcomponents that have properties of type GEO. Typically, libecal serializes the icalcomponent (and the properties of type GEO) fine, send it to the backend, and the backend has to deserialize it into an icalcomponent. This is where the function icalvalue_new_from_string_with_error() fails. icalvalue_new_from_string_with_error() just does not implement the GEO property parsing yet. You will find a attached a patch that proposes an implementation for that parsing.
Created attachment 86702 [details] [review] a patch that implements the parsing of GEO properties To apply this patch, please use the -p2 option of the 'patch' program. The patch also add a GEO property to the test file libical/test-data/calendar.ics, so that it can be easily tested. Just run the program libical/libical/src/test/parser libical/test-data/calendar.ics Notice that there is a GEO property in the output now. Without an error.
The patch is well done. Please commit the patch to svn HEAD. It would be better to have some test data with locale specific values.
Okay I have pushed it to trunk, in changeset number #r597. Thank you for taking time to review this bug. As far as locale specific tests are concerned, basically, the GEO property is made of two decimal numbers that have the character ',' as decimal separator. There is nothing locale specific in the GEO property itself. The locale of the user running the test can nevertheless affect the parsing, because in some locales, the character ',' is not the decimal separator. So I agree that we need to take that in account during the testing. To do so, we need to have a proper testing framework that can set the locale used by the parser to some arbitrary values.
so can we close this one now, as the patch has been committed?
Yes, closing.