Re: [PATCH 01/15] rebase: extend the options for handling of empty commits

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 21, 2019, 23:49 UTC
Message-ID: <xmqqimm96xzc.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CABPp-BF5MaO60GAXbqYPfEePKyn_K-B_hQXNvTN_L4S6iA_93w@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 14 quoted lines

>> > +static long parse_empty_value(const char *value)
>> > +{
>> > +     if (!value)
>> > +             return EMPTY_UNSPECIFIED;
>> > +     else if (!strcasecmp(value, "drop"))
>> > +             return EMPTY_DROP;
>> > +     else if (!strcasecmp(value, "keep"))
>> > +             return EMPTY_KEEP;
>> > +     else if (!strcasecmp(value, "ask"))
>> > +             return EMPTY_ASK;
>>
>> Not an error but just silently ignored?
>
> Oops, I'll switch it to an error.

Not necessarily (see "Ahh, OK" below).

Show 16 quoted lines

>> > +     return EMPTY_UNSPECIFIED;
>> > +}
>> > +
>>
>> > +static int parse_opt_empty(const struct option *opt, const char *arg, int unset)
>> > +{
>> > +     struct rebase_options *options = opt->value;
>> > +     long value = parse_empty_value(arg);
>>
>> Ahh, OK.
>>
>> Wouldn't it be better to make the variable and the parsing helper
>> function of type "enum empty_type", not "long", just like the field
>> in the rebase_options struct?
>
> Indeed, I'll fix this up.

Thanks. I was wondering if there is something subtle I didn't see going on around here.