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

[Felipe Contreras <felipe.contreras@gmail.com>]

On Thu, May 9, 2013 at 5:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?

No, apparently it skips them. But try 'git fast-export --export-marks
marks HEAD'.

>> 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 ;-)

Yeah, and I already did my part: I sent a patch to fix this style. Not
that it has anything to do with this patch.

-- 
Felipe Contreras