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@xxxxxxxx>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html