A good (second) try.
James Bowes <jbowes@xxxxxxxxxxxxxxxxxx> wrote:
> diff --git a/builtin-gc.c b/builtin-gc.c
> +
> +static int pack_refs;
Actually I think you want to use:
static int pack_refs = -1;
See below for why...
> +static int gc_config(const char *var, const char *value)
> +{
> + if (!strcmp(var, "gc.packrefs"))
> + if (strlen(value) == 0 || !strcmp(value, "notbare"))
> + pack_refs = !is_bare_repository();
> + else
> + pack_refs = git_config_bool(var, value);
> + else
> + return git_default_config(var, value);
> + return 0;
> +}
Gaaah. How about some curly braces around the then part of that
first if?
Actually, we typically just write this more like:
static int gc_config(const char *var, const char *value)
{
if (!strcmp(var, "gc.packrefs")) {
if (!strcmp(value, "notbare"))
pack_refs = -1;
else
pack_refs = git_config_bool(var, value);
}
return git_default_config(var, value);
}
> +int cmd_gc(int argc, const char **argv, const char *prefix)
> +{
> + int i;
> + int prune = 0;
> +
> + git_config(gc_config);
if (pack_refs < 0)
pack_refs = !is_bare_repository();
The is_bare_repository function guesses until the configuration
is done parsing; once the configuration has been parsed it has a
definate answer one way or the other. So what I'm suggesting you
do here is set pack_refs = -1 to mean use the is_bare_repository
setting, otherwise it stays what it was set to.
> + if (pack_refs)
> + if (run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
> + goto failure;
....
> + if (prune)
> + if (run_command_v_opt(argv_prune, RUN_GIT_CMD))
> + goto failure;
Gaah. Tabs-vs-spaces, not to mention that these aren't even lining
up the same way. I too prefer what Dsco suggested already:
if (prune && run_command_v_opt(argv_prune, RUN_GIT_CMD))
return error("failed to run %s", argv_prune[0]);
--
Shawn.
-
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