[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[REVIEW]: ISO 8601 Relaxation With Err Msg's

From: Christopher Ness <chris_at_nesser.org>
Date: 2005-07-20 01:13:31 CEST

Hi!

I finally got that patch hammered out, but I'm concerned about a few
things - in no particular order.

* Memory leaks though svn_error_t structures.
* Causes a test to fail, but it's supposed to fail. Maybe someone else
can explain why this is happening when I run `make check` (I had to put
the string in there myself, but removed it from the patch since I don't
check it for UTF8 compliance):

subversion/libsvn_subr/date.c:251: (apr_err=125003)
svn_tests: '2000-01-01+24:00' is not in ISO-8601 format
FAIL: lt-time-test 5: test svn_parse_date

* Translation of a new error message. Is _(message) good enough to let
the translators know to also translate the "message" string?
* When I try to run my source though `indent` lots of other code gets
repositioned casing changes in white space. :(
* These warnings from my compiler (I've never been much of a pointer
wizard), hopefully someone can set me straight on the double pointer.

subversion/libsvn_subr/opt.c:392: warning: passing argument 4 of
'parse_one_rev' from incompatible pointer type
subversion/libsvn_subr/opt.c:398: warning: passing argument 4 of
'parse_one_rev' from incompatible pointer type

Other than that, please review the patch and let me know what you think.

I'd like to resolve the compilation warnings, failing of the date test
in `make check` and learn more about the svn_error_t and memory leaks
before putting this up for commit.

[[[
This change set relaxes the ISO-8601 standard to allow for omitted
numbers in the month, day and hour of a date. It also exposes a more
verbose error message to the user when the revision is a date, but not a
valid input.

Patch By: Christopher Ness <chris@nesser.org>
Review By: PUT-YOUR-NAME-HERE

* subversion/include/svn_opt.h
  (svn_opt_parse_revision): Now returns svn_error_t*

* subversion/libsvn_subr/opt.c
  (parse_one_rev): Passes error status by reference. Uses error
  status of called functions instead of ignoring them.
  (svn_opt_parse_revision): Returns svn_error_t* and uses error
  values from functions.
  (svn_opt_parse_path): Checks if error values to exist, following
  suit with the interface changes to svn_opt_parse_revision().

* subversion/libsvn_subr/date.c
  (svn_parse_date): Comment reason to relax the Months, Days and
  Hours in the formats with separating characters. Changes made to
  the templates to implement the relaxation. Return a new error
  string explaining why the syntax was incorrect.

* subversion/clients/cmdline/main.c
  (main): Added a second error variable to test if the users input
  was successfully converted to UTF8. New logic req'd to expose the
  new date message and translation. More error messages can be handled
  here in the future.

* subversion/tests/libsvn_subr/time-test.c
  (localtz_tests): Add test cases to check for regressions of the new
  date relaxations. Added an invalid day of "00"
]]]

Cheers,
Chris

-- 
PGP Public Key: http://www.nesser.org/pgp-key/
03:28:41 up 10:44, 3 users, load average: 1.28, 1.27, 0.84

Received on Wed Jul 20 01:13:04 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.