Re: [PATCH 1/3] fast-export: improve argument parsing

From: Junio C Hamano <gitster@pobox.com>
Date: May 9, 2013, 22:17 UTC
Message-ID: <7vzjw37q7o.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1368063095-22861-2-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> We don't want to pass arguments specific to fast-export to
> setup_revisions.

Interesting.  What bad things happen with the current order?

Does "fast-export --export-marks=foo" causes setup_revisions() to mistakenly eat --export-marks=foo and barf?

Show 20 quoted lines

>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  builtin/fast-export.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index d60d675..6e46057 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -686,8 +686,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
>  	revs.topo_order = 1;
>  	revs.show_source = 1;
>  	revs.rewrite_parents = 1;
> +	argc = parse_options(argc, argv, prefix, options, fast_export_usage,
> +			PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
>  	argc = setup_revisions(argc, argv, &revs, NULL);
> -	argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
>  	if (argc > 1)
>  		usage_with_options (fast_export_usage, options);

There is a SP between the function name and its arguments here ;-)