{"thread":{"id":"8276","subject":"[PATCH] revert/cherry-pick: allow the last parameter to be -h","startedAt":"2007-05-22T21:29:45Z","lastAt":"2007-05-23T07:04:13Z","messageCount":7,"participants":["Jonas Fonseca","Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"42992","messageId":"20070522212945.GA8002@diku.dk","threadId":"8276","inReplyTo":null,"subject":"[PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-05-22T21:29:45Z","receivedAt":"2007-05-22T21:29:45Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"... to ask for the usage string.\n\nSigned-off-by: Jonas Fonseca \n---\n builtin-revert.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex ea2f15b..7984aeb 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -61,6 +61,8 @@ static void parse_options(int argc, const char **argv)\n \t}\n \n \targ = argv[argc - 1];\n+\tif (!strcmp(arg, \"-h\"))\n+\t\tusage(usage_str);\n \tif (get_sha1(arg, sha1))\n \t\tdie (\"Cannot find '%s'\", arg);\n \tcommit = (struct commit *)parse_object(sha1);\n-- \n1.5.2.rc3.800.ga489e-dirty\n\n-- \nJonas Fonseca\n"},{"id":"43001","messageId":"20070522221156.GL30871@steel.home","threadId":"8276","inReplyTo":"20070522212945.GA8002@diku.dk","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-05-22T22:11:56Z","receivedAt":"2007-05-22T22:11:56Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Jonas Fonseca, Tue, May 22, 2007 23:29:45 +0200:\n> +\tif (!strcmp(arg, \"-h\"))\n> +\t\tusage(usage_str);\n\n $ git rev-list --usage\n usage: git-rev-list [OPTION] ... [ -- paths... ]\n limiting output:\n\t--max-count=nr\n\t--max-age=epoch\n ...\n\nWhy should cherry-pick be different?\n"},{"id":"43003","messageId":"7vwsz07b10.fsf@assigned-by-dhcp.cox.net","threadId":"8276","inReplyTo":"20070522221156.GL30871@steel.home","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-22T22:23:39Z","receivedAt":"2007-05-22T22:23:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> Jonas Fonseca, Tue, May 22, 2007 23:29:45 +0200:\n>> +\tif (!strcmp(arg, \"-h\"))\n>> +\t\tusage(usage_str);\n>\n> $ git rev-list --usage\n> usage: git-rev-list [OPTION] ... [ -- paths... ]\n> limiting output:\n> \t--max-count=nr\n> \t--max-age=epoch\n> ...\n>\n> Why should cherry-pick be different?\n\nGood question. FYI\n\n\t$ git rev-list --huh?\n\nworks equally well ;-)\n"},{"id":"43017","messageId":"20070523053110.GA23971@diku.dk","threadId":"8276","inReplyTo":"7vwsz07b10.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-05-23T05:31:11Z","receivedAt":"2007-05-23T05:31:11Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Junio C Hamano wrote Tue, May 22, 2007:\n> Alex Riesen writes:\n> \n> > Jonas Fonseca, Tue, May 22, 2007 23:29:45 +0200:\n> >> +\tif (!strcmp(arg, \"-h\"))\n> >> +\t\tusage(usage_str);\n> >\n> > $ git rev-list --usage\n> > usage: git-rev-list [OPTION] ... [ -- paths... ]\n> > limiting output:\n> > \t--max-count=nr\n> > \t--max-age=epoch\n> > ...\n> >\n> > Why should cherry-pick be different?\n> \n> Good question. FYI\n> \n> \t$ git rev-list --huh?\n> \n> works equally well ;-)\n\nBecause it is different?\n\n $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes\n fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'\n\nBecause, contrary to git-rev-list, git-revert/cherry-pick is considered\npart of the porcelain? Because asking that question to every small UI\nimprovement is not very useful?\n\nAnd yes I could spell out '--help', but it just seems weird that I need\nto know the calling convention of git-revert (_and_ git-cherry-pick for\nthat matter) in order to get the usage string, because that was what I\nwanted to know in the first place. Anyway, if you don't like it, please\njust drop the patch. :)\n\n-- \nJonas Fonseca\n"},{"id":"43018","messageId":"7v8xbg5bno.fsf@assigned-by-dhcp.cox.net","threadId":"8276","inReplyTo":"20070523053110.GA23971@diku.dk","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-23T05:52:59Z","receivedAt":"2007-05-23T05:52:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonas Fonseca writes:\n\n> Junio C Hamano wrote Tue, May 22, 2007:\n>> Alex Riesen writes:\n>> ...\n>> > Why should cherry-pick be different?\n>> \n>> Good question. FYI\n>> \n>> \t$ git rev-list --huh?\n>> \n>> works equally well ;-)\n>\n> Because it is different?\n>\n> $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes\n> fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'\n>\n> Because, contrary to git-rev-list, git-revert/cherry-pick is considered\n> part of the porcelain?\n\nNo, I did not notice it until now but you are right. The\ncommand line argument parser for these commands is done somewhat\nsloppily, compared to others.\n\nHow about doing something like this instead?\n\n-- >8 --\nFix command line parameter parser of revert/cherry-pick\n\nThe parser was inconsistently done, in that it did not look at\nthe last command line parameter to see if it could be an unknown\noption, although it was designed to notice unknown options if\nthey were given in positions the command expects to find them\n(i.e. everything except the last parameter, which ought to be\n). This prevented a very natural invocation\n\n\t$ git cherry-pick --help\n\nfrom issuing the usage help.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex ea2f15b..80c348c 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -45,8 +45,10 @@ static void parse_options(int argc, const char **argv)\n \tif (argc < 2)\n \t\tusage(usage_str);\n \n-\tfor (i = 1; i < argc - 1; i++) {\n+\tfor (i = 1; i < argc; i++) {\n \t\targ = argv[i];\n+\t\tif (arg[0] != '-')\n+\t\t\tbreak;\n \t\tif (!strcmp(arg, \"-n\") || !strcmp(arg, \"--no-commit\"))\n \t\t\tno_commit = 1;\n \t\telse if (!strcmp(arg, \"-e\") || !strcmp(arg, \"--edit\"))\n@@ -59,7 +61,8 @@ static void parse_options(int argc, const char **argv)\n \t\telse if (strcmp(arg, \"-r\"))\n \t\t\tusage(usage_str);\n \t}\n-\n+\tif (i != argc - 1)\n+\t\tusage(usage_str);\n \targ = argv[argc - 1];\n \tif (get_sha1(arg, sha1))\n \t\tdie (\"Cannot find '%s'\", arg);\n"},{"id":"43020","messageId":"20070523065750.GA25931@diku.dk","threadId":"8276","inReplyTo":"7v8xbg5bno.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-05-23T06:57:50Z","receivedAt":"2007-05-23T06:57:50Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Junio C Hamano wrote Tue, May 22, 2007:\n> Jonas Fonseca writes:\n> \n> > Junio C Hamano wrote Tue, May 22, 2007:\n> >> Alex Riesen writes:\n> >> ...\n> >> > Why should cherry-pick be different?\n> >> \n> >> Good question. FYI\n> >> \n> >> \t$ git rev-list --huh?\n> >> \n> >> works equally well ;-)\n> >\n> > Because it is different?\n> >\n> > $ git revert --why-must-it-be-so-hard-to-learn-git-sometimes\n> > fatal: Cannot find '--why-must-it-be-so-hard-to-learn-git-sometimes'\n> >\n> > Because, contrary to git-rev-list, git-revert/cherry-pick is considered\n> > part of the porcelain?\n> \n> No, I did not notice it until now but you are right. The\n> command line argument parser for these commands is done somewhat\n> sloppily, compared to others.\n> \n> How about doing something like this instead?\n\nFWIW, I like it. Sorry for my quick and dirty patch.\n\n> -- >8 --\n> Fix command line parameter parser of revert/cherry-pick\n> \n> The parser was inconsistently done, [...]in that it did not look at\n> the last command line parameter to see if it could be an unknown\n> option, although it was designed to notice unknown options if\n> they were given in positions the command expects to find them\n> (i.e. everything except the last parameter, which ought to be\n> ). This prevented a very natural invocation\n> \n> \t$ git cherry-pick --help\n> \n> from issuing the usage help.\n\nBut --help is handled elsewhere, you meant -h ...\n\n-- \nJonas Fonseca\n"},{"id":"43021","messageId":"7vsl9o3tsi.fsf@assigned-by-dhcp.cox.net","threadId":"8276","inReplyTo":"20070523065750.GA25931@diku.dk","subject":"Re: [PATCH] revert/cherry-pick: allow the last parameter to be -h","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-23T07:04:13Z","receivedAt":"2007-05-23T07:04:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonas Fonseca writes:\n\n> But --help is handled elsewhere, you meant -h ...\n\nQuite true. I meant --usage.\n"}]}