[PATCH] git.c: make autocorrected aliases work

[PATCH] git.c: make autocorrected aliases work

[Adeodato Simó]

help_unknown_cmd() is able to autocorrect a command to an alias, and not
only to internal or external commands. However, main() was not passing the
autocorrected command through handle_alias(), hence it failed if it was an
alias.

This commit makes the autocorrected command go through handle_alias, once
handle_internal_command() and execv_dashed_external() have been tried. Since
this is done twice in main() now, moved that logic to a new run_argv()
function.

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---
 git.c |   46 +++++++++++++++++++++++++++-------------------
 1 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/git.c b/git.c
index e0d9071..f443b4c 100644
--- a/git.c
+++ b/git.c
@@ -416,12 +416,35 @@ static void execv_dashed_external(const char **argv)
 	strbuf_release(&cmd);
 }
 
+static int run_argv(int *argcp, const char ***argv)
+{
+	int done_alias = 0;
+
+	while (1) {
+		/* See if it's an internal command */
+		handle_internal_command(*argcp, *argv);
+
+		/* .. then try the external ones */
+		execv_dashed_external(*argv);
+
+		/* It could be an alias -- this works around the insanity
+		 * of overriding "git log" with "git show" by having
+		 * alias.log = show
+		 */
+		if (done_alias || !handle_alias(argcp, argv))
+			break;
+		done_alias = 1;
+	}
+
+	return done_alias;
+}
+
 
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
 	char *slash = (char *)cmd + strlen(cmd);
-	int done_alias = 0;
+	int was_alias = 0;
 
 	/*
 	 * Take the basename of argv[0] as the command
@@ -478,32 +501,17 @@ int main(int argc, const char **argv)
 	 */
 	setup_path();
 
