Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'
- From
Ralf Thielow <ralf.thielow@googlemail.com>
- Date
- May 5, 2011, 19:02 UTC
- Message-ID
- <BANLkTikMu3fjbALTAwaaknskBw0H7LbZ-g@mail.gmail.com>
- 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 <gitster@pobox.com>:
Show 48 quoted lines
> 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.
>