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

[Brandon Williams <bmwill@google.com>]

On 05/23, Stefan Beller wrote:
> On Tue, May 23, 2017 at 12:36 PM, Brandon Williams <bmwill@google.com> wrote:
> >
> > 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)

Different subsystem, you don't need a dummy first argument.  The
revision walking code does (for some reason) need a dummy first
argument.

> 
> >> +
> >> +     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 \!\!)

All good, I didn't notice that they were bit fields.

-- 
Brandon Williams