[PATCH] Check for -amend as a common wrong usage of --amend.

[PATCH] Check for -amend as a common wrong usage of --amend.

[Pascal Obry]

It happens from time to time to type -amend (with a single
dash) when --amend is meant. In those case there is no mistake
and git commit all files modified with the log message set
to "end". As -amend is just doing something stupid it is
better to check for this wrong usage and give hint to the
user about the possible mistake.

Signed-off-by: Pascal Obry <pascal@obry.net>
---
 parse-options.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 7a08a0c..248515d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -233,6 +233,13 @@ int parse_options(int argc, const char **argv, const struct option *options,
 			continue;
 		}
 
+		if (!strcmp(arg + 1, "amend")) {
+		        error("-amend looks suspicious, don't you meant --amend\n");
+		        args.argc--;
+		        args.argv++;
+		        break;
+		}
+
 		if (arg[1] != '-') {
 			args.opt = arg + 1;
 			do {
-- 
1.5.4.rc4.23.gcab31

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Pascal Obry]

Typing too fast I've just made this mistake the third time today. It is 
of course easy to revert but a check seems appropriate here.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Johannes Schindelin]

Hi,

On Thu, 24 Jan 2008, Pascal Obry wrote:

> diff --git a/parse-options.c b/parse-options.c
> index 7a08a0c..248515d 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -233,6 +233,13 @@ int parse_options(int argc, const char **argv, const struct option *options,
>  			continue;
>  		}
>  
> +		if (!strcmp(arg + 1, "amend")) {
> +		        error("-amend looks suspicious, don't you meant --amend\n");
> +		        args.argc--;
> +		        args.argv++;
> +		        break;
> +		}
> +
>  		if (arg[1] != '-') {
>  			args.opt = arg + 1;
>  			do {

That is ugly.  In a source file which is by no means specific to 
git-commit, you cannot possibly mean to check for "amend".

I don't like it,
Dscho

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Pascal Obry]

Johannes Schindelin a écrit :
> That is ugly.  In a source file which is by no means specific to 
> git-commit, you cannot possibly mean to check for "amend".

Agreed :( I'll try to come with something better.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Charles Bailey]

On Thu, Jan 24, 2008 at 07:52:26PM +0100, Pascal Obry wrote:
> Johannes Schindelin a écrit :
> >That is ugly.  In a source file which is by no means specific to 
> >git-commit, you cannot possibly mean to check for "amend".
> 
> Agreed :( I'll try to come with something better.
> 
> Pascal.
> 

Would this be better handled by a commit-msg hook.  E.g.:

test "$(cat $1)" = "end" && {
    echo >&2 Commit message is \"end\", possible mis-type of --amend
    echo >&2 Use --no-verify to really commit with this commit message
	exit 1
}

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Pascal Obry]

Charles Bailey a écrit :
> Would this be better handled by a commit-msg hook.  E.g.:

I do not agree. Why check this late as this option is boggus? And 
furthermore I do not want to have to install this commit message hook in 
all my Git repositories.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Joey Hess]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Pascal Obry wrote:
> Johannes Schindelin a écrit :
>> That is ugly.  In a source file which is by no means specific to  
>> git-commit, you cannot possibly mean to check for "amend".
>
> Agreed :( I'll try to come with something better.

Some option parsers avoid this sort of ambiguity by not allowing short
options that take a string to be bundled in the same word with other
short options.

So, for example, git-commit -am<msg> would not be allowed, while
git-commit -a -m<msg> and perhaps git-commit -am <msg> would be allowed.

There could still be problems if there were a --mend option that could
be typoed as -mend.

I don't know enough about compatability to say if this would work for git.

-- 
see shy jo
<relurk>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Jörg Sommer]

Hi Pascal,

Pascal Obry <pascal@obry.net> wrote:
> Typing too fast I've just made this mistake the third time today. It is 
> of course easy to revert but a check seems appropriate here.

Why not use an alias?

% git config --get alias.cia
commit --amend

Bye, Jörg.
-- 
Two types have compatible type if their types are the same.
[ANSI C, 6.2.7]

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Junio C Hamano]

Joey Hess <joey@kitenet.net> writes:

> Some option parsers avoid this sort of ambiguity by not allowing short
> options that take a string to be bundled in the same word with other
> short options.
>
> So, for example, git-commit -am<msg> would not be allowed, while
> git-commit -a -m<msg> and perhaps git-commit -am <msg> would be allowed.
>
> There could still be problems if there were a --mend option that could
> be typoed as -mend.
>
> I don't know enough about compatability to say if this would work for git.

Yeah, I think that is quite a sensible workaround.

