Madan U Sreenivasan wrote:
[...]
> static const apr_getopt_option_t cl_options[] =
> @@ -66,6 +67,8 @@
> N_("lists all the tests with their short description")},
> {"verbose", verbose_opt, 0,
> N_("print extra information")},
> + {"help", help_opt, 0,
> + N_("print help information")},
> {0, 0, 0, 0}
> };
>
> @@ -146,7 +149,38 @@
> }
>
>
> +/* Print usage information. */
> +static void
> +usage(char *cmd)
> +{
> + char *usage_str = "usage: %s [options] [arguments]\n"
Should be "const char *". gcc 4.0.2 says:
subversion/tests/svn_test_main.c:173: warning: initialization discards
qualifiers from pointer target type
This string should probably be marked for localisation by wrapping it in "_(...)".
> + "If a valid test case number is provided, execute that test.\n"
> + "Otherwise execute all the tests in %s.\n"
> + "\n"
> + "Available options are:\n"
> + " --help : Print this usage message.\n"
> + " --url arg : Use ARG as the base url for test execution.\n"
> + " --list : List the tests available in %s.\n"
> + " --fs-type arg : Create repositories using fs backend ARG.\n"
> + " ARG could be either 'bdb' (default) or 'fsfs'.\n"
> + " -v [--verbose] : Print verbose output.\n"
> + " --cleanup : Cleanup the data leftover from the earlier\n"
> + " test executions.\n"
Descriptions for these options already exist in the array above. It would be
better not to have two different sets of descriptions. Perhaps combine the two
sets into one, taking the best parts of each, since there are some good bits in
the existing descriptions that are missing here (such as "--cleanup" only
applying when a test succeeds).
If we were considering this C file alone, it would be sensible to iterate
through the array of options and print the corresponding description fields
from the array. Consider whether it is simple to do the same thing in the
Python version. If not, it might be better to write out the help string in
full (as you have done) so that it can be duplicated exactly in the Python
file, but in that case I think you should remove the descriptions from the
array, replacing them with empty strings, as they are not used at all.
> + "\n"
> + "Available arguments are:\n"
> + " arg : A valid test case number.\n"
"arg" is a bit confusing as the two things listed as "arg" (in the "options"
section above) are not test case numbers.
> + " list : Deprecated form of the --list option.\n"
> + " BASE_URL=arg : An alternate form of the --url option.\n\n";
These two lines differ slightly but unnecessarily from the corresponding lines
in the Python version.
These three "arguments" are all of different kinds: the first, "arg", is a
place-holder for a number and should definitely be considered a non-option
argument; the second, "list", is a literal string and is semantically
equivalent to an option but cannot be used with the numeric "arg"; the third,
"BASE_URL=arg", is also semantically equivalent to an option but can be used in
addition to "arg" or "list". I think it might be clearer to structure the help
without an "arguments" section, and add the two option-like arguments onto the
"options" section, something like this:
[[[
usage: %s [options] [test-number]
If a test case number is provided, execute that test.\n"
Otherwise execute all the tests in %s.\n"
Valid options:
--help : Print this usage message.\n"
--list : List the tests available in %s.\n"
--url arg : Use ARG as the base url for test execution.\n"
--fs-type arg : Create repositories using fs backend ARG.\n"
ARG could be either 'bdb' (default) or 'fsfs'.\n"
-v [--verbose] : Print verbose output.\n"
--cleanup : Cleanup the data leftover from the earlier\n"
test executions.\n"
list : A deprecated form of the --list option.\n"
BASE_URL=arg : A deprecated form of the --url option.\n"
]]]
[...]
> @@ -304,6 +338,10 @@
> case verbose_opt:
> verbose_mode = 1;
> break;
> + case help_opt:
> + /* Print usage */
Redundant comment.
> + usage(svn_path_basename(argv[0], pool));
> + exit(0);
[...]
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jun 18 21:17:54 2006