* [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
@ 2010-08-29 17:39 Thiago Farina
2010-08-29 21:46 ` Jonathan Nieder
0 siblings, 1 reply; 10+ messages in thread
From: Thiago Farina @ 2010-08-29 17:39 UTC (permalink / raw)
To: git; +Cc: gitster
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
2010-08-29 17:39 [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it Thiago Farina
@ 2010-08-29 21:46 ` Jonathan Nieder
2010-08-29 22:20 ` [PATCH v2] builtin/merge_recursive.c: Add an " Thiago Farina
2010-08-29 22:24 ` [PATCH] builtin/merge_recursive.c: Add a " Thiago Farina
0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Nieder @ 2010-08-29 21:46 UTC (permalink / raw)
To: Thiago Farina; +Cc: git, gitster
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-29 21:46 ` Jonathan Nieder
@ 2010-08-29 22:20 ` Thiago Farina
2010-08-30 2:42 ` Jonathan Nieder
2010-08-29 22:24 ` [PATCH] builtin/merge_recursive.c: Add a " Thiago Farina
1 sibling, 1 reply; 10+ messages in thread
From: Thiago Farina @ 2010-08-29 22:20 UTC (permalink / raw)
To: git; +Cc: gitster, jrnieder
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it.
2010-08-29 21:46 ` Jonathan Nieder
2010-08-29 22:20 ` [PATCH v2] builtin/merge_recursive.c: Add an " Thiago Farina
@ 2010-08-29 22:24 ` Thiago Farina
1 sibling, 0 replies; 10+ messages in thread
From: Thiago Farina @ 2010-08-29 22:24 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-29 22:20 ` [PATCH v2] builtin/merge_recursive.c: Add an " Thiago Farina
@ 2010-08-30 2:42 ` Jonathan Nieder
2010-08-30 3:30 ` [PATCH v3] " Thiago Farina
2010-08-30 3:32 ` [PATCH v2] " Thiago Farina
0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Nieder @ 2010-08-30 2:42 UTC (permalink / raw)
To: Thiago Farina; +Cc: git, gitster
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-30 2:42 ` Jonathan Nieder
@ 2010-08-30 3:30 ` Thiago Farina
2010-08-30 6:04 ` Junio C Hamano
2010-08-30 3:32 ` [PATCH v2] " Thiago Farina
1 sibling, 1 reply; 10+ messages in thread
From: Thiago Farina @ 2010-08-30 3:30 UTC (permalink / raw)
To: git; +Cc: jrnieder, gitster
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-30 2:42 ` Jonathan Nieder
2010-08-30 3:30 ` [PATCH v3] " Thiago Farina
@ 2010-08-30 3:32 ` Thiago Farina
1 sibling, 0 replies; 10+ messages in thread
From: Thiago Farina @ 2010-08-30 3:32 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-30 3:30 ` [PATCH v3] " Thiago Farina
@ 2010-08-30 6:04 ` Junio C Hamano
2010-08-31 23:42 ` Jonathan Nieder
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-08-30 6:04 UTC (permalink / raw)
To: Thiago Farina; +Cc: git, jrnieder, gitster
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]);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-30 6:04 ` Junio C Hamano
@ 2010-08-31 23:42 ` Jonathan Nieder
2010-09-01 2:20 ` Thiago Farina
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2010-08-31 23:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Thiago Farina, git
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] builtin/merge_recursive.c: Add an usage string and make use of it.
2010-08-31 23:42 ` Jonathan Nieder
@ 2010-09-01 2:20 ` Thiago Farina
0 siblings, 0 replies; 10+ messages in thread
From: Thiago Farina @ 2010-09-01 2:20 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-09-01 2:20 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-29 17:39 [PATCH] builtin/merge_recursive.c: Add a usage string and make use of it Thiago Farina
2010-08-29 21:46 ` Jonathan Nieder
2010-08-29 22:20 ` [PATCH v2] builtin/merge_recursive.c: Add an " Thiago Farina
2010-08-30 2:42 ` Jonathan Nieder
2010-08-30 3:30 ` [PATCH v3] " Thiago Farina
2010-08-30 6:04 ` Junio C Hamano
2010-08-31 23:42 ` Jonathan Nieder
2010-09-01 2:20 ` Thiago Farina
2010-08-30 3:32 ` [PATCH v2] " Thiago Farina
2010-08-29 22:24 ` [PATCH] builtin/merge_recursive.c: Add a " Thiago Farina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.