GNOME Bugzilla – Bug 650968
[PATCH] fix g_time_val_from_iso8601() support for date without time specified
Last modified: 2018-05-24 13:09:26 UTC
Created attachment 188457 [details] Small test program that reproduces the bug. If the iso_date parameter is a date without time, such as "2011-05-24", g_time_val_from_iso8601() returns TRUE without modifying the time_ parameter, which is inconsistent with the documentation. I believe it should either return FALSE in that case, or modify time_ as if iso_date had been "2011-05-24T00:00:00Z". Attached a small test case allowing to reproduce the issue.
Created attachment 188474 [details] [review] Proposed patch: fill in the time_ structure when only a date is specified
This was introduced by the patch in bug 627892. It is unclear what the desired behavior was; that patch added a test case that makes sure that g_time_val_from_iso8601() returns TRUE in this case, but it didn't test the timeval. My understanding of ISO 8601 is that "2011-05-25" is NOT supposed to be considered the same as "2001-05-25T00:00:00Z". Rather it means any time on that date, and so g_time_val_from_iso8601() should be returning FALSE. Jens?
Oh dear. When I wrote that patch I assumed that the time will be set to midnight. But that is really broken now, sorry. Well, thinking about it now, g_time_val_from_iso8601 should probably return FALSE there again.
Agreed. And I think we should document more clearly what g_time_val_from_iso8601() expects as input (an iso8601-formatted date _and_ time). Will try to cook a patch doing that. I'm also starting to think a g_date_set_from_iso8601() could be a good complement, for people to have something to handle their iso8601 _dates_ (not including time). We could benefit from that in Grilo.
Created attachment 188656 [details] [review] return FALSE in g_time_val_from_iso8601() if only a date is specified This is an alternative proposal, that does not accept simple dates (with no time specified). The rationale is that a GTimeVal represents an instant, whereas a simple date represents a whole period.
Comment on attachment 188656 [details] [review] return FALSE in g_time_val_from_iso8601() if only a date is specified you need to update tests/testglib.c too, to change the expected return value of converting REF_STR_DATE_ONLY. (btw, do you have git access or do you need someone else to commit this?)
Created attachment 188677 [details] [review] return FALSE in g_time_val_from_iso8601() if only a date is specified. Return false for a date only string, and updated tests accordingly. I have a git access, but AFAIK, it only gives me commit rights on grilo.
Regarding comment 4, I filed bug 651168 that introduces g_date_set_iso_8601().
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/413.