revert/cherry-pick: allow the last parameter to be -h

PATCH In review Jonas Fonseca 3 participants 7 messages Jun 15, 2016

lore

Share & tools

lore

Jonas Fonseca

author

2016-06-15 22:43 UTC #0 lore raw

PATCH 1/1 revert/cherry-pick: allow the last parameter to be -h

... to ask for the usage string.

Signed-off-by Jonas Fonseca <[email protected]>

---
 builtin-revert.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

builtin-revert.c +2 -0

index ea2f15b..7984aeb 100644
@@ -61,6 +61,8 @@ static void parse_options(int argc, const char **argv)
 	}
 
 	arg = argv[argc - 1];
+	if (!strcmp(arg, "-h"))
+		usage(usage_str);
 	if (get_sha1(arg, sha1))
 		die ("Cannot find '%s'", arg);
 	commit = (struct commit *)parse_object(sha1);

Signature · 4 lines

1.5.2.rc3.800.ga489e-dirty

-- 
Jonas Fonseca

Jonas Fonseca

author

2016-06-15 22:43 UTC #3 lore raw

Junio C Hamano <[email protected]> wrote Tue, May 22, 2007:

Quoted 13 lines

Quoted 11 lines · Alex Riesen
Jonas Fonseca, Tue, May 22, 2007 23:29:45 +0200:
builtin-revert.c static void parse_options(int argc, const char **argv) · line 65
Quoted 2 lines
builtin-revert.c +2 -0
+	if (!strcmp(arg, "-h"))
+		usage(usage_str);
    $ git rev-list --usage
    usage: git-rev-list [OPTION] <commit-id>... [ -- paths... ]
      limiting output:
	--max-count=nr
	--max-age=epoch
    ...

Quoted 7 lines

Quoted 1 line
Why should cherry-pick be different?
Good question. FYI
	$ git rev-list --huh?
works equally well ;-)

Because it is different?

   $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
   fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'

Because, contrary to git-rev-list, git-revert/cherry-pick is considered part of the porcelain? Because asking that question to every small UI improvement is not very useful?

And yes I could spell out '--help', but it just seems weird that I need to know the calling convention of git-revert (_and_ git-cherry-pick for that matter) in order to get the usage string, because that was what I wanted to know in the first place. Anyway, if you don't like it, please just drop the patch. :)

Signature · 1 line

Jonas Fonseca

Junio C Hamano

2016-06-15 22:43 UTC #4 lore raw

Quoted 11 lines · Jonas Fonseca

Junio C Hamano <[email protected]> wrote Tue, May 22, 2007:
Quoted 9 lines
Alex Riesen <[email protected]> writes:
...
Quoted 1 line
Why should cherry-pick be different?
Good question. FYI
	$ git rev-list --huh?
works equally well ;-)

Quoted 7 lines

Because it is different?
   $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
   fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'
Because, contrary to git-rev-list, git-revert/cherry-pick is considered part of the porcelain?

No, I did not notice it until now but you are right. The command line argument parser for these commands is done somewhat sloppily, compared to others.

How about doing something like this instead?

-- >8 --
Fix command line parameter parser of revert/cherry-pick

The parser was inconsistently done, in that it did not look at the last command line parameter to see if it could be an unknown option, although it was designed to notice unknown options if they were given in positions the command expects to find them (i.e. everything except the last parameter, which ought to be <commit-ish>). This prevented a very natural invocation

	$ git cherry-pick --help

from issuing the usage help.

Signed-off-by Junio C Hamano <[email protected]>

---

builtin-revert.c +5 -2

index ea2f15b..80c348c 100644
@@ -45,8 +45,10 @@ static void parse_options(int argc, const char **argv)
 	if (argc < 2)
 		usage(usage_str);
 
-	for (i = 1; i < argc - 1; i++) {
+	for (i = 1; i < argc; i++) {
 		arg = argv[i];
+		if (arg[0] != '-')
+			break;
 		if (!strcmp(arg, "-n") || !strcmp(arg, "--no-commit"))
 			no_commit = 1;
 		else if (!strcmp(arg, "-e") || !strcmp(arg, "--edit"))

@@ -59,7 +61,8 @@ static void parse_options(int argc, const char **argv)
 		else if (strcmp(arg, "-r"))
 			usage(usage_str);
 	}
-
+	if (i != argc - 1)
+		usage(usage_str);
 	arg = argv[argc - 1];
 	if (get_sha1(arg, sha1))
 		die ("Cannot find '%s'", arg);

Jonas Fonseca

author

2016-06-15 22:43 UTC #5 lore raw

Junio C Hamano <[email protected]> wrote Tue, May 22, 2007:

Quoted 18 lines

Quoted 16 lines · Jonas Fonseca
Junio C Hamano <[email protected]> wrote Tue, May 22, 2007:
Quoted 9 lines
Alex Riesen <[email protected]> writes:
...
Quoted 1 line
Why should cherry-pick be different?
Good question. FYI
	$ git rev-list --huh?
works equally well ;-)
Because it is different?
   $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes
   fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'

Quoted 8 lines

Quoted 2 lines
Because, contrary to git-rev-list, git-revert/cherry-pick is considered part of the porcelain?
No, I did not notice it until now but you are right. The command line argument parser for these commands is done somewhat sloppily, compared to others.
How about doing something like this instead?

FWIW, I like it. Sorry for my quick and dirty patch.

Quoted 10 lines

-- >8 --
Fix command line parameter parser of revert/cherry-pick
The parser was inconsistently done, [...]in that it did not look at the last command line parameter to see if it could be an unknown option, although it was designed to notice unknown options if they were given in positions the command expects to find them (i.e. everything except the last parameter, which ought to be <commit-ish>). This prevented a very natural invocation

Quoted 3 lines

	$ git cherry-pick --help
from issuing the usage help.

But --help is handled elsewhere, you meant -h ...

Signature · 1 line

Jonas Fonseca

Junio C Hamano

2016-06-15 22:43 UTC #6 lore raw

Quoted 1 line · Jonas Fonseca

But --help is handled elsewhere, you meant -h ...

Quite true. I meant --usage.