{"thread":{"id":"27269","subject":"[PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'","startedAt":"2011-05-05T16:58:10Z","lastAt":"2011-05-05T19:49:08Z","messageCount":4,"participants":["Ralf Thielow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"167125","messageId":"1304614690-12493-1-git-send-email-ralf.thielow@googlemail.com","threadId":"27269","inReplyTo":null,"subject":"[PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@googlemail.com","sentAt":"2011-05-05T16:58:10Z","receivedAt":"2011-05-05T16:58:10Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Show the usage with options for 'checkout' command on missing argument 'branch'.\n\nSigned-off-by: Ralf Thielow \n---\n builtin/checkout.c | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 38632fc..4aa613a 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \tif (opts.writeout_stage)\n \t\tdie(_(\"--ours/--theirs is incompatible with switching branches.\"));\n \n+\tif (new.name == NULL) {\n+\t\tusage_with_options(checkout_usage, options);\n+\t\treturn;\n+\t}\n+\n \treturn switch_branches(&opts, &new);\n }\n-- \n1.7.5.1\n"},{"id":"167143","messageId":"7vsjst6l5o.fsf@alter.siamese.dyndns.org","threadId":"27269","inReplyTo":"1304614690-12493-1-git-send-email-ralf.thielow@googlemail.com","subject":"Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-05T18:45:55Z","receivedAt":"2011-05-05T18:45:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> Show the usage with options for 'checkout' command on missing argument 'branch'.\n\nPlease describe what exact command line you typed, what output and side\neffect you got from the command, what you _think_ should have happened\ninstead, and what the differences between two are.\n\nIn other words, what problem you are trying to solve?\n\n> Signed-off-by: Ralf Thielow \n> ---\n> builtin/checkout.c | 5 +++++\n> 1 files changed, 5 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 38632fc..4aa613a 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> \tif (opts.writeout_stage)\n> \t\tdie(_(\"--ours/--theirs is incompatible with switching branches.\"));\n> \n> +\tif (new.name == NULL) {\n> +\t\tusage_with_options(checkout_usage, options);\n> +\t\treturn;\n> +\t}\n>\n> \treturn switch_branches(&opts, &new);\n> }\n\nWhat value are you returning from a function whose return type is int?\n\nIf you read the function switch_branches(), you would notice that it is\nprepared to handle the case where new.name is NULL (by the way, check\nagainst NULL is typically spelled as \"if (!new.name)\" as you can see\nthere), and then would realize that your change by itself cannot be a\ncorrect fix for whatever problem you are trying to solve.\n\nHave you run \"make test\" at all?\n\nIf you are changing this command:\n\n\t$ git checkout -b frotz\n\tSwitched to a new branch 'junk'\n\nto error out with a message, then that is a regression.\n"},{"id":"167157","messageId":"BANLkTikMu3fjbALTAwaaknskBw0H7LbZ-g@mail.gmail.com","threadId":"27269","inReplyTo":"7vsjst6l5o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@googlemail.com","sentAt":"2011-05-05T19:02:47Z","receivedAt":"2011-05-05T19:02:47Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Simple \"git checkout\" or \"git checkout \" don't tell me that i've done a mistake\non usage. It does nothing. This can be different when using parameters, but in\nthe end i think it should tell me the usage in this case.\n\n2011/5/5 Junio C Hamano :\n> Ralf Thielow writes:\n>\n>> Show the usage with options for 'checkout' command on missing argument 'branch'.\n>\n> Please describe what exact command line you typed, what output and side\n> effect you got from the command, what you _think_ should have happened\n> instead, and what the differences between two are.\n>\n> In other words, what problem you are trying to solve?\n>\n>> Signed-off-by: Ralf Thielow \n>> ---\n>>  builtin/checkout.c |    5 +++++\n>>  1 files changed, 5 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/builtin/checkout.c b/builtin/checkout.c\n>> index 38632fc..4aa613a 100644\n>> --- a/builtin/checkout.c\n>> +++ b/builtin/checkout.c\n>> @@ -1086,5 +1086,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n>>       if (opts.writeout_stage)\n>>               die(_(\"--ours/--theirs is incompatible with switching branches.\"));\n>>\n>> +     if (new.name == NULL) {\n>> +             usage_with_options(checkout_usage, options);\n>> +             return;\n>> +     }\n>>\n>>       return switch_branches(&opts, &new);\n>>  }\n>\n> What value are you returning from a function whose return type is int?\n>\n> If you read the function switch_branches(), you would notice that it is\n> prepared to handle the case where new.name is NULL (by the way, check\n> against NULL is typically spelled as \"if (!new.name)\" as you can see\n> there), and then would realize that your change by itself cannot be a\n> correct fix for whatever problem you are trying to solve.\n>\n> Have you run \"make test\" at all?\n>\n> If you are changing this command:\n>\n>        $ git checkout -b frotz\n>        Switched to a new branch 'junk'\n>\n> to error out with a message, then that is a regression.\n>\n"},{"id":"167168","messageId":"7vwri553nv.fsf@alter.siamese.dyndns.org","threadId":"27269","inReplyTo":"BANLkTikMu3fjbALTAwaaknskBw0H7LbZ-g@mail.gmail.com","subject":"Re: [PATCH] builtin/checkout.c: show usage with options on missing argument 'branch'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-05T19:49:08Z","receivedAt":"2011-05-05T19:49:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ralf Thielow writes:\n\n> Simple \"git checkout\" or \"git checkout \" don't tell me that i've done a mistake\n> on usage. It does nothing.\n\nThat is a designed behaviour. You did nothing wrong.\n\nBesides, that is a way people who use \"checkout -t -b\" to create topic\nbranches check their branch status. Pay attention to what the last\ncommand does in the following transcript.\n\n $ git checkout master\n $ git checkout -t -b side\n Branch side set up to track local branch master.\n Switched to a new branch 'side'\n $ git checkout master\n Switched to branch 'master'\n $ edit ; git commit ;# on 'master'\n $ git checkout side\n Switched to branch 'side'\n Your branch is behind 'master' by 1 commit, ...\n ... time passes ...\n $ git checkout\n Your branch is behind 'master' by 1 commit, ...\n\n \n"}]}