Re: [PATCH] Check for -amend as a common wrong usage of --amend.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Sat, Jan 26, 2008 at 06:20:41AM +0000, Junio C Hamano wrote:
> Joey Hess <joey@kitenet.net> writes:
> 
> > Some option parsers avoid this sort of ambiguity by not allowing short
> > options that take a string to be bundled in the same word with other
> > short options.
> >
> > So, for example, git-commit -am<msg> would not be allowed, while
> > git-commit -a -m<msg> and perhaps git-commit -am <msg> would be allowed.
> >
> > There could still be problems if there were a --mend option that could
> > be typoed as -mend.
> >
> > I don't know enough about compatability to say if this would work for git.
> 
> Yeah, I think that is quite a sensible workaround.

  I agree, I think that we should refuse things where the string after a
/one/ dash starts with 3 or more consecutive characters that are also
the beginning of a long option. I think that 2 is usually a bit "short"
to assume that it's a typo. I'll provide a patch soon

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] parse-options: catch some likely in presense of aggregated options.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 4006 bytes --]

If options are aggregated, and that the whole token looks like (is the exact
prefix of length >= 3 of) a long option, then parse_options rejects it.

The typo check isn't performed if there is no aggregation, because the stuck
for is the recommended one, hence if we have `-o` being a valid short option
that takes an argument, and --option a long one, then we _MUST_ accept
-option as it is our official recommended form.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c          |   30 ++++++++++++++++++++++++++++--
 t/t0040-parse-options.sh |   11 +++++++++++
 test-parse-options.c     |    1 +
 3 files changed, 40 insertions(+), 2 deletions(-)

    On Sat, Jan 26, 2008 at 10:42:16AM +0000, Pierre Habouzit wrote:
    >   I agree, I think that we should refuse things where the string after a
    > /one/ dash starts with 3 or more consecutive characters that are also
    > the beginning of a long option. I think that 2 is usually a bit "short"
    > to assume that it's a typo. I'll provide a patch soon

    Here it is, and we have now:

      $ git commit -amend
      error: did you mean `--amend` (with two dashes ?)


diff --git a/parse-options.c b/parse-options.c
index 7a08a0c..d9562ba 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -216,6 +216,26 @@ is_abbreviated:
 	return error("unknown option `%s'", arg);
 }
 
+void check_typos(const char *arg, const struct option *options)
+{
+	if (strlen(arg) < 3)
+		return;
+
+	if (!prefixcmp(arg, "no-")) {
+		error ("did you mean `--%s` (with two dashes ?)", arg);
+		exit(129);
+	}
+
+	for (; options->type != OPTION_END; options++) {
+		if (!options->long_name)
+			continue;
+		if (!prefixcmp(options->long_name, arg)) {
+			error ("did you mean `--%s` (with two dashes ?)", arg);
+			exit(129);
+		}
+	}
+}
+
 static NORETURN void usage_with_options_internal(const char * const *,
                                                  const struct option *, int);
 
@@ -235,12 +255,18 @@ int parse_options(int argc, const char **argv, const struct option *options,
 
 		if (arg[1] != '-') {
 			args.opt = arg + 1;
-			do {
+			if (*args.opt == 'h')
+				usage_with_options(usagestr, options);
+			if (parse_short_opt(&args, options) < 0)
+				usage_with_options(usagestr, options);
+			if (args.opt)
+				check_typos(arg + 1, options);
+			while (args.opt) {
 				if (*args.opt == 'h')
 					usage_with_options(usagestr, options);
 				if (parse_short_opt(&args, options) < 0)
 					usage_with_options(usagestr, options);
-			} while (args.opt);
+			}
 			continue;
 		}
 
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 462fdf2..0a3b55d 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -19,6 +19,7 @@ string options
                           get a string
     --string2 <str>       get another string
     --st <st>             get another string (pervert ordering)
+    -o <str>              get another string
 
 EOF
 
@@ -103,4 +104,14 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
 	git diff expect output
 '
 
+cat > expect.err << EOF
+error: did you mean \`--boolean\` (with two dashes ?)
+EOF
+
+test_expect_success 'detect possible typos' '
+	! test-parse-options -boolean > output 2> output.err &&
+	test ! -s output &&
+	git diff expect.err output.err
+'
+
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 4d3e2ec..eed8a02 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -19,6 +19,7 @@ int main(int argc, const char **argv)
 		OPT_STRING('s', "string", &string, "string", "get a string"),
 		OPT_STRING(0, "string2", &string, "str", "get another string"),
 		OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"),
+		OPT_STRING('o', NULL, &string, "str", "get another string"),
 		OPT_END(),
 	};
 	int i;
-- 
1.5.4.rc4.24.g5232a


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]