How's this?
- Julian
Improve svn_opt_parse_path's documentation, implementation, and testing.
* subversion/include/svn_opt.h
(svn_opt_parse_path): Improve description.
(svn_opt_parse_revision): State the possible error return.
* subversion/libsvn_subr/opt.c
(svn_opt_parse_path): Ensure that only one revision number is present,
not a colon-separated pair. Remove unnecessary use of a sub-pool.
* subversion/tests/libsvn_subr/opt-test.c
New file, with a test for svn_opt_parse_path.
* build.conf
(opt-test): New section for the new test file.
Index: subversion/include/svn_opt.h
===================================================================
--- subversion/include/svn_opt.h (revision 11819)
+++ subversion/include/svn_opt.h (working copy)
@@ -233,6 +233,9 @@
* It is typical, though not required, for @a *start_revision and
* @a *end_revision to be @c svn_opt_revision_unspecified kind on entry.
*
+ * If a revision specifier is invalid, return an @c
+ * SVN_ERR_CL_ARG_PARSING_ERROR.
+ *
* Use @a pool for temporary allocations.
*/
int svn_opt_parse_revision (svn_opt_revision_t *start_revision,
@@ -322,17 +325,35 @@
/**
* @since New in 1.1.
*
- * Parse a working-copy or url in @a path, looking for an "@" sign.
+ * Parse a working-copy or URL in @a path, extracting any trailing
+ * revision specifier of the form "@rev" from the last component of
+ * the path.
*
* Some examples would be:
*
- * - foo/bar/baz@@13
- * - http://blah/bloo@27
- * - blarg/snarf@@HEAD
- *
- * If an "@" is found, return the two halves in @a *truepath and @a
- * *rev, allocating from @a pool. If no "@" is found, set @a *truepath
- * to @a path and @a *rev to kind 'unspecified'.
+ * - foo/bar -> "foo/bar", (unspecified)
+ * - foo/bar@13 -> "foo/bar", (number, 13)
+ * - foo/bar@HEAD -> "foo/bar", (head)
+ * - foo/bar@{1999-12-31} -> "foo/bar", (date, 1999-12-31)
+ * - http://a/b@27 -> "http://a/b", (number, 27)
+ * - http://a/b@COMMITTED -> "http://a/b", (committed) [*]
+ * - foo/bar@1:2 -> error
+ * - foo/bar@baz -> error
+ * - foo/bar@ -> error
+ * - foo/bar@13/ -> "foo/bar@13/", (unspecified)
+ * - foo/bar@@13 -> "foo/bar@", (number, 13)
+ * - foo/@bar@HEAD -> "foo/@bar", (head)
+ * - foo@/bar -> "foo@/bar", (unspecified)
+ * - foo@HEAD/bar -> "foo@HEAD/bar", (unspecified)
+ *
+ * [*] Syntactically valid but probably not semantically useful.
+ *
+ * If a trailing revision specifier is found, parse it into @a *rev and
+ * put the rest of the path into @a *truepath, allocating from @a pool;
+ * or return an @c SVN_ERR_CL_ARG_PARSING_ERROR if the revision
+ * specifier is invalid. If no trailing revision specifier is found,
+ * set @a *truepath to @a path and @a rev->kind to @c
+ * svn_opt_revision_unspecified.
*/
svn_error_t *
svn_opt_parse_path (svn_opt_revision_t *rev,
Index: subversion/libsvn_subr/opt.c
===================================================================
--- subversion/libsvn_subr/opt.c (revision 11819)
+++ subversion/libsvn_subr/opt.c (working copy)
@@ -487,27 +487,29 @@
apr_pool_t *pool)
{
int i;
- apr_pool_t *subpool = svn_pool_create (pool);
- svn_opt_revision_t start_revision, end_revision;
/* scanning from right to left, just to be friendly to any
screwed-up filenames that might *actually* contain @-signs. :-) */
for (i = (strlen (path) - 1); i >= 0; i--)
{
/* If we hit a path separator, stop looking. */
+ /* This is OK only because our revision specifiers can't contain '/'. */
if (path[i] == '/')
break;
if (path[i] == '@')
{
const char *native_rev;
+ svn_opt_revision_t start_revision, end_revision;
SVN_ERR (svn_utf_cstring_from_utf8 (&native_rev, path + i + 1,
- subpool));
+ pool));
+ end_revision.kind = svn_opt_revision_unspecified;
if (svn_opt_parse_revision (&start_revision,
&end_revision,
- native_rev, subpool))
+ native_rev, pool)
+ || end_revision.kind != svn_opt_revision_unspecified)
return svn_error_createf (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("Syntax error parsing revision '%s'"),
path + i + 1);
@@ -516,7 +518,6 @@
rev->kind = start_revision.kind;
rev->value = start_revision.value;
- svn_pool_destroy (subpool);
return SVN_NO_ERROR;
}
}
@@ -525,7 +526,6 @@
*truepath = path;
rev->kind = svn_opt_revision_unspecified;
- svn_pool_destroy (subpool);
return SVN_NO_ERROR;
}
Index: subversion/tests/libsvn_subr/opt-test.c
===================================================================
--- subversion/tests/libsvn_subr/opt-test.c (revision 0)
+++ subversion/tests/libsvn_subr/opt-test.c (revision 0)
@@ -0,0 +1,99 @@
+/*
+ * opt-test.c -- test the option functions
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+#include <string.h>
+#include <svn_opt.h>
+#include <apr_general.h>
+#include "svn_test.h"
+
+
+static svn_error_t *
+test_parse_peg_rev (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ int i;
+ static struct {
+ const char *input;
+ const char *path; /* NULL means an error is expected. */
+ svn_opt_revision_t peg;
+ } const tests[] = {
+ { "foo/bar", "foo/bar", {svn_opt_revision_unspecified} },
+ { "foo/bar@13", "foo/bar", {svn_opt_revision_number, {13}} },
+ { "foo/bar@HEAD", "foo/bar", {svn_opt_revision_head} },
+ { "foo/bar@{1999-12-31}", "foo/bar", {svn_opt_revision_date, {0}} },
+ { "http://a/b@27", "http://a/b", {svn_opt_revision_number, {27}} },
+ { "http://a/b@COMMITTED", "http://a/b", {svn_opt_revision_committed} },
+ { "foo/bar@1:2", NULL, {svn_opt_revision_unspecified} },
+ { "foo/bar@baz", NULL, {svn_opt_revision_unspecified} },
+ { "foo/bar@", NULL, {svn_opt_revision_unspecified} },
+ { "foo/bar@13/", "foo/bar@13/", {svn_opt_revision_unspecified} },
+ { "foo/bar@@13", "foo/bar@", {svn_opt_revision_number, {13}} },
+ { "foo/@bar@HEAD", "foo/@bar", {svn_opt_revision_head} },
+ { "foo@/bar", "foo@/bar", {svn_opt_revision_unspecified} },
+ { "foo@HEAD/bar", "foo@HEAD/bar", {svn_opt_revision_unspecified} },
+ };
+
+ *msg = "test svn_opt_parse_path";
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++)
+ {
+ const char *path;
+ svn_opt_revision_t peg;
+ svn_error_t *err;
+
+ err = svn_opt_parse_path (&peg, &path, tests[i].input, pool);
+ if (err)
+ {
+ svn_error_clear (err);
+ if (tests[i].path)
+ {
+ return svn_error_createf
+ (SVN_ERR_TEST_FAILED, NULL,
+ "svn_opt_parse_path ('%s') returned an error instead of '%s'",
+ tests[i].input, tests[i].path);
+ }
+ }
+ else
+ {
+ if ((path == NULL)
+ || (tests[i].path == NULL)
+ || (strcmp (path, tests[i].path) != 0)
+ || (peg.kind != tests[i].peg.kind)
+ || (peg.kind == svn_opt_revision_number && peg.value.number != tests[i].peg.value.number))
+ return svn_error_createf
+ (SVN_ERR_TEST_FAILED, NULL,
+ "svn_opt_parse_path ('%s') returned '%s' instead of '%s'", tests[i].input,
+ path ? path : "NULL", tests[i].path ? tests[i].path : "NULL");
+ }
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
+/* The test table. */
+
+struct svn_test_descriptor_t test_funcs[] =
+ {
+ SVN_TEST_NULL,
+ SVN_TEST_PASS (test_parse_peg_rev),
+ SVN_TEST_NULL
+ };
Index: build.conf
===================================================================
--- build.conf (revision 11819)
+++ build.conf (working copy)
@@ -592,6 +592,14 @@
install = test
libs = libsvn_test apr
+# test options library
+[opt-test]
+type = exe
+path = subversion/tests/libsvn_subr
+sources = opt-test.c
+install = test
+libs = libsvn_test libsvn_subr apr
+
# test path library
[path-test]
type = exe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 10 20:53:37 2004