{"thread":{"id":"21417","subject":"[PATCH] clone: detect extra arguments","startedAt":"2009-10-29T08:10:30Z","lastAt":"2009-10-31T11:32:27Z","messageCount":6,"participants":["Jonathan Nieder","Jeff King","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"126252","messageId":"20091029081030.GA11213@progeny.tock","threadId":"21417","inReplyTo":null,"subject":"[PATCH] clone: detect extra arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-29T08:10:30Z","receivedAt":"2009-10-29T08:10:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"If git clone is given more than two non-option arguments, it\nsilently throws away all but the first one. Complain instead.\n\nDiscovered by comparing the new builtin clone to the old\ngit-clone.sh.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin-clone.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5762a6f..76ad581 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -377,6 +377,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t builtin_clone_usage, 0);\n \n+\tif (argc > 2)\n+\t\tdie(\"Too many arguments.\");\n+\n \tif (argc == 0)\n \t\tdie(\"You must specify a repository to clone.\");\n \n-- \n1.6.5.2\n"},{"id":"126286","messageId":"20091029160614.GB7622@sigill.intra.peff.net","threadId":"21417","inReplyTo":"20091029081030.GA11213@progeny.tock","subject":"Re: [PATCH] clone: detect extra arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-29T16:06:15Z","receivedAt":"2009-10-29T16:06:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 29, 2009 at 03:10:30AM -0500, Jonathan Nieder wrote:\n\n> If git clone is given more than two non-option arguments, it\n> silently throws away all but the first one. Complain instead.\n> [...]\n> +\tif (argc > 2)\n> +\t\tdie(\"Too many arguments.\");\n> +\n\nShould we maybe be showing the usage in this case?\n\n> \tif (argc == 0)\n> \t\tdie(\"You must specify a repository to clone.\");\n\nProbably we should do the same here, too.\n\n-Peff\n"},{"id":"126374","messageId":"20091030111919.GA13242@progeny.tock","threadId":"21417","inReplyTo":"20091029160614.GB7622@sigill.intra.peff.net","subject":"Re: [PATCH] clone: detect extra arguments","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-10-30T11:19:19Z","receivedAt":"2009-10-30T11:19:19Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> Should we maybe be showing the usage in this case?\n\nSounds reasonable. How about this patch on top?\n\n-- %< --\nSubject: [PATCH] clone: print usage on wrong number of arguments\n\ngit clone's short usage string is only 22 lines, so an error\nmessage plus usage string still fits comfortably on an 80x24\nterminal.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin-clone.c | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 76ad581..736d9e1 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -378,10 +378,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t builtin_clone_usage, 0);\n \n \tif (argc > 2)\n-\t\tdie(\"Too many arguments.\");\n+\t\tusage_msg_opt(\"Too many arguments.\",\n+\t\t\tbuiltin_clone_usage, builtin_clone_options);\n \n \tif (argc == 0)\n-\t\tdie(\"You must specify a repository to clone.\");\n+\t\tusage_msg_opt(\"You must specify a repository to clone.\",\n+\t\t\tbuiltin_clone_usage, builtin_clone_options);\n \n \tif (option_mirror)\n \t\toption_bare = 1;\n-- \n1.6.5.2\n"},{"id":"126384","messageId":"20091030144525.GA22583@coredump.intra.peff.net","threadId":"21417","inReplyTo":"20091030111919.GA13242@progeny.tock","subject":"Re: [PATCH] clone: detect extra arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-30T14:45:25Z","receivedAt":"2009-10-30T14:45:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2009 at 06:19:19AM -0500, Jonathan Nieder wrote:\n\n> > Should we maybe be showing the usage in this case?\n> \n> Sounds reasonable. How about this patch on top?\n\nI do think it's an improvement, but...\n\n> -- %< --\n> Subject: [PATCH] clone: print usage on wrong number of arguments\n> \n> git clone's short usage string is only 22 lines, so an error\n> message plus usage string still fits comfortably on an 80x24\n> terminal.\n\nThe extra blank lines introduced by usage_msg_opt make it 25 lines,\nscrolling the message right off of my terminal screen. ;)\n\nBut looking at the usage message, there is some potential for cleanup.\nSo maybe this on top (or between your 1 and 2)?\n\n-- >8 --\nSubject: [PATCH] clone: hide \"naked\" option from usage message\n\nThis is just a little-known synonym for bare, and there is\nlittle point in advertising both (we don't even include it\nin the manpage). Removing it also makes the usage message\none line shorter, giving just enough room for an information\nmessage in a 24-line terminal.\n\nSigned-off-by: Jeff King \n---\n builtin-clone.c | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 736d9e1..ce0d79a 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -51,7 +51,9 @@ static struct option builtin_clone_options[] = {\n \tOPT_BOOLEAN('n', \"no-checkout\", &option_no_checkout,\n \t\t \"don't create a checkout\"),\n \tOPT_BOOLEAN(0, \"bare\", &option_bare, \"create a bare repository\"),\n-\tOPT_BOOLEAN(0, \"naked\", &option_bare, \"create a bare repository\"),\n+\t{ OPTION_BOOLEAN, 0, \"naked\", &option_bare, NULL,\n+\t\t\"create a bare repository\",\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n \tOPT_BOOLEAN(0, \"mirror\", &option_mirror,\n \t\t \"create a mirror repository (implies bare)\"),\n \tOPT_BOOLEAN('l', \"local\", &option_local,\n-- \n1.6.5.1.143.g1dab74.dirty\n"},{"id":"126385","messageId":"20091030145108.GA881@coredump.intra.peff.net","threadId":"21417","inReplyTo":"20091030144525.GA22583@coredump.intra.peff.net","subject":"Re: [PATCH] clone: detect extra arguments","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-10-30T14:51:09Z","receivedAt":"2009-10-30T14:51:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2009 at 10:45:25AM -0400, Jeff King wrote:\n\n> But looking at the usage message, there is some potential for cleanup.\n\nAlso, we should probably do this (I did it as a patch on master, though,\nas it is an independent fix):\n\n-- >8 --\nSubject: [PATCH] clone: fix --recursive usage message\n\nLooks like a mistaken cut-and-paste in e7fed18a.\n\nSigned-off-by: Jeff King \n---\n builtin-clone.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5762a6f..436e8da 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -61,7 +61,7 @@ static struct option builtin_clone_options[] = {\n \tOPT_BOOLEAN('s', \"shared\", &option_shared,\n \t\t \"setup as shared repository\"),\n \tOPT_BOOLEAN(0, \"recursive\", &option_recursive,\n-\t\t \"setup as shared repository\"),\n+\t\t \"initialize submodules in the clone\"),\n \tOPT_STRING(0, \"template\", &option_template, \"path\",\n \t\t \"path the template repository\"),\n \tOPT_STRING(0, \"reference\", &option_reference, \"repo\",\n-- \n1.6.5.1.143.g1dab74.dirty\n"},{"id":"126518","messageId":"200910311232.27278.johan@herland.net","threadId":"21417","inReplyTo":"20091030145108.GA881@coredump.intra.peff.net","subject":"Re: [PATCH] clone: detect extra arguments","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-10-31T11:32:27Z","receivedAt":"2009-10-31T11:32:27Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Friday 30 October 2009, Jeff King wrote:\n> On Fri, Oct 30, 2009 at 10:45:25AM -0400, Jeff King wrote:\n> > But looking at the usage message, there is some potential for cleanup.\n> \n> Also, we should probably do this (I did it as a patch on master, though,\n> as it is an independent fix):\n> \n> -- >8 --\n> Subject: [PATCH] clone: fix --recursive usage message\n> \n> Looks like a mistaken cut-and-paste in e7fed18a.\n\nYes. Please fix my screwup.\n\n> Signed-off-by: Jeff King \n\nAcked-by: Johan Herland \n\n> ---\n> builtin-clone.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 5762a6f..436e8da 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -61,7 +61,7 @@ static struct option builtin_clone_options[] = {\n> \tOPT_BOOLEAN('s', \"shared\", &option_shared,\n> \t\t \"setup as shared repository\"),\n> \tOPT_BOOLEAN(0, \"recursive\", &option_recursive,\n> -\t\t \"setup as shared repository\"),\n> +\t\t \"initialize submodules in the clone\"),\n> \tOPT_STRING(0, \"template\", &option_template, \"path\",\n> \t\t \"path the template repository\"),\n> \tOPT_STRING(0, \"reference\", &option_reference, \"repo\",\n> \n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"}]}