Junio C Hamano <gitster@xxxxxxxxx> writes: > Stefan Beller <sbeller@xxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html