{"thread":{"id":"11722","subject":"[PATCH] Check for -amend as a common wrong usage of --amend.","startedAt":"2008-01-24T18:13:59Z","lastAt":"2008-01-26T11:26:57Z","messageCount":11,"participants":["Pascal Obry","Johannes Schindelin","Charles Bailey","Joey Hess","Junio C Hamano","Jörg Sommer","Pierre Habouzit"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"66497","messageId":"1201198439-3516-1-git-send-email-pascal@obry.net","threadId":"11722","inReplyTo":null,"subject":"[PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Pascal Obry","fromEmail":"pascal.obry@gmail.com","sentAt":"2008-01-24T18:13:59Z","receivedAt":"2008-01-24T18:13:59Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"It happens from time to time to type -amend (with a single\ndash) when --amend is meant. In those case there is no mistake\nand git commit all files modified with the log message set\nto \"end\". As -amend is just doing something stupid it is\nbetter to check for this wrong usage and give hint to the\nuser about the possible mistake.\n\nSigned-off-by: Pascal Obry \n---\n parse-options.c | 7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 7a08a0c..248515d 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -233,6 +233,13 @@ int parse_options(int argc, const char **argv, const struct option *options,\n \t\t\tcontinue;\n \t\t}\n \n+\t\tif (!strcmp(arg + 1, \"amend\")) {\n+\t\t error(\"-amend looks suspicious, don't you meant --amend\\n\");\n+\t\t args.argc--;\n+\t\t args.argv++;\n+\t\t break;\n+\t\t}\n+\n \t\tif (arg[1] != '-') {\n \t\t\targs.opt = arg + 1;\n \t\t\tdo {\n-- \n1.5.4.rc4.23.gcab31\n"},{"id":"66498","messageId":"4798D5E7.8070907@obry.net","threadId":"11722","inReplyTo":"1201198439-3516-1-git-send-email-pascal@obry.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2008-01-24T18:16:07Z","receivedAt":"2008-01-24T18:16:07Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"\nTyping too fast I've just made this mistake the third time today. It is \nof course easy to revert but a check seems appropriate here.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"66501","messageId":"alpine.LSU.1.00.0801241818441.5731@racer.site","threadId":"11722","inReplyTo":"1201198439-3516-1-git-send-email-pascal@obry.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-24T18:20:00Z","receivedAt":"2008-01-24T18:20:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 24 Jan 2008, Pascal Obry wrote:\n\n> diff --git a/parse-options.c b/parse-options.c\n> index 7a08a0c..248515d 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -233,6 +233,13 @@ int parse_options(int argc, const char **argv, const struct option *options,\n> \t\t\tcontinue;\n> \t\t}\n> \n> +\t\tif (!strcmp(arg + 1, \"amend\")) {\n> +\t\t error(\"-amend looks suspicious, don't you meant --amend\\n\");\n> +\t\t args.argc--;\n> +\t\t args.argv++;\n> +\t\t break;\n> +\t\t}\n> +\n> \t\tif (arg[1] != '-') {\n> \t\t\targs.opt = arg + 1;\n> \t\t\tdo {\n\nThat is ugly. In a source file which is by no means specific to \ngit-commit, you cannot possibly mean to check for \"amend\".\n\nI don't like it,\nDscho\n"},{"id":"66506","messageId":"4798DE6A.1050201@obry.net","threadId":"11722","inReplyTo":"alpine.LSU.1.00.0801241818441.5731@racer.site","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2008-01-24T18:52:26Z","receivedAt":"2008-01-24T18:52:26Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Johannes Schindelin a écrit :\n> That is ugly. In a source file which is by no means specific to \n> git-commit, you cannot possibly mean to check for \"amend\".\n\nAgreed :( I'll try to come with something better.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"66511","messageId":"20080124192532.GA3389@hashpling.org","threadId":"11722","inReplyTo":"4798DE6A.1050201@obry.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2008-01-24T19:25:32Z","receivedAt":"2008-01-24T19:25:32Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Thu, Jan 24, 2008 at 07:52:26PM +0100, Pascal Obry wrote:\n> Johannes Schindelin a écrit :\n> >That is ugly. In a source file which is by no means specific to \n> >git-commit, you cannot possibly mean to check for \"amend\".\n> \n> Agreed :( I'll try to come with something better.\n> \n> Pascal.\n> \n\nWould this be better handled by a commit-msg hook. E.g.:\n\ntest \"$(cat $1)\" = \"end\" && {\n echo >&2 Commit message is \\\"end\\\", possible mis-type of --amend\n echo >&2 Use --no-verify to really commit with this commit message\n\texit 1\n}\n"},{"id":"66512","messageId":"4798E87D.8030701@obry.net","threadId":"11722","inReplyTo":"20080124192532.GA3389@hashpling.org","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Pascal Obry","fromEmail":"pascal@obry.net","sentAt":"2008-01-24T19:35:25Z","receivedAt":"2008-01-24T19:35:25Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Charles Bailey a écrit :\n> Would this be better handled by a commit-msg hook. E.g.:\n\nI do not agree. Why check this late as this option is boggus? And \nfurthermore I do not want to have to install this commit message hook in \nall my Git repositories.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"66518","messageId":"20080124204711.GC17765@kodama.kitenet.net","threadId":"11722","inReplyTo":"4798DE6A.1050201@obry.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Joey Hess","fromEmail":"joey@kitenet.net","sentAt":"2008-01-24T20:47:11Z","receivedAt":"2008-01-24T20:47:11Z","isPatch":true,"sender":{"key":"joey@kitenet.net","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Pascal Obry wrote:\n> Johannes Schindelin a écrit :\n>> That is ugly. In a source file which is by no means specific to \n>> git-commit, you cannot possibly mean to check for \"amend\".\n>\n> Agreed :( I'll try to come with something better.\n\nSome option parsers avoid this sort of ambiguity by not allowing short\noptions that take a string to be bundled in the same word with other\nshort options.\n\nSo, for example, git-commit -am would not be allowed, while\ngit-commit -a -m and perhaps git-commit -am would be allowed.\n\nThere could still be problems if there were a --mend option that could\nbe typoed as -mend.\n\nI don't know enough about compatability to say if this would work for git.\n\n-- \nsee shy jo\n\n"},{"id":"66630","messageId":"slrnfpkujd.all.joerg@alea.gnuu.de","threadId":"11722","inReplyTo":"4798D5E7.8070907@obry.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Jörg Sommer","fromEmail":"joerg@alea.gnuu.de","sentAt":"2008-01-26T00:10:21Z","receivedAt":"2008-01-26T00:10:21Z","isPatch":true,"sender":{"key":"joerg@alea.gnuu.de","avatar":null},"body":"Hi Pascal,\n\nPascal Obry wrote:\n> Typing too fast I've just made this mistake the third time today. It is \n> of course easy to revert but a check seems appropriate here.\n\nWhy not use an alias?\n\n% git config --get alias.cia\ncommit --amend\n\nBye, Jörg.\n-- \nTwo types have compatible type if their types are the same.\n[ANSI C, 6.2.7]\n"},{"id":"66620","messageId":"7vd4rp3y6e.fsf@gitster.siamese.dyndns.org","threadId":"11722","inReplyTo":"20080124204711.GC17765@kodama.kitenet.net","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-26T06:20:41Z","receivedAt":"2008-01-26T06:20:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> Some option parsers avoid this sort of ambiguity by not allowing short\n> options that take a string to be bundled in the same word with other\n> short options.\n>\n> So, for example, git-commit -am would not be allowed, while\n> git-commit -a -m and perhaps git-commit -am would be allowed.\n>\n> There could still be problems if there were a --mend option that could\n> be typoed as -mend.\n>\n> I don't know enough about compatability to say if this would work for git.\n\nYeah, I think that is quite a sensible workaround.\n"},{"id":"66631","messageId":"20080126104216.GA13922@artemis.madism.org","threadId":"11722","inReplyTo":"7vd4rp3y6e.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Check for -amend as a common wrong usage of --amend.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-01-26T10:42:16Z","receivedAt":"2008-01-26T10:42:16Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Jan 26, 2008 at 06:20:41AM +0000, Junio C Hamano wrote:\n> Joey Hess writes:\n> \n> > Some option parsers avoid this sort of ambiguity by not allowing short\n> > options that take a string to be bundled in the same word with other\n> > short options.\n> >\n> > So, for example, git-commit -am would not be allowed, while\n> > git-commit -a -m and perhaps git-commit -am would be allowed.\n> >\n> > There could still be problems if there were a --mend option that could\n> > be typoed as -mend.\n> >\n> > I don't know enough about compatability to say if this would work for git.\n> \n> Yeah, I think that is quite a sensible workaround.\n\n I agree, I think that we should refuse things where the string after a\n/one/ dash starts with 3 or more consecutive characters that are also\nthe beginning of a long option. I think that 2 is usually a bit \"short\"\nto assume that it's a typo. I'll provide a patch soon\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"66635","messageId":"20080126112657.GB13922@artemis.madism.org","threadId":"11722","inReplyTo":"20080126104216.GA13922@artemis.madism.org","subject":"[PATCH] parse-options: catch some likely in presense of aggregated options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-01-26T11:26:57Z","receivedAt":"2008-01-26T11:26:57Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"If options are aggregated, and that the whole token looks like (is the exact\nprefix of length >= 3 of) a long option, then parse_options rejects it.\n\nThe typo check isn't performed if there is no aggregation, because the stuck\nfor is the recommended one, hence if we have `-o` being a valid short option\nthat takes an argument, and --option a long one, then we _MUST_ accept\n-option as it is our official recommended form.\n\nSigned-off-by: Pierre Habouzit \n---\n parse-options.c | 30 ++++++++++++++++++++++++++++--\n t/t0040-parse-options.sh | 11 +++++++++++\n test-parse-options.c | 1 +\n 3 files changed, 40 insertions(+), 2 deletions(-)\n\n On Sat, Jan 26, 2008 at 10:42:16AM +0000, Pierre Habouzit wrote:\n > I agree, I think that we should refuse things where the string after a\n > /one/ dash starts with 3 or more consecutive characters that are also\n > the beginning of a long option. I think that 2 is usually a bit \"short\"\n > to assume that it's a typo. I'll provide a patch soon\n\n Here it is, and we have now:\n\n $ git commit -amend\n error: did you mean `--amend` (with two dashes ?)\n\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 7a08a0c..d9562ba 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -216,6 +216,26 @@ is_abbreviated:\n \treturn error(\"unknown option `%s'\", arg);\n }\n \n+void check_typos(const char *arg, const struct option *options)\n+{\n+\tif (strlen(arg) < 3)\n+\t\treturn;\n+\n+\tif (!prefixcmp(arg, \"no-\")) {\n+\t\terror (\"did you mean `--%s` (with two dashes ?)\", arg);\n+\t\texit(129);\n+\t}\n+\n+\tfor (; options->type != OPTION_END; options++) {\n+\t\tif (!options->long_name)\n+\t\t\tcontinue;\n+\t\tif (!prefixcmp(options->long_name, arg)) {\n+\t\t\terror (\"did you mean `--%s` (with two dashes ?)\", arg);\n+\t\t\texit(129);\n+\t\t}\n+\t}\n+}\n+\n static NORETURN void usage_with_options_internal(const char * const *,\n const struct option *, int);\n \n@@ -235,12 +255,18 @@ int parse_options(int argc, const char **argv, const struct option *options,\n \n \t\tif (arg[1] != '-') {\n \t\t\targs.opt = arg + 1;\n-\t\t\tdo {\n+\t\t\tif (*args.opt == 'h')\n+\t\t\t\tusage_with_options(usagestr, options);\n+\t\t\tif (parse_short_opt(&args, options) < 0)\n+\t\t\t\tusage_with_options(usagestr, options);\n+\t\t\tif (args.opt)\n+\t\t\t\tcheck_typos(arg + 1, options);\n+\t\t\twhile (args.opt) {\n \t\t\t\tif (*args.opt == 'h')\n \t\t\t\t\tusage_with_options(usagestr, options);\n \t\t\t\tif (parse_short_opt(&args, options) < 0)\n \t\t\t\t\tusage_with_options(usagestr, options);\n-\t\t\t} while (args.opt);\n+\t\t\t}\n \t\t\tcontinue;\n \t\t}\n \ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex 462fdf2..0a3b55d 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -19,6 +19,7 @@ string options\n get a string\n --string2 get another string\n --st get another string (pervert ordering)\n+ -o get another string\n \n EOF\n \n@@ -103,4 +104,14 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '\n \tgit diff expect output\n '\n \n+cat > expect.err << EOF\n+error: did you mean \\`--boolean\\` (with two dashes ?)\n+EOF\n+\n+test_expect_success 'detect possible typos' '\n+\t! test-parse-options -boolean > output 2> output.err &&\n+\ttest ! -s output &&\n+\tgit diff expect.err output.err\n+'\n+\n test_done\ndiff --git a/test-parse-options.c b/test-parse-options.c\nindex 4d3e2ec..eed8a02 100644\n--- a/test-parse-options.c\n+++ b/test-parse-options.c\n@@ -19,6 +19,7 @@ int main(int argc, const char **argv)\n \t\tOPT_STRING('s', \"string\", &string, \"string\", \"get a string\"),\n \t\tOPT_STRING(0, \"string2\", &string, \"str\", \"get another string\"),\n \t\tOPT_STRING(0, \"st\", &string, \"st\", \"get another string (pervert ordering)\"),\n+\t\tOPT_STRING('o', NULL, &string, \"str\", \"get another string\"),\n \t\tOPT_END(),\n \t};\n \tint i;\n-- \n1.5.4.rc4.24.g5232a\n\n"}]}