Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'

[Ralf Thielow <ralf.thielow@googlemail.com>]

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 <gitster@pobox.com>:
> Ralf Thielow <ralf.thielow@googlemail.com> 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 <ralf.thielow@googlemail.com>
>> ---
>>  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.
>