Re: [PATCH 3/3] builtins: check for startup_info->help, print and exit early

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/10/19 Junio C Hamano <gitster@xxxxxxxxx>:
> Nguyán ThÃi Ngác Duy <pclouds@xxxxxxxxx> writes:
>
>> These commands need more than just git_config() before parsing
>> commmand line arguments. Some of these activities will unconditionally
>> look into a repository. When startup_info->help is TRUE, no repository
>> is set up and the caller expects callees to print help usage and exit,
>> no more.
>>
>> Do as expected.
>> ---
>
> No sign-off given to any of the three patches...

Oops, I removed old signoffs and accidentally mine too.

>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 87976f0..9f152ed 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -667,6 +667,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>> Â Â Â Â Â Â Â OPT_END(),
>> Â Â Â };
>>
>> + Â Â if (startup_info->help)
>> + Â Â Â Â Â Â usage_with_options(builtin_branch_usage, options);
>> +
>> Â Â Â git_config(git_branch_config, NULL);
>
> I can just say:
>
> Â Â$ cd / && git branch -h
> Â Âusage: git branch [options] ...
>
> without your patch just fine (and no I am not insane to make / controlled
> by git).
>
> The same for checkout-index, commit, ls-files, etc.
>
>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index c304638..f040171 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -191,6 +191,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>>
>> Â Â Â git_config(gc_config, NULL);
>>
>> + Â Â if (startup_info->help)
>> + Â Â Â Â Â Â usage_with_options(builtin_gc_usage, builtin_gc_options);
>> +
>> Â Â Â if (pack_refs < 0)
>> Â Â Â Â Â Â Â pack_refs = !is_bare_repository();
>
> This one is curious. ÂWhy do you need a call to git_config() in "gc" only?
> You don't do that for e.g. "branch".

Hmm.. I don't think that git_config() is needed. It does no harm, but
no gain either.

> While I do not see anything glaringly wrong with this change, I am not
> sure I am getting the point of these patches.

As Jonathan pointed out, the series makes "git foo -h" works even in
special cases where setup code may terminate program early. Apparently
my test in hurry does not work great.
-- 
Duy
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]