# [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.

10 messages from 2010-08-29 to 2010-09-01. Participants: Thiago Farina, Jonathan Nieder, Junio C Hamano.
Thread: https://gitlist.dev/t/24901

## Thiago Farina, 2010-08-29 17:39

Subject: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
Message-ID: <c9d88eb8cbd5b025b0c0112be05d3c0fe993de25.1283103426.git.tfransosi@gmail.com>
URL: https://gitlist.dev/e/c9d88eb8cbd5b025b0c0112be05d3c0fe993de25.1283103426.git.tfransosi%40gmail.com

```
This improves the usage output by adding builtin_merge_recursive_usage string
that follows the same pattern used by the other builtin commands.

Also it uses usage() function instead of the usagef() function.

The previous output for git merger-recursive was:
usage: merge-recursive <base>... -- <head> <remote> ...

Now the output is:
usage: git merge-recursive <base>... -- <head> <remote> ...

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/merge-recursive.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index d8875d5..46c2a85 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -3,6 +3,9 @@
 #include "tag.h"
 #include "merge-recursive.h"
 
+static const char builtin_merge_recursive_usage[] =
+	"git merge-recursive <base>... -- <head> <remote> ...";
+
 static const char *better_branch_name(const char *branch)
 {
 	static char githead_env[8 + 40 + 1];
@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		o.subtree_shift = "";
 
 	if (argc < 4)
-		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
+		usage(builtin_merge_recursive_usage);
 
 	for (i = 1; i < argc; ++i) {
 		const char *arg = argv[i];
@@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		if (!prefixcmp(arg, "--")) {
 			if (!arg[2])
 				break;
-			if (!strcmp(arg+2, "ours"))
+			if (!strcmp(arg + 2, "ours"))
 				o.recursive_variant = MERGE_RECURSIVE_OURS;
-			else if (!strcmp(arg+2, "theirs"))
+			else if (!strcmp(arg + 2, "theirs"))
 				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
-			else if (!strcmp(arg+2, "subtree"))
+			else if (!strcmp(arg + 2, "subtree"))
 				o.subtree_shift = "";
-			else if (!prefixcmp(arg+2, "subtree="))
+			else if (!prefixcmp(arg + 2, "subtree="))
 				o.subtree_shift = arg + 10;
 			else
 				die("Unknown option %s", arg);
 			continue;
 		}
-		if (bases_count < ARRAY_SIZE(bases)-1) {
+		if (bases_count < ARRAY_SIZE(bases) - 1) {
 			unsigned char *sha = xmalloc(20);
 			if (get_sha1(argv[i], sha))
 				die("Could not parse object '%s'", argv[i]);
@@ -58,7 +61,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		else
 			warning("Cannot handle more than %d bases. "
 				"Ignoring %s.",
-				(int)ARRAY_SIZE(bases)-1, argv[i]);
+				(int)ARRAY_SIZE(bases) - 1, argv[i]);
 	}
 	if (argc - i != 3) /* "--" "<head>" "<remote>" */
 		die("Not handling anything other than two heads merge.");
-- 
1.7.2.1.95.g3d045

```

## Jonathan Nieder, 2010-08-29 21:46

Subject: Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
Message-ID: <20100829214648.GE1890@burratino>
URL: https://gitlist.dev/e/20100829214648.GE1890%40burratino
In-Reply-To: <c9d88eb8cbd5b025b0c0112be05d3c0fe993de25.1283103426.git.tfransosi@gmail.com>