-	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
-		execv_dashed_external(argv);
-
-		/* It could be an alias -- this works around the insanity
-		 * of overriding "git log" with "git show" by having
-		 * alias.log = show
-		 */
-		if (done_alias || !handle_alias(&argc, &argv))
-			break;
-		done_alias = 1;
-	}
+	was_alias = run_argv(&argc, &argv);
 
 	if (errno == ENOENT) {
-		if (done_alias) {
+		if (was_alias) {
 			fprintf(stderr, "Expansion of alias '%s' failed; "
 				"'%s' is not a git-command\n",
 				cmd, argv[0]);
 			exit(1);
 		}
 		argv[0] = help_unknown_cmd(cmd);
-		handle_internal_command(argc, argv);
-		execv_dashed_external(argv);
+		run_argv(&argc, &argv);
 	}
 
 	fprintf(stderr, "Failed to run command '%s': %s\n",
-- 
1.6.1.62.g677ca

[PATCH v2] git.c: make autocorrected aliases work

[Adeodato Simó]

help_unknown_cmd() is able to autocorrect a command to an alias, and not
only to internal or external commands. However, main() was not passing the
autocorrected command through handle_alias(), hence it failed if it was an
alias.

This commit makes the autocorrected command go through handle_alias(), once
handle_internal_command() and execv_dashed_external() have been tried. Since
this is done twice in main() now, moved that logic to a new run_argv()
function.

Also, print the same "Expansion of alias 'x' failed" message when the alias
was autocorrected, rather than a generic "Failed to run command 'x'".

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---

Here's a version of the patch that improves the error reporting, in case
this is desired. With the previous patch 'aliasx' -> 'alias' -> 'enoent'
printed "Failed to run command 'aliasx'", now it correctly prints
"Expansion of alias 'alias' failed: 'enoent' is not a git-command".

This is the incremental diff:

diff -u b/git.c b/git.c
--- b/git.c
+++ b/git.c
@@ -444,7 +444,6 @@
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
 	char *slash = (char *)cmd + strlen(cmd);
-	int was_alias = 0;
 
 	/*
 	 * Take the basename of argv[0] as the command
@@ -501,17 +500,23 @@
 	 */
 	setup_path();
 
-	was_alias = run_argv(&argc, &argv);
-
-	if (errno == ENOENT) {
+	while (1) {
+		static int done_help = 0;
+		static int was_alias = 0;
+		was_alias = run_argv(&argc, &argv);
+		if (errno != ENOENT)
+			break;
 		if (was_alias) {
 			fprintf(stderr, "Expansion of alias '%s' failed; "
 				"'%s' is not a git-command\n",
 				cmd, argv[0]);
 			exit(1);
 		}
-		argv[0] = help_unknown_cmd(cmd);
-		run_argv(&argc, &argv);
+		if (!done_help) {
+			cmd = argv[0] = help_unknown_cmd(cmd);
+			done_help = 1;
+		} else
+			break;
 	}
 
 	fprintf(stderr, "Failed to run command '%s': %s\n",

 git.c |   53 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/git.c b/git.c
index e0d9071..ee331aa 100644
--- a/git.c
+++ b/git.c
@@ -416,12 +416,34 @@ static void execv_dashed_external(const char **argv)
 	strbuf_release(&cmd);
 }
 
+static int run_argv(int *argcp, const char ***argv)
+{
+	int done_alias = 0;
+
+	while (1) {
+		/* See if it's an internal command */
+		handle_internal_command(*argcp, *argv);
+
+		/* .. then try the external ones */
+		execv_dashed_external(*argv);
+
+		/* It could be an alias -- this works around the insanity
+		 * of overriding "git log" with "git show" by having
+		 * alias.log = show
+		 */
+		if (done_alias || !handle_alias(argcp, argv))
+			break;
+		done_alias = 1;
+	}
+
+	return done_alias;
+}
+
 
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
 	char *slash = (char *)cmd + strlen(cmd);
-	int done_alias = 0;
 
 	/*
 	 * Take the basename of argv[0] as the command
@@ -479,31 +501,22 @@ int main(int argc, const char **argv)
 	setup_path();
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
-		execv_dashed_external(argv);
-
-		/* It could be an alias -- this works around the insanity
-		 * of overriding "git log" with "git show" by having
-		 * alias.log = show
-		 */
-		if (done_alias || !handle_alias(&argc, &argv))
+		static int done_help = 0;
+		static int was_alias = 0;
+		was_alias = run_argv(&argc, &argv);
+		if (errno != ENOENT)
 			break;
-		done_alias = 1;
-	}
-
-	if (errno == ENOENT) {
-		if (done_alias) {
+		if (was_alias) {
 			fprintf(stderr, "Expansion of alias '%s' failed; "
 				"'%s' is not a git-command\n",
 				cmd, argv[0]);
 			exit(1);
 		}
-		argv[0] = help_unknown_cmd(cmd);
-		handle_internal_command(argc, argv);
-		execv_dashed_external(argv);
+		if (!done_help) {
+			cmd = argv[0] = help_unknown_cmd(cmd);
+			done_help = 1;
+		} else
+			break;
 	}
 
 	fprintf(stderr, "Failed to run command '%s': %s\n",
-- 
1.6.1.62.g677ca

[PATCH v2 resend] git.c: make autocorrected aliases work

[Adeodato Simó]

help_unknown_cmd() is able to autocorrect a command to an alias, and not
only to internal or external commands. However, main() was not passing the
autocorrected command through handle_alias(), hence it failed if it was an
alias.

This commit makes the autocorrected command go through handle_alias(), once
handle_internal_command() and execv_dashed_external() have been tried. Since
this is done twice in main() now, moved that logic to a new run_argv()
function.

Also, print the same "Expansion of alias 'x' failed" message when the alias
was autocorrected, rather than a generic "Failed to run command 'x'".

Signed-off-by: Adeodato Simó <dato@net.com.org.es>
---

Meh, I didn't realize that by attaching an incremental diff, I'd break
`git am`. Sorry about that.

 git.c |   53 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/git.c b/git.c
index e0d9071..ee331aa 100644
--- a/git.c
+++ b/git.c
@@ -416,12 +416,34 @@ static void execv_dashed_external(const char **argv)
 	strbuf_release(&cmd);
 }
 
+static int run_argv(int *argcp, const char ***argv)
+{
+	int done_alias = 0;
+
+	while (1) {
+		/* See if it's an internal command */
+		handle_internal_command(*argcp, *argv);
+
+		/* .. then try the external ones */
+		execv_dashed_external(*argv);
+
+		/* It could be an alias -- this works around the insanity
+		 * of overriding "git log" with "git show" by having
+		 * alias.log = show
+		 */
+		if (done_alias || !handle_alias(argcp, argv))
+			break;
+		done_alias = 1;
+	}
+
+	return done_alias;
+}
+
 
 int main(int argc, const char **argv)
 {
 	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
 	char *slash = (char *)cmd + strlen(cmd);
-	int done_alias = 0;
 
 	/*
 	 * Take the basename of argv[0] as the command
@@ -479,31 +501,22 @@ int main(int argc, const char **argv)
 	setup_path();
 
 	while (1) {
-		/* See if it's an internal command */
-		handle_internal_command(argc, argv);
-
-		/* .. then try the external ones */
-		execv_dashed_external(argv);
-
-		/* It could be an alias -- this works around the insanity
-		 * of overriding "git log" with "git show" by having
-		 * alias.log = show
-		 */
-		if (done_alias || !handle_alias(&argc, &argv))
+		static int done_help = 0;
+		static int was_alias = 0;
+		was_alias = run_argv(&argc, &argv);
+		if (errno != ENOENT)
 			break;
-		done_alias = 1;
-	}
-
-	if (errno == ENOENT) {
-		if (done_alias) {
+		if (was_alias) {
 			fprintf(stderr, "Expansion of alias '%s' failed; "
 				"'%s' is not a git-command\n",
 				cmd, argv[0]);
 			exit(1);
 		}
-		argv[0] = help_unknown_cmd(cmd);
-		handle_internal_command(argc, argv);
-		execv_dashed_external(argv);
+		if (!done_help) {
+			cmd = argv[0] = help_unknown_cmd(cmd);
+			done_help = 1;
+		} else
+			break;
 	}
 
 	fprintf(stderr, "Failed to run command '%s': %s\n",
-- 
1.6.1.62.g677ca

Re: [PATCH v2 resend] git.c: make autocorrected aliases work

[Alexander Potashev]

On 18:16 Sun 04 Jan     , Adeodato Simó wrote:
> help_unknown_cmd() is able to autocorrect a command to an alias, and not
> only to internal or external commands. However, main() was not passing the
> autocorrected command through handle_alias(), hence it failed if it was an
> alias.
> 
> This commit makes the autocorrected command go through handle_alias(), once
> handle_internal_command() and execv_dashed_external() have been tried. Since
> this is done twice in main() now, moved that logic to a new run_argv()
> function.
> 
> Also, print the same "Expansion of alias 'x' failed" message when the alias
> was autocorrected, rather than a generic "Failed to run command 'x'".
> 
> Signed-off-by: Adeodato Simó <dato@net.com.org.es>
> ---
> 
> Meh, I didn't realize that by attaching an incremental diff, I'd break
> `git am`. Sorry about that.
> 
>  git.c |   53 +++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/git.c b/git.c
> index e0d9071..ee331aa 100644
> --- a/git.c
> +++ b/git.c
> @@ -416,12 +416,34 @@ static void execv_dashed_external(const char **argv)
>  	strbuf_release(&cmd);
>  }
>  
> +static int run_argv(int *argcp, const char ***argv)
> +{
> +	int done_alias = 0;
> +
> +	while (1) {
> +		/* See if it's an internal command */
> +		handle_internal_command(*argcp, *argv);
> +
> +		/* .. then try the external ones */
> +		execv_dashed_external(*argv);
> +
> +		/* It could be an alias -- this works around the insanity
> +		 * of overriding "git log" with "git show" by having
> +		 * alias.log = show
> +		 */
> +		if (done_alias || !handle_alias(argcp, argv))
> +			break;
> +		done_alias = 1;
> +	}
> +
> +	return done_alias;
> +}
> +
>  
>  int main(int argc, const char **argv)
>  {
>  	const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
>  	char *slash = (char *)cmd + strlen(cmd);
> -	int done_alias = 0;
>  
>  	/*
>  	 * Take the basename of argv[0] as the command
> @@ -479,31 +501,22 @@ int main(int argc, const char **argv)
>  	setup_path();
>  
>  	while (1) {
> -		/* See if it's an internal command */
> -		handle_internal_command(argc, argv);
> -
> -		/* .. then try the external ones */
> -		execv_dashed_external(argv);
> -
> -		/* It could be an alias -- this works around the insanity
> -		 * of overriding "git log" with "git show" by having
> -		 * alias.log = show
> -		 */
> -		if (done_alias || !handle_alias(&argc, &argv))
> +		static int done_help = 0;
> +		static int was_alias = 0;
> +		was_alias = run_argv(&argc, &argv);
> +		if (errno != ENOENT)
>  			break;
> -		done_alias = 1;
> -	}
> -
> -	if (errno == ENOENT) {
> -		if (done_alias) {
> +		if (was_alias) {
>  			fprintf(stderr, "Expansion of alias '%s' failed; "
>  				"'%s' is not a git-command\n",
>  				cmd, argv[0]);
>  			exit(1);

Why not using 'die' here?

		die("Expansion of alias '%s' failed;
			'%s' is not a git-command",
  			cmd, argv[0]);

DISCLAIMER: I have never used git's 'die'

>  		}
> -		argv[0] = help_unknown_cmd(cmd);
> -		handle_internal_command(argc, argv);
> -		execv_dashed_external(argv);
> +		if (!done_help) {
> +			cmd = argv[0] = help_unknown_cmd(cmd);
> +			done_help = 1;
> +		} else
> +			break;
>  	}
>  
>  	fprintf(stderr, "Failed to run command '%s': %s\n",
> -- 
> 1.6.1.62.g677ca

Re: [PATCH v2 resend] git.c: make autocorrected aliases work

[Junio C Hamano]

Alexander Potashev <aspotashev@gmail.com> writes:

>> @@ -479,31 +501,22 @@ int main(int argc, const char **argv)
>> ...
>> +		was_alias = run_argv(&argc, &argv);
>> +		if (errno != ENOENT)
>>  			break;
>> +		if (was_alias) {
>>  			fprintf(stderr, "Expansion of alias '%s' failed; "
>>  				"'%s' is not a git-command\n",
>>  				cmd, argv[0]);
>>  			exit(1);
>
> Why not using 'die' here?

The code is in the context, and I do not think it is a good idea to
conflate such a change to a patch that wants to add aliases auto
correction.

While I do not think it matters too much in practice (unless existing
scripts that runs git depends on the exact error status value), there are
two differences: the message will say "fatal: " in front, and the command
exits with 128 not with 1.

Re: [PATCH v2 resend] git.c: make autocorrected aliases work

[Junio C Hamano]

Thanks, queued.