From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 Oct 2010 16:33:07 GMT
Subject: Re: [PATCH 3/3] builtins: check for startup_info->help, print and exit early
Message-ID: <7vvd4y16t8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1287495320-27278-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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...

> 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".

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