Hi,
Sorry for the slow response.
Dmitry Ivankov wrote:
> There was custom options parsing. As more options arise it will
> be easier to add and document new options with parse-options api.
In particular this gives us a "test-svn-fe -h" command --- sounds
good. Might make sense to combine this with the patch that
parsoptifies contrib/svn-fe/svn-fe.c.
> --- a/test-svn-fe.c
> +++ b/test-svn-fe.c
> @@ -3,28 +3,38 @@
> */
>
> #include "git-compat-util.h"
> +#include "parse-options.h"
> #include "vcs-svn/svndump.h"
> #include "vcs-svn/svndiff.h"
> #include "vcs-svn/sliding_window.h"
> #include "vcs-svn/line_buffer.h"
>
> -static const char test_svnfe_usage[] =
> - "test-svn-fe (<dumpfile> | [-d] <preimage> <delta> <len>)";
> +static const char * const test_svnfe_usage[] = {
> + "test-svn-fe (<dumpfile> | -d <preimage> <delta> <len>)",
> + NULL
> +};
With this API, we're allowed to print multiple usage strings. Might as
well take advantage of that for clarity:
static const char * const test_svnfe_usage[] = {
"test-svn-fe <dumpfile>",
"test-svn-fe -d <preimage> <delta> <len>",
NULL
};
>
> +static int d;
> +
The variable name is not so memorable. Maybe something like
"apply_delta" would do.
> -static int apply_delta(int argc, char *argv[])
> +static struct option test_svnfe_options[] = {
> + OPT_SET_INT('d', NULL, &d, "test apply_delta", 1),
> + OPT_END()
> +};
Might make sense to take the opportunity to add a mnemonic long
option name while at it:
OPT_SET_INT('d', "apply-delta", ...
[...]
> @@ -37,10 +47,16 @@ static int apply_delta(int argc, char *argv[])
> return 0;
> }
>
> -int main(int argc, char *argv[])
> +int main(int argc, const char *argv[])
> {
> - if (argc == 2) {
> - if (svndump_init(argv[1]))
> + argc = parse_options(argc, argv, NULL, test_svnfe_options,
> + test_svnfe_usage, 0);
> +
> + if (d)
> + return apply_delta(argc, argv);
> +
> + if (argc == 1) {
Probably easier to read with the simple and exceptional case first.
if (apply_delta_instead)
return apply_delta(argc, argv);
if (argc != 1)
usage_with_options(...);
if (svndump_init(argv[0]))
return 1;
...
> + if (svndump_init(argv[0]))
> return 1;
> svndump_read(NULL);
> svndump_deinit();
> @@ -48,7 +64,5 @@ int main(int argc, char *argv[])
> return 0;
> }
>
> - if (argc >= 2 && !strcmp(argv[1], "-d"))
> - return apply_delta(argc, argv);
> - usage(test_svnfe_usage);
> + usage_with_options(test_svnfe_usage, test_svnfe_options);
Except for the minor nits noted above (in particular, hopefully this
can be squashed with the corresponding svn-fe patch),
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
test-svn-fe.c | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/test-svn-fe.c b/test-svn-fe.c
index 0aab2450..db56b6ba 100644
--- a/test-svn-fe.c
+++ b/test-svn-fe.c
@@ -3,21 +3,23 @@
*/
#include "git-compat-util.h"
-#include "parse-options.h"
#include "vcs-svn/svndump.h"
#include "vcs-svn/svndiff.h"
#include "vcs-svn/sliding_window.h"
#include "vcs-svn/line_buffer.h"
+#include "parse-options.h"
static const char * const test_svnfe_usage[] = {
- "test-svn-fe (<dumpfile> | -d <preimage> <delta> <len>)",
+ "test-svn-fe <dumpfile>",
+ "test-svn-fe -d <preimage> <delta> <len>",
NULL
};
-static int d;
+static int apply_delta_instead;
static struct option test_svnfe_options[] = {
- OPT_SET_INT('d', NULL, &d, "test apply_delta", 1),
+ OPT_SET_INT('d', "apply-delta",
+ &apply_delta_instead, "apply a subversion-format delta", 1),
OPT_END()
};
@@ -52,17 +54,16 @@ int main(int argc, const char *argv[])
argc = parse_options(argc, argv, NULL, test_svnfe_options,
test_svnfe_usage, 0);
- if (d)
+ if (apply_delta_instead)
return apply_delta(argc, argv);
- if (argc == 1) {
- if (svndump_init(argv[0]))
- return 1;
- svndump_read(NULL);
- svndump_deinit();
- svndump_reset();
- return 0;
- }
+ if (argc != 1)
+ usage_with_options(test_svnfe_usage, test_svnfe_options);
- usage_with_options(test_svnfe_usage, test_svnfe_options);
+ if (svndump_init(argv[0]))
+ return 1;
+ svndump_read(NULL);
+ svndump_deinit();
+ svndump_reset();
+ return 0;
}
--
1.7.6
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html