Atharva Raykar <raykar.ath@xxxxxxxxx> writes:
>> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>>> static void update_submodule(struct update_clone_data *ucd)
>>> {
>>> fprintf(stdout, "dummy %s %d\t%s\n",
>>> @@ -2379,6 +2552,79 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>>> return update_submodules(&suc);
>>> }
>>>
>>> +static int run_update_procedure(int argc, const char **argv, const char *prefix)
>>> +{
>>> + int force = 0, quiet = 0, nofetch = 0, just_cloned = 0;
>>> + char *prefixed_path, *update = NULL;
>>> + char *sha1 = NULL, *subsha1 = NULL;
>>> + struct update_data update_data = UPDATE_DATA_INIT;
>>> +
>>> + struct option options[] = {
>>> + OPT__QUIET(&quiet, N_("suppress output for update by rebase or merge")),
>>> + OPT__FORCE(&force, N_("force checkout updates"), 0),
>>> + OPT_BOOL('N', "no-fetch", &nofetch,
>>> + N_("don't fetch new objects from the remote site")),
>>> + OPT_BOOL(0, "just-cloned", &just_cloned,
>>> + N_("overrides update mode in case the repository is a fresh clone")),
>>> + OPT_INTEGER(0, "depth", &update_data.depth, N_("depth for shallow fetch")),
>>> + OPT_STRING(0, "prefix", &prefix,
>>> + N_("path"),
>>> + N_("path into the working tree")),
>>> + OPT_STRING(0, "update", &update,
>>> + N_("string"),
>>> + N_("rebase, merge, checkout or none")),
>>> + OPT_STRING(0, "recursive-prefix", &update_data.recursive_prefix, N_("path"),
>>> + N_("path into the working tree, across nested "
>>> + "submodule boundaries")),
>>> + OPT_STRING(0, "sha1", &sha1, N_("string"),
>>> + N_("SHA1 expected by superproject")),
>>> + OPT_STRING(0, "subsha1", &subsha1, N_("string"),
>>> + N_("SHA1 of submodule's HEAD")),
>>> + OPT_END()
>>> + };
>>> +
>>> + const char *const usage[] = {
>>> + N_("git submodule--helper run-update-procedure [<options>] <path>"),
>>> + NULL
>>> + };
>>> +
>>> + argc = parse_options(argc, argv, prefix, options, usage, 0);
>>> +
>>> + if (argc != 1)
>>> + usage_with_options(usage, options);
>>> + update_data.force = !!force;
>>> + update_data.quiet = !!quiet;
>>> + update_data.nofetch = !!nofetch;
>>> + update_data.just_cloned = !!just_cloned;
>>
>> For all of these just pass the reference to the update_data variable
>> directly in the OPT_*(). No need to set an "int force", only to copy it
>> over to update_data.force. Let's just use the latter only.
>
> Hmm, I'm trying to remember why the single bit values are treated this way
> in this whole file...
>
> ...there seems to be no good reason for it. The API docs for parse options
> state that OPT_BOOL() is guaranteed to return either zero or one, so that
> double negation does look unnecessary.
I forgot to mention why I did not address this change in my v3 patch.
The reason why we are handling boolean values this way is because they
are declared as bitfields in the 'update_data' struct. Since we cannot
take the address of bitfields, we have to use a different variable to
store when using 'parse_options()'.
