From: Ralf Thielow Date: Thu, 05 May 2011 19:02:47 GMT Subject: Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch' Message-ID: In-Reply-To: <7vsjst6l5o.fsf@alter.siamese.dyndns.org> Simple "git checkout" or "git checkout " don't tell me that i've done a mistake on usage. It does nothing. This can be different when using parameters, but in the end i think it should tell me the usage in this case. 2011/5/5 Junio C Hamano : > Ralf Thielow writes: > >> Show the usage with options for 'checkout' command on missing argument 'branch'. > > Please describe what exact command line you typed, what output and side > effect you got from the command, what you _think_ should have happened > instead, and what the differences between two are. > > In other words, what problem you are trying to solve? > >> Signed-off-by: Ralf Thielow >> --- >>  builtin/checkout.c |    5 +++++ >>  1 files changed, 5 insertions(+), 0 deletions(-) >> >> diff --git a/builtin/checkout.c b/builtin/checkout.c >> index 38632fc..4aa613a 100644 >> --- a/builtin/checkout.c >> +++ b/builtin/checkout.c >> @@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) >>       if (opts.writeout_stage) >>               die(_("--ours/--theirs is incompatible with switching branches.")); >> >> +     if (new.name == NULL) { >> +             usage_with_options(checkout_usage, options); >> +             return; >> +     } >> >>       return switch_branches(&opts, &new); >>  } > > What value are you returning from a function whose return type is int? > > If you read the function switch_branches(), you would notice that it is > prepared to handle the case where new.name is NULL (by the way, check > against NULL is typically spelled as "if (!new.name)" as you can see > there), and then would realize that your change by itself cannot be a > correct fix for whatever problem you are trying to solve. > > Have you run "make test" at all? > > If you are changing this command: > >        $ git checkout -b frotz >        Switched to a new branch 'junk' > > to error out with a message, then that is a regression. >