Re: [GSoC][PATCH v4 2/2] submodule: port subcommand foreach from shell to C

From: Stefan Beller <sbeller@google.com>
Date: May 23, 2017, 20:57 UTC
Message-ID: <CAGZ79kYPUO34YUVR_u4sRuYz+Geo=wxwNEfCnyx+NQWQCQTkaQ@mail.gmail.com>
In-Reply-To: <20170523193627.GG115919@google.com>

On Tue, May 23, 2017 at 12:36 PM, Brandon Williams <bmwill@google.com> wrote:

Show 9 quoted lines

>
> You can set .git_cmd = 1 instead.
>
>> +             cpr.dir = list_item->name;
>> +             prepare_submodule_repo_env(&cpr.env_array);
>> +
>> +             argv_array_pushl(&cpr.args, "git", "--super-prefix", displaypath,
>
> And then you don't need to include "git" here.

even if git_cmd = 1 is set, you'd need a first dummy argument? cf. find_unpushed_submodules, See comment in 9cfa1c260f (serialize collection of refs that contain submodule changes, 2016-11-16)

Show 8 quoted lines

>> +
>> +     info.argc = argc;
>> +     info.argv = argv;
>> +     info.prefix = prefix;
>> +     info.quiet = !!quiet;
>> +     info.recursive = !!recursive;
>
> If these values are boolean why do we need to do the extra '!!'?

Actually that was my advice. As we only have a limited space in a single bit, strange things happen when you were to do:

    quiet = 2; /* be extra quiet */
    info.quiet = quiet;

This is not the case here, but other commands have evolved over time to first take a OPT_BOOL, and then in a later patch an OPT_INT. (some commands take a "-v -v -v")

And by having the double negative we'd have some defensive programming right here. (To prove I am not telling crazy stories, $ git log -S \!\!)