```
Hi Thiago,

Thiago Farina wrote:

> This improves the usage output by adding builtin_merge_recursive_usage string
> that follows the same pattern used by the other builtin commands.
> 
> Also it uses usage() function instead of the usagef() function.

FWIW cmd_merge_recursive is used to handle four different commands.

$ git grep -F -e cmd_merge_recursive
builtin.h:extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);
builtin/merge-recursive.c:int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
git.c:          { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
git.c:          { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
git.c:          { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
git.c:          { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },

What will be the output from

 $ git merge-subtree -h

after this change?

> --- a/builtin/merge-recursive.c
> +++ b/builtin/merge-recursive.c
> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
>  		if (!prefixcmp(arg, "--")) {
>  			if (!arg[2])
>  				break;
> -			if (!strcmp(arg+2, "ours"))
> +			if (!strcmp(arg + 2, "ours"))

Changes like these, while nice, would make more sense as a separate
patch.

Hope that helps,
Jonathan

```

## Thiago Farina, 2010-08-29 22:20

Subject: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <9e0261a0eebe275e2ed1fa651e5ab80c35f6048c.1283120150.git.tfransosi@gmail.com>
URL: https://gitlist.dev/e/9e0261a0eebe275e2ed1fa651e5ab80c35f6048c.1283120150.git.tfransosi%40gmail.com
In-Reply-To: <20100829214648.GE1890@burratino>

```
This improves the usage output by adding builtin_merge_recursive_usage string
that follows the same pattern used by the other builtin commands.

The previous output for git merger-recursive was:
usage: merge-recursive <base>... -- <head> <remote> ...

Now the output is:
usage: git merge-recursive <base>... -- <head> <remote> ...

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/merge-recursive.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index d8875d5..189ba42 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -3,6 +3,9 @@
 #include "tag.h"
 #include "merge-recursive.h"
 
+static const char builtin_merge_recursive_usage[] =
+	"git %s <base>... -- <head> <remote> ...";
+
 static const char *better_branch_name(const char *branch)
 {
 	static char githead_env[8 + 40 + 1];
@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		o.subtree_shift = "";
 
 	if (argc < 4)
-		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
+		usagef(builtin_merge_recursive_usage, argv[0]);
 
 	for (i = 1; i < argc; ++i) {
 		const char *arg = argv[i];
@@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		if (!prefixcmp(arg, "--")) {
 			if (!arg[2])
 				break;
-			if (!strcmp(arg+2, "ours"))
+			if (!strcmp(arg + 2, "ours"))
 				o.recursive_variant = MERGE_RECURSIVE_OURS;
-			else if (!strcmp(arg+2, "theirs"))
+			else if (!strcmp(arg + 2, "theirs"))
 				o.recursive_variant = MERGE_RECURSIVE_THEIRS;
-			else if (!strcmp(arg+2, "subtree"))
+			else if (!strcmp(arg + 2, "subtree"))
 				o.subtree_shift = "";
-			else if (!prefixcmp(arg+2, "subtree="))
+			else if (!prefixcmp(arg + 2, "subtree="))
 				o.subtree_shift = arg + 10;
 			else
 				die("Unknown option %s", arg);
 			continue;
 		}
-		if (bases_count < ARRAY_SIZE(bases)-1) {
+		if (bases_count < ARRAY_SIZE(bases) - 1) {
 			unsigned char *sha = xmalloc(20);
 			if (get_sha1(argv[i], sha))
 				die("Could not parse object '%s'", argv[i]);
@@ -58,7 +61,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		else
 			warning("Cannot handle more than %d bases. "
 				"Ignoring %s.",
-				(int)ARRAY_SIZE(bases)-1, argv[i]);
+				(int)ARRAY_SIZE(bases) - 1, argv[i]);
 	}
 	if (argc - i != 3) /* "--" "<head>" "<remote>" */
 		die("Not handling anything other than two heads merge.");
-- 
1.7.2.1.95.g3d045

```

## Thiago Farina, 2010-08-29 22:24

Subject: Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
Message-ID: <AANLkTimopOKiq1ef1YvbR28ArcEZ4nV+P_-DcfSKsJyO@mail.gmail.com>
URL: https://gitlist.dev/e/AANLkTimopOKiq1ef1YvbR28ArcEZ4nV%2BP_-DcfSKsJyO%40mail.gmail.com
In-Reply-To: <20100829214648.GE1890@burratino>

