{"thread":{"id":"24901","subject":"[PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.","startedAt":"2010-08-29T17:39:19Z","lastAt":"2010-09-01T02:20:08Z","messageCount":10,"participants":["Thiago Farina","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"149218","messageId":"c9d88eb8cbd5b025b0c0112be05d3c0fe993de25.1283103426.git.tfransosi@gmail.com","threadId":"24901","inReplyTo":null,"subject":"[PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-08-29T17:39:19Z","receivedAt":"2010-08-29T17:39:19Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"This improves the usage output by adding builtin_merge_recursive_usage string\nthat follows the same pattern used by the other builtin commands.\n\nAlso it uses usage() function instead of the usagef() function.\n\nThe previous output for git merger-recursive was:\nusage: merge-recursive <base>... -- <head> <remote> ...\n\nNow the output is:\nusage: git merge-recursive <base>... -- <head> <remote> ...\n\nSigned-off-by: Thiago Farina <tfransosi@gmail.com>\n---\n builtin/merge-recursive.c |   17 ++++++++++-------\n 1 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex d8875d5..46c2a85 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -3,6 +3,9 @@\n #include \"tag.h\"\n #include \"merge-recursive.h\"\n \n+static const char builtin_merge_recursive_usage[] =\n+\t\"git merge-recursive <base>... -- <head> <remote> ...\";\n+\n static const char *better_branch_name(const char *branch)\n {\n \tstatic char githead_env[8 + 40 + 1];\n@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\to.subtree_shift = \"\";\n \n \tif (argc < 4)\n-\t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n+\t\tusage(builtin_merge_recursive_usage);\n \n \tfor (i = 1; i < argc; ++i) {\n \t\tconst char *arg = argv[i];\n@@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--\")) {\n \t\t\tif (!arg[2])\n \t\t\t\tbreak;\n-\t\t\tif (!strcmp(arg+2, \"ours\"))\n+\t\t\tif (!strcmp(arg + 2, \"ours\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(arg+2, \"theirs\"))\n+\t\t\telse if (!strcmp(arg + 2, \"theirs\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(arg+2, \"subtree\"))\n+\t\t\telse if (!strcmp(arg + 2, \"subtree\"))\n \t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(arg+2, \"subtree=\"))\n+\t\t\telse if (!prefixcmp(arg + 2, \"subtree=\"))\n \t\t\t\to.subtree_shift = arg + 10;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (bases_count < ARRAY_SIZE(bases)-1) {\n+\t\tif (bases_count < ARRAY_SIZE(bases) - 1) {\n \t\t\tunsigned char *sha = xmalloc(20);\n \t\t\tif (get_sha1(argv[i], sha))\n \t\t\t\tdie(\"Could not parse object '%s'\", argv[i]);\n@@ -58,7 +61,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\twarning(\"Cannot handle more than %d bases. \"\n \t\t\t\t\"Ignoring %s.\",\n-\t\t\t\t(int)ARRAY_SIZE(bases)-1, argv[i]);\n+\t\t\t\t(int)ARRAY_SIZE(bases) - 1, argv[i]);\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n-- \n1.7.2.1.95.g3d045\n"},{"id":"149242","messageId":"20100829214648.GE1890@burratino","threadId":"24901","inReplyTo":"c9d88eb8cbd5b025b0c0112be05d3c0fe993de25.1283103426.git.tfransosi@gmail.com","subject":"Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-29T21:46:48Z","receivedAt":"2010-08-29T21:46:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Thiago,\n\nThiago Farina wrote:\n\n> This improves the usage output by adding builtin_merge_recursive_usage string\n> that follows the same pattern used by the other builtin commands.\n> \n> Also it uses usage() function instead of the usagef() function.\n\nFWIW cmd_merge_recursive is used to handle four different commands.\n\n$ git grep -F -e cmd_merge_recursive\nbuiltin.h:extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\nbuiltin/merge-recursive.c:int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\ngit.c:          { \"merge-recursive\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\ngit.c:          { \"merge-recursive-ours\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\ngit.c:          { \"merge-recursive-theirs\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\ngit.c:          { \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n\nWhat will be the output from\n\n $ git merge-subtree -h\n\nafter this change?\n\n> --- a/builtin/merge-recursive.c\n> +++ b/builtin/merge-recursive.c\n> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n>  \t\tif (!prefixcmp(arg, \"--\")) {\n>  \t\t\tif (!arg[2])\n>  \t\t\t\tbreak;\n> -\t\t\tif (!strcmp(arg+2, \"ours\"))\n> +\t\t\tif (!strcmp(arg + 2, \"ours\"))\n\nChanges like these, while nice, would make more sense as a separate\npatch.\n\nHope that helps,\nJonathan\n"},{"id":"149247","messageId":"9e0261a0eebe275e2ed1fa651e5ab80c35f6048c.1283120150.git.tfransosi@gmail.com","threadId":"24901","inReplyTo":"20100829214648.GE1890@burratino","subject":"[PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-08-29T22:20:21Z","receivedAt":"2010-08-29T22:20:21Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"This improves the usage output by adding builtin_merge_recursive_usage string\nthat follows the same pattern used by the other builtin commands.\n\nThe previous output for git merger-recursive was:\nusage: merge-recursive <base>... -- <head> <remote> ...\n\nNow the output is:\nusage: git merge-recursive <base>... -- <head> <remote> ...\n\nSigned-off-by: Thiago Farina <tfransosi@gmail.com>\n---\n builtin/merge-recursive.c |   17 ++++++++++-------\n 1 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex d8875d5..189ba42 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -3,6 +3,9 @@\n #include \"tag.h\"\n #include \"merge-recursive.h\"\n \n+static const char builtin_merge_recursive_usage[] =\n+\t\"git %s <base>... -- <head> <remote> ...\";\n+\n static const char *better_branch_name(const char *branch)\n {\n \tstatic char githead_env[8 + 40 + 1];\n@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\to.subtree_shift = \"\";\n \n \tif (argc < 4)\n-\t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n+\t\tusagef(builtin_merge_recursive_usage, argv[0]);\n \n \tfor (i = 1; i < argc; ++i) {\n \t\tconst char *arg = argv[i];\n@@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--\")) {\n \t\t\tif (!arg[2])\n \t\t\t\tbreak;\n-\t\t\tif (!strcmp(arg+2, \"ours\"))\n+\t\t\tif (!strcmp(arg + 2, \"ours\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_OURS;\n-\t\t\telse if (!strcmp(arg+2, \"theirs\"))\n+\t\t\telse if (!strcmp(arg + 2, \"theirs\"))\n \t\t\t\to.recursive_variant = MERGE_RECURSIVE_THEIRS;\n-\t\t\telse if (!strcmp(arg+2, \"subtree\"))\n+\t\t\telse if (!strcmp(arg + 2, \"subtree\"))\n \t\t\t\to.subtree_shift = \"\";\n-\t\t\telse if (!prefixcmp(arg+2, \"subtree=\"))\n+\t\t\telse if (!prefixcmp(arg + 2, \"subtree=\"))\n \t\t\t\to.subtree_shift = arg + 10;\n \t\t\telse\n \t\t\t\tdie(\"Unknown option %s\", arg);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (bases_count < ARRAY_SIZE(bases)-1) {\n+\t\tif (bases_count < ARRAY_SIZE(bases) - 1) {\n \t\t\tunsigned char *sha = xmalloc(20);\n \t\t\tif (get_sha1(argv[i], sha))\n \t\t\t\tdie(\"Could not parse object '%s'\", argv[i]);\n@@ -58,7 +61,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\twarning(\"Cannot handle more than %d bases. \"\n \t\t\t\t\"Ignoring %s.\",\n-\t\t\t\t(int)ARRAY_SIZE(bases)-1, argv[i]);\n+\t\t\t\t(int)ARRAY_SIZE(bases) - 1, argv[i]);\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n-- \n1.7.2.1.95.g3d045\n"},{"id":"149248","messageId":"AANLkTimopOKiq1ef1YvbR28ArcEZ4nV+P_-DcfSKsJyO@mail.gmail.com","threadId":"24901","inReplyTo":"20100829214648.GE1890@burratino","subject":"Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-08-29T22:24:31Z","receivedAt":"2010-08-29T22:24:31Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Aug 29, 2010 at 6:46 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi Thiago,\n>\n> Thiago Farina wrote:\n>\n>> This improves the usage output by adding builtin_merge_recursive_usage string\n>> that follows the same pattern used by the other builtin commands.\n>>\n>> Also it uses usage() function instead of the usagef() function.\n>\n> FWIW cmd_merge_recursive is used to handle four different commands.\n>\n> $ git grep -F -e cmd_merge_recursive\n> builtin.h:extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\n> builtin/merge-recursive.c:int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n> git.c:          { \"merge-recursive\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n> git.c:          { \"merge-recursive-ours\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n> git.c:          { \"merge-recursive-theirs\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n> git.c:          { \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n>\n> What will be the output from\n>\n>  $ git merge-subtree -h\n>\nThanks for catching this, fixed in the v2 patch.\n"},{"id":"149275","messageId":"20100830024259.GC4010@burratino","threadId":"24901","inReplyTo":"9e0261a0eebe275e2ed1fa651e5ab80c35f6048c.1283120150.git.tfransosi@gmail.com","subject":"Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-30T02:42:59Z","receivedAt":"2010-08-30T02:42:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n\n> The previous output for git merger-recursive was:\n> usage: merge-recursive <base>... -- <head> <remote> ...\n> \n> Now the output is:\n> usage: git merge-recursive <base>... -- <head> <remote> ...\n[...]\n\n> --- a/builtin/merge-recursive.c\n> +++ b/builtin/merge-recursive.c\n> @@ -3,6 +3,9 @@\n>  #include \"tag.h\"\n>  #include \"merge-recursive.h\"\n>  \n> +static const char builtin_merge_recursive_usage[] =\n> +\t\"git %s <base>... -- <head> <remote> ...\";\n\nNow that you've researched it, wouldn't it make sense to include an\nexplanation for this %s in the log message?\n\nStill, ack on this part.\n\n> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n>  \t\tif (!prefixcmp(arg, \"--\")) {\n>  \t\t\tif (!arg[2])\n>  \t\t\t\tbreak;\n> -\t\t\tif (!strcmp(arg+2, \"ours\"))\n> +\t\t\tif (!strcmp(arg + 2, \"ours\"))\n\nMy comment on the rest still applies: it is best if patches do not\ncontain unrelated changes to unrelated parts of the files they touch.\n"},{"id":"149277","messageId":"b58adc6cdfd6a5753fac1dde3371b3745b1f58b1.1283138840.git.tfransosi@gmail.com","threadId":"24901","inReplyTo":"20100830024259.GC4010@burratino","subject":"[PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-08-30T03:30:22Z","receivedAt":"2010-08-30T03:30:22Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"This improves the usage output by adding builtin_merge_recursive_usage string\nthat follows the same pattern used by the other builtin commands.\n\nThe previous output for git merger-recursive was:\nusage: merge-recursive <base>... -- <head> <remote> ...\n\nNow the output is:\nusage: git merge-recursive <base>... -- <head> <remote> ...\n\nSince cmd_merge_recursive is used to handle four different commands we need\nthe %s in the usage string, so the following example:\n\n$ git merge-subtree -h\n\nWill output:\nusage: git merge-subtree <base>... -- <head> <remote> ...\n\nSigned-off-by: Thiago Farina <tfransosi@gmail.com>\n---\n builtin/merge-recursive.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c\nindex d8875d5..3d00adb 100644\n--- a/builtin/merge-recursive.c\n+++ b/builtin/merge-recursive.c\n@@ -3,6 +3,9 @@\n #include \"tag.h\"\n #include \"merge-recursive.h\"\n \n+static const char builtin_merge_recursive_usage[] =\n+\t\"git %s <base>... -- <head> <remote> ...\";\n+\n static const char *better_branch_name(const char *branch)\n {\n \tstatic char githead_env[8 + 40 + 1];\n@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\to.subtree_shift = \"\";\n \n \tif (argc < 4)\n-\t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n+\t\tusagef(builtin_merge_recursive_usage, argv[0]);\n \n \tfor (i = 1; i < argc; ++i) {\n \t\tconst char *arg = argv[i];\n-- \n1.7.2.1.95.g3d045\n"},{"id":"149278","messageId":"AANLkTimEtAg0kmXRtc57XsF5Hpp8wqbn8tS0bz8q8K4+@mail.gmail.com","threadId":"24901","inReplyTo":"20100830024259.GC4010@burratino","subject":"Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-08-30T03:32:58Z","receivedAt":"2010-08-30T03:32:58Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Aug 29, 2010 at 11:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Now that you've researched it, wouldn't it make sense to include an\n> explanation for this %s in the log message?\n>\nSure, added an the explanation of %s in the usage string and the example too.\n\n> Still, ack on this part.\n>\nThanks.\n\n>> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n>>               if (!prefixcmp(arg, \"--\")) {\n>>                       if (!arg[2])\n>>                               break;\n>> -                     if (!strcmp(arg+2, \"ours\"))\n>> +                     if (!strcmp(arg + 2, \"ours\"))\n>\n> My comment on the rest still applies: it is best if patches do not\n> contain unrelated changes to unrelated parts of the files they touch.\n>\nReverted these style fixes in patchset v3.\n"},{"id":"149283","messageId":"7vk4n8hc05.fsf@alter.siamese.dyndns.org","threadId":"24901","inReplyTo":"b58adc6cdfd6a5753fac1dde3371b3745b1f58b1.1283138840.git.tfransosi@gmail.com","subject":"Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-30T06:04:42Z","receivedAt":"2010-08-30T06:04:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thiago Farina <tfransosi@gmail.com> writes:\n\n> Since cmd_merge_recursive is used to handle four different commands we need\n> the %s in the usage string, so the following example:\n>\n> $ git merge-subtree -h\n>\n> Will output:\n> usage: git merge-subtree <base>... -- <head> <remote> ...\n>\n> Signed-off-by: Thiago Farina <tfransosi@gmail.com>\n\nHmm, why isn't this a single liner\n\n> -\t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n> +\t\tusagef(\"git %s <base>... -- <head> <remote> ...\", argv[0]);\n"},{"id":"149511","messageId":"20100831234226.GF6747@burratino","threadId":"24901","inReplyTo":"7vk4n8hc05.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-31T23:42:26Z","receivedAt":"2010-08-31T23:42:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> Hmm, why isn't this a single liner\n>\n>> -\t\tusagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n>> +\t\tusagef(\"git %s <base>... -- <head> <remote> ...\", argv[0]);\n\nI can't speak for Thiago, but I assume it's for unity of style with\nmore complicated commands like \"git fetch\" that include a usage\nmessage at the top of the file as:\n\n - a quick introduction for people who don't like to scroll\n\n - a variable that can be used from other functions when command-line\n   mistakes are noticed late.\n\nI would have gone for the one-liner, myself.\n"},{"id":"149516","messageId":"AANLkTimRB5-kCfp9TRnRxQnNV6dapUXmSbRDCPBW93LC@mail.gmail.com","threadId":"24901","inReplyTo":"20100831234226.GF6747@burratino","subject":"Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-09-01T02:20:08Z","receivedAt":"2010-09-01T02:20:08Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Tue, Aug 31, 2010 at 8:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Junio C Hamano wrote:\n>\n>> Hmm, why isn't this a single liner\n>>\n>>> -            usagef(\"%s <base>... -- <head> <remote> ...\", argv[0]);\n>>> +            usagef(\"git %s <base>... -- <head> <remote> ...\", argv[0]);\n>\n> I can't speak for Thiago, but I assume it's for unity of style with\n> more complicated commands like \"git fetch\" that include a usage\n> message at the top of the file as:\n>\n>  - a quick introduction for people who don't like to scroll\n>\n>  - a variable that can be used from other functions when command-line\n>   mistakes are noticed late.\n>\nPrecise explanation :-)\n\nThanks Jonathan.\n"}]}