Re: Weird behavior of shell variables in git aliases

[Jeff King <peff@peff.net>]

On Tue, Mar 22, 2011 at 07:18:44AM -0400, Jeff King wrote:

> Doesn't it also break a lot of other more garden-variety aliases that
> rely on the automagic "$@", like:
> 
>   $ git config alias.log-nopager
>   !git --no-pager log
> [...]
>   $ GIT_TRACE=1 git.master log-nopager --oneline
>   trace: exec: 'git-log-nopager' '--oneline'
>   trace: run_command: 'git-log-nopager' '--oneline'
>   trace: run_command: 'git --no-pager log' '--oneline'
>   trace: exec: 'sh' '-c' 'git --no-pager log "$@"' 'git --no-pager log' '--oneline'
>   trace: built-in: git 'log' '--oneline'
>   93c7d44 foo
> 
>   $ GIT_TRACE=1 git.jch.shell-alias log-nopager --oneline
>   trace: exec: 'git-log-nopager' '--oneline'
>   trace: run_command: 'git-log-nopager' '--oneline'
>   trace: run_command: 'sh' '-c' 'git --no-pager log' '-' '--oneline'
>   trace: exec: 'sh' '-c' 'git --no-pager log' '-' '--oneline'
>   trace: built-in: git 'log'
>   commit 93c7d44635e8bb56a4fd864d024ce75a2ad4ffcf
>   Author: Jeff King <peff@peff.net>
>   Date:   Tue Mar 22 06:57:02 2011 -0400
> 
>       foo

One other solution would be to make the "$@" more magic by detecting
when the alias uses positional parameters and omitting it in that
case.  Something like (on top of your patch):

diff --git a/git.c b/git.c
index 8d54466..1daf89c 100644
--- a/git.c
+++ b/git.c
@@ -161,6 +161,27 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 	return handled;
 }
 
+static int alias_uses_positional_parameters(const char *s)
+{
+	while ((s = strchr(s, '$')))
+		if (s[1] == '@' || s[1] == '#' ||
+		   (s[1] >= '1' && s[1] <= '9'))
+			return 1;
+	return 0;
+}
+
+static char *alias_to_shell(const char *in)
+{
+	struct strbuf out = STRBUF_INIT;
+
+	if (alias_uses_positional_parameters(in))
+		return xstrdup(in);
+
+	strbuf_addstr(&out, in);
+	strbuf_addstr(&out, " \"$@\"");
+	return strbuf_detach(&out, NULL);
+}
+
 static int handle_alias(int *argcp, const char ***argv)
 {
 	int envchanged = 0, ret = 0, saved_errno = errno;
@@ -186,7 +207,7 @@ static int handle_alias(int *argcp, const char ***argv)
 			alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 4));
 			alias_argv[0] = "sh";
 			alias_argv[1] = "-c";
-			alias_argv[2] = alias_string + 1;
+			alias_argv[2] = alias_to_shell(alias_string + 1);
 			alias_argv[3] = "-";
 			for (i = 1; i < argc; ++i)
 				alias_argv[i + 3] = (*argv)[i];


But I think that is a little too magic for my taste. Although the false
positives ("!echo 'literal $#'") and false negatives (you want "!foo" to
_ignore_ its parameters) are pretty obscure, I would prefer to keep
things simple.

-Peff