Re: [PATCH] replacement for getdate.y

C. Scott Ananian wrote:

>The attached patch replaces getdate.y and getdate.cw with the new
>svn_date.c code. Instructions:
>

This is great work.

Unfortunately, IMO it not ready for commit yet. Here's what I think
needs doing:

    * Loose <time.h>, <ctype.h>, and use the APR equivalents. 99.9% of
      the time, we'll have an apr_exploded_time_t available, and it
      makes no sense to convert that to and from struct tm. Youl'' also
      be able to toss mktime_UTC, and use apr_explode_gmt() instead.
    * Given the above, you obviously won't be able to use strptime. We
      need a replacement, anyway because strptime isn't available,
      e.g., on Windows. That probably means writing an apr_strptime()
      function, and using that. (I /said/ this was no longer a
      bite-sized task.)
    * I strongly recommend using strtol instead of atoi, because you can
      use the returned end pointer to check the correctness of the
      format. That'll make some of the parsing easier.
    * I wouldn't start using gettext just now. Maybe just use it as a
      marker. I'd like to take time to review the gettext usage, anyway.
      At first glance, it's too simplistic. (For instance, in Slovenian,
      having only a choice of "%d <whatever> ago" and "%d <whatever>s
      ago" is not enough. See `info gettext', the bit about mgettext().
      Welcome to i18n. :-)

Given that replacing strptime is anything mut trivial, you might
consider dropping support for parsing locale-specific (%X) dates for
now. I think the sscanf should be enough for the other forms you
support, or at least for a subset?

-- 
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