From: Bo Yang <struggleyb.nku@gmail.com>
Date: Wed, 30 Jun 2010 14:35:47 GMT
Subject: Re: [PATCH 02/12] parse-options: add two helper functions
Message-ID: <AANLkTikKo3yr0eY9Lmv98TtFfTWjYG7HG6GPfwmoB6Aa@mail.gmail.com>
In-Reply-To: <7vaaqggwsh.fsf@alter.siamese.dyndns.org>

Hi Junio,

On Mon, Jun 28, 2010 at 2:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> diff --git a/parse-options.c b/parse-options.c
>> index cbb49d3..013dbdb 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -439,6 +439,27 @@ unknown:
>>       return PARSE_OPT_DONE;
>>  }
>>
>> +const char *parse_options_current(struct parse_opt_ctx_t *ctx)
>> +{
>> +     return ctx->argv[0];
>> +}
>> +
>> +int parse_options_next(struct parse_opt_ctx_t *ctx, int retain)
>> +{
>> +     if (ctx->argc <= 0)
>> +             return -1;
>> +
>> +     if (retain == 1)
>> +     {
>> +             ctx->out[ctx->cpidx++] = ctx->argv[0];
>> +     }
>
> Style.  Either drop the unnecessary curly pair, or open the curly at the
> end of the line that has the closing parenthesis of "if" condition.
>
> These two functions makes sense.  You could then use parse-options-step to
> let the machinery mostly be driven by the usual table lookup, and when
> (and only when) the machinery says "I don't know what this is", you can
> check "current" to see what it is (e.g. it may be "-L"), handle it
> yourself, and if you need to do something different (e.g. eat the <path>
> that immedately follows "-L"), you can use "next" to skip it without ever
> showing that to the table based parseopt machinery.
>
> I however wonder why you would need two passes if you have these two APIs
> into parse-options machinery, though...
>

Hmm, yes, I have rewrite the the parsing code as a one pass one. It is
3 or 4 weeks since this parsing code written, so I can't remember
clearly why it adopt this two pass way. :-)

-- 
Regards!
Bo
----------------------------
My blog: http://blog.morebits.org
Why Git: http://www.whygitisbetterthanx.com/