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

Subsystems: the rest

DORMANTno replies

7 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Thiago Farina <hidden>
Date: 2016-06-15 22:49:24 

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 <redacted>
---
 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

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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:24 

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?
quoted hunk
--- 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

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

From: Thiago Farina <hidden>
Date: 2016-06-15 22:49:24 

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 <redacted>
---
 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

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

From: Thiago Farina <hidden>
Date: 2016-06-15 22:49:24 

On Sun, Aug 29, 2010 at 6:46 PM, Jonathan Nieder [off-list ref] wrote:
Hi Thiago,

Thiago Farina wrote:
quoted
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.

Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:49:24 

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> ...
[...]
quoted hunk
--- 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.
quoted hunk
@@ -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.

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

From: Thiago Farina <hidden>
Date: 2016-06-15 22:49:24 

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 <redacted>
---
 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

Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.

From: Thiago Farina <hidden>
Date: 2016-06-15 22:49:24 

On Sun, Aug 29, 2010 at 11:42 PM, Jonathan Nieder [off-list ref] 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.
quoted
@@ -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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help