mark benedetto king wrote:
>Here's a quick patch for review.
>
>Remove the getdate.y-based date-parser, replacing it with a simpler
>though less powerful one.
>
>
[... ]
>+/** Convert human-readable date @a text into an @c apr_time_t, using
>+ * @a now as the current time and @a gmtoff as an indicator of
>+ * seconds east of GMT, storing the result in @a val. Return
>+ * TRUE iff the date is parsed successfully.
> */
>-time_t svn_parse_date (char *text, struct getdate_time *now);
>+svn_boolean_t svn_parse_date (const char *text, apr_time_t now,
>+ apr_int32_t gmtoff, apr_time_t *val);
>
>
A few issues here:
* We usually put output parameters first in the argument list.
* Wouldn't it be better for this function to return an svn_error_t*?
Even if you don't need it now, you almost certainly will once the
date parser is extended to support more formats (and L10N).
* The same argument suggests this function would need a pool someday.
>Index: subversion/libsvn_subr/date.c
>===================================================================
>--- subversion/libsvn_subr/date.c (revision 0)
>+++ subversion/libsvn_subr/date.c (revision 0)
>@@ -0,0 +1,194 @@
>+#include <apr_lib.h>
>+#include <svn_time.h>
>+
>+/* Possible return values from char_match() */
>+typedef enum {
>+ match_none, /* no match */
>+ match_simple, /* direct match from template char to value char */
>+ match_skip, /* potential skip */
>+ match_complete, /* match at end-of-template/end-of-value */
>+} match_type_t;
>+
>+
>+/* Determine how a particular character VCHAR matches the current
>+ template character TCHAR.
>+
>+ Legal template characters are:
>
>
Cound we say "valid" here instead of "legal"? People might get the
impression that typing an incorrect date might get them sent to jail. :-)
[...]
>+/* Attempt to match the date-string in VALUE to provided TEMPLATE.
>+ As each character is matched, the corresponding OPCODE is applied
>+ using the corresponding OPERAND. The matched values are store in
>
>
^^^^^^ stored
>+ RESULTS, the first COUNT integers of which are zeroed at entry.
>+ Return TRUE iff the VALUE matches the TEMPLATE.
>+
>+ Template/Value matching is described above. Legal opcodes
>+ are:
>
>
legal->valid again
[...]
>+ if (template_match (value, /* try ISO-8601 extended */
>+ "YYYY-MM-DD?Thh?:mm?:ss?.s?s?s?s?s?s?Z?+HH?:MM",
>+ "AAAA AA AA? AA? AA? AA? M?M?M?M?M?M?S SAA AA",
>+ "0000 11 22Z 33Z 44Z 55Z 6Z6Z6Z6Z6Z6Z7 788 99",
>+ results, RESULT_COUNT)
>
>
Won't this match "yyyy-mm-dd hh:mm:ssZ+hh:mm"? If I understand the
template logic correctly, you're allowed to use both the magic "Z" UTC
indicator and an explicit offset at the same time.
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Dec 21 07:31:35 2003