On Tue, Jan 22, 2013 at 11:01 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Lars Hjemli <hje...@gmail.com> 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.


[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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to