On Tue, Nov 09 2021, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> Change the parse_nodash_opt() function to use "enum
>> parse_opt_result". In 352e761388b (parse-options.[ch]: consistently
>> use "enum parse_opt_result", 2021-10-08) its only caller
>> parse_options_step() started using that return type, and the
>> get_value() which will be called and return from it uses the same
>> enum.
>>
>> Let's do the same here so that this function always returns an "enum
>> parse_opt_result" value.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>> ---
>> parse-options.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/parse-options.c b/parse-options.c
>> index fc5b43ff0b2..629e7963497 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -404,8 +404,9 @@ static enum parse_opt_result parse_long_opt(
>> return PARSE_OPT_UNKNOWN;
>> }
>>
>> -static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
>> - const struct option *options)
>> +static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
>> + const char *arg,
>> + const struct option *options)
>> {
>> const struct option *all_opts = options;
>>
>> @@ -415,7 +416,7 @@ static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
>> if (options->short_name == arg[0] && arg[1] == '\0')
>> return get_value(p, options, all_opts, OPT_SHORT);
>> }
>> - return -2;
>> + return PARSE_OPT_ERROR;
>> }
>
> The current caller only checks to skip a token that yields 0 (aka
> PARSE_OPT_DONE) and does not distinguish between other values, so
> this won't change the behaviour of the current code, but it is
> not clear if returning -1 (aka PARSE_OPT_ERROR) is better than -2
> (aka PARSE_OPT_HELP).
I think PARSE_OPT_ERROR is probably better.
It looks like the -2 return value might have been somewhat blindly
copy/pasted between 07fe54db3cd (parse-opt: do not print errors on
unknown options, return -2 intead., 2008-06-23) and 51a9949eda7
(parseopt: add PARSE_OPT_NODASH, 2009-05-07).
I.e. we use the full enum values for the code in the former, but in the
latter we're just looking for "not zero", so error/-1 seemed like a
better fit.
