On Tue, Jan 22, 2013 at 11:01 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Lars Hjemli <hjemli@xxxxxxxxx> writes:
>
>> +static struct option builtin_all_options[] = {
>> + OPT_BOOLEAN('c', "clean", &only_clean, N_("only show clean repositories")),
>> + OPT_BOOLEAN('d', "dirty", &only_dirty, N_("only show dirty repositories")),
>> + OPT_END(),
>> +};
>
> Shouldn't this be more like OPT_SET_INT() on a same variable that is
> initialized to "all"? Alternatively you could validate the input
> and die when both are given.
OPT_SET_INT() seems appropriate, will fix.
>
>> +int cmd_all(int argc, const char **argv, const char *prefix)
>> +{
>> + struct strbuf path = STRBUF_INIT;
>> +
>> + if (!getcwd(root, sizeof(root)))
>> + return 1;
>> +
>> + argc = parse_options(argc, argv, prefix, builtin_all_options,
>> + builtin_all_usage, PARSE_OPT_STOP_AT_NON_OPTION);
>> +
>> + unsetenv(GIT_DIR_ENVIRONMENT);
>> + unsetenv(GIT_WORK_TREE_ENVIRONMENT);
>
> Don't you need to clear other variables whose uses are closely tied
> to a single repository, like GIT_INDEX_FILE, etc.?
>
> I suspect that explicitly exporting GIT_DIR and GIT_WORK_TREE (and
> nothing else) in handle_repo() to the location you discovered before
> you run the per-repository command via run_command_v_opt(), might be
> a better alternative. The user could be sharing objects in all
> repositories by permanently setting GIT_OBJECT_DIRECTORY to a single
> place.
>
This sounds like a nice plan, I'll test it and send an updated patch.
>> diff --git a/command-list.txt b/command-list.txt
>> index 7e8cfec..f955895 100644
>> --- a/command-list.txt
>> +++ b/command-list.txt
>> @@ -1,6 +1,7 @@
>> # List of known git commands.
>> # command name category [deprecated] [common]
>> git-add mainporcelain common
>> +git-all mainporcelain
>> git-am mainporcelain
>> git-annotate ancillaryinterrogators
>> git-apply plumbingmanipulators
>
> I am not very interested in this topic in the first place, but this
> does not (at least not yet) sound like a main Porcelain to me.
There doesn't seem to be a better category, but I'm open for suggestions.
>
> "all" may be a word other people may want to use to call collections
> of things other than "Git repositories", and that use may turn out
> to be more useful in general. A name that makes it clear that this
> is about "repositories", i.e. along the lines of "git for-each-repo"
> or something, would be a better name that does not squat on such a
> short and sweet name.
>
In principle I agree with your reasoning on this, but in practice I
fail to see what other kind of things `git all` could naturally refer
to. Also, having a short and sweet way to perform the tasks
implemented by this patch was my main motivation for writing it [1],
hence `git for-each-repo` isn't as compelling (too much typing).
There's always aliases, but I'd prefer it if future git supported `git
all` by default.
--
larsh
[1] Originally as a shell script, used at $WORK for ~2 years
--
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