Junio C Hamano <[email protected]> writes:

> Stefan Beller <[email protected]> writes:
>
>> +            else {
>> +                    submodule->update_command = NULL;
>> +                    if (!strcmp(value, "none"))
>> +                            submodule->update = SM_UPDATE_NONE;
>> +                    else if (!strcmp(value, "checkout"))
>> +                            submodule->update = SM_UPDATE_CHECKOUT;
>> +                    else if (!strcmp(value, "rebase"))
>> +                            submodule->update = SM_UPDATE_REBASE;
>> +                    else if (!strcmp(value, "merge"))
>> +                            submodule->update = SM_UPDATE_MERGE;
>> +                    else if (!skip_prefix(value, "!", 
>> &submodule->update_command))
>> +                            die(_("invalid value for %s"), var);
>> +            }
>
> I think this "string to enum" parser can become a separate helper
> function in this patch, so that later patch can use it to parse the
> "--update" option in the update_clone() function.
>
> That would solve the slight inconsistency we have seen
>
> +             if ((pp->update && !strcmp(pp->update, "none")) ||
> +                 (!pp->update && sub->update == SM_UPDATE_NONE)) {
>
> in that patch.

In addition, you would eventually want the reverse translation,
i.e. SM_UPDATE_ENUM to string, when you rebase the second patch of
your sb/submodule-init series (I just tried it myself and realized
that a patch to refactor the above part would be the best place to
add such a helper).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to