```
On Sun, Aug 29, 2010 at 6:46 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Thiago,
>
> Thiago Farina wrote:
>
>> This improves the usage output by adding builtin_merge_recursive_usage string
>> that follows the same pattern used by the other builtin commands.
>>
>> Also it uses usage() function instead of the usagef() function.
>
> FWIW cmd_merge_recursive is used to handle four different commands.
>
> $ git grep -F -e cmd_merge_recursive
> builtin.h:extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);
> builtin/merge-recursive.c:int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
> git.c:          { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
> git.c:          { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
> git.c:          { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
> git.c:          { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
>
> What will be the output from
>
>  $ git merge-subtree -h
>
Thanks for catching this, fixed in the v2 patch.

```

## Jonathan Nieder, 2010-08-30 02:42

Subject: Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <20100830024259.GC4010@burratino>
URL: https://gitlist.dev/e/20100830024259.GC4010%40burratino
In-Reply-To: <9e0261a0eebe275e2ed1fa651e5ab80c35f6048c.1283120150.git.tfransosi@gmail.com>

```
Thiago Farina wrote:

> The previous output for git merger-recursive was:
> usage: merge-recursive <base>... -- <head> <remote> ...
> 
> Now the output is:
> usage: git merge-recursive <base>... -- <head> <remote> ...
[...]

> --- a/builtin/merge-recursive.c
> +++ b/builtin/merge-recursive.c
> @@ -3,6 +3,9 @@
>  #include "tag.h"
>  #include "merge-recursive.h"
>  
> +static const char builtin_merge_recursive_usage[] =
> +	"git %s <base>... -- <head> <remote> ...";

Now that you've researched it, wouldn't it make sense to include an
explanation for this %s in the log message?

Still, ack on this part.

> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
>  		if (!prefixcmp(arg, "--")) {
>  			if (!arg[2])
>  				break;
> -			if (!strcmp(arg+2, "ours"))
> +			if (!strcmp(arg + 2, "ours"))

My comment on the rest still applies: it is best if patches do not
contain unrelated changes to unrelated parts of the files they touch.

```

## Thiago Farina, 2010-08-30 03:30

Subject: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <b58adc6cdfd6a5753fac1dde3371b3745b1f58b1.1283138840.git.tfransosi@gmail.com>
URL: https://gitlist.dev/e/b58adc6cdfd6a5753fac1dde3371b3745b1f58b1.1283138840.git.tfransosi%40gmail.com
In-Reply-To: <20100830024259.GC4010@burratino>

```
This improves the usage output by adding builtin_merge_recursive_usage string
that follows the same pattern used by the other builtin commands.

The previous output for git merger-recursive was:
usage: merge-recursive <base>... -- <head> <remote> ...

Now the output is:
usage: git merge-recursive <base>... -- <head> <remote> ...

Since cmd_merge_recursive is used to handle four different commands we need
the %s in the usage string, so the following example:

$ git merge-subtree -h

Will output:
usage: git merge-subtree <base>... -- <head> <remote> ...

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/merge-recursive.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index d8875d5..3d00adb 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -3,6 +3,9 @@
 #include "tag.h"
 #include "merge-recursive.h"
 
+static const char builtin_merge_recursive_usage[] =
+	"git %s <base>... -- <head> <remote> ...";
+
 static const char *better_branch_name(const char *branch)
 {
 	static char githead_env[8 + 40 + 1];
@@ -29,7 +32,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
 		o.subtree_shift = "";
 
 	if (argc < 4)
-		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
+		usagef(builtin_merge_recursive_usage, argv[0]);
 
 	for (i = 1; i < argc; ++i) {
 		const char *arg = argv[i];
-- 
1.7.2.1.95.g3d045

```

## Thiago Farina, 2010-08-30 03:32

