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