From: Nguyen Thai Ngoc Duy Date: Wed, 20 Oct 2010 01:18:22 GMT Subject: Re: [PATCH 3/3] builtins: check for startup_info->help, print and exit early Message-ID: In-Reply-To: <7vvd4y16t8.fsf@alter.siamese.dyndns.org> 2010/10/19 Junio C Hamano : > Nguyễn Thái Ngọc Duy 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