Subject: Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <AANLkTimEtAg0kmXRtc57XsF5Hpp8wqbn8tS0bz8q8K4+@mail.gmail.com>
URL: https://gitlist.dev/e/AANLkTimEtAg0kmXRtc57XsF5Hpp8wqbn8tS0bz8q8K4%2B%40mail.gmail.com
In-Reply-To: <20100830024259.GC4010@burratino>

```
On Sun, Aug 29, 2010 at 11:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Now that you've researched it, wouldn't it make sense to include an
> explanation for this %s in the log message?
>
Sure, added an the explanation of %s in the usage string and the example too.

> Still, ack on this part.
>
Thanks.

>> @@ -37,19 +40,19 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
>>               if (!prefixcmp(arg, "--")) {
>>                       if (!arg[2])
>>                               break;
>> -                     if (!strcmp(arg+2, "ours"))
>> +                     if (!strcmp(arg + 2, "ours"))
>
> My comment on the rest still applies: it is best if patches do not
> contain unrelated changes to unrelated parts of the files they touch.
>
Reverted these style fixes in patchset v3.

```

## Junio C Hamano, 2010-08-30 06:04

Subject: Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <7vk4n8hc05.fsf@alter.siamese.dyndns.org>
URL: https://gitlist.dev/e/7vk4n8hc05.fsf%40alter.siamese.dyndns.org
In-Reply-To: <b58adc6cdfd6a5753fac1dde3371b3745b1f58b1.1283138840.git.tfransosi@gmail.com>

```
Thiago Farina <tfransosi@gmail.com> writes:

> Since cmd_merge_recursive is used to handle four different commands we need
> the %s in the usage string, so the following example:
>
> $ git merge-subtree -h
>
> Will output:
> usage: git merge-subtree <base>... -- <head> <remote> ...
>
> Signed-off-by: Thiago Farina <tfransosi@gmail.com>

Hmm, why isn't this a single liner

> -		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
> +		usagef("git %s <base>... -- <head> <remote> ...", argv[0]);

```

## Jonathan Nieder, 2010-08-31 23:42

Subject: Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <20100831234226.GF6747@burratino>
URL: https://gitlist.dev/e/20100831234226.GF6747%40burratino
In-Reply-To: <7vk4n8hc05.fsf@alter.siamese.dyndns.org>

```
Junio C Hamano wrote:

> Hmm, why isn't this a single liner
>
>> -		usagef("%s <base>... -- <head> <remote> ...", argv[0]);
>> +		usagef("git %s <base>... -- <head> <remote> ...", argv[0]);

I can't speak for Thiago, but I assume it's for unity of style with
more complicated commands like "git fetch" that include a usage
message at the top of the file as:

 - a quick introduction for people who don't like to scroll

 - a variable that can be used from other functions when command-line
   mistakes are noticed late.

I would have gone for the one-liner, myself.

```

## Thiago Farina, 2010-09-01 02:20

Subject: Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
Message-ID: <AANLkTimRB5-kCfp9TRnRxQnNV6dapUXmSbRDCPBW93LC@mail.gmail.com>
URL: https://gitlist.dev/e/AANLkTimRB5-kCfp9TRnRxQnNV6dapUXmSbRDCPBW93LC%40mail.gmail.com
In-Reply-To: <20100831234226.GF6747@burratino>

```
On Tue, Aug 31, 2010 at 8:42 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>
>> Hmm, why isn't this a single liner
>>
>>> -            usagef("%s <base>... -- <head> <remote> ...", argv[0]);
>>> +            usagef("git %s <base>... -- <head> <remote> ...", argv[0]);
>
> I can't speak for Thiago, but I assume it's for unity of style with
> more complicated commands like "git fetch" that include a usage
> message at the top of the file as:
>
>  - a quick introduction for people who don't like to scroll
>
>  - a variable that can be used from other functions when command-line
>   mistakes are noticed late.
>
Precise explanation :-)

Thanks Jonathan.

```