git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h

From
Junio C Hamano <junkio@cox.net>
Date
May 23, 2007, 05:52 UTC
Message-ID
<7v8xbg5bno.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<20070523053110.GA23971@diku.dk>
Jonas Fonseca <fonseca@diku.dk> writes:
Show 18 quoted lines
> Junio C Hamano <junkio@cox.net> wrote Tue, May 22, 2007:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>> ...
>> > 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?

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 <junkio@cox.net>
---
diff --git a/builtin-revert.c b/builtin-revert.c
index ea2f15b..80c348c 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -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);
Previous: Jonas FonsecaNext: Jonas Fonseca
Message 5 of 7 in “revert/cherry-pick: allow the last parameter to be -h”
  1. revert/cherry-pick: allow the last parameter to be -hJonas Fonseca, May 22, 2007
  2. Alex RiesenMay 22, 2007
  3. Junio C HamanoMay 22, 2007
  4. Jonas FonsecaMay 23, 2007
  5. Junio C HamanoMay 23, 2007
  6. Jonas FonsecaMay 23, 2007
  7. Junio C HamanoMay 23, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.