Weird behavior of shell variables in git aliases

Weird behavior of shell variables in git aliases

[Dun Peal]

It seems that a variable is available only once?!  How can the
following shell session be explained:

    $ git config alias.test0
    !echo $1
    $ git test0 foo
    foo
    $ git config alias.test1
    !echo $1 && echo $1
    $ git test1 foo

    foo
    $ git config alias.test2
    !BRANCH=$1 && echo $BRANCH && echo $BRANCH
    $ git test2 foo

    foo

Thanks, D.

Re: Weird behavior of shell variables in git aliases

[Jeff King]

On Mon, Mar 21, 2011 at 09:39:43AM -0700, Dun Peal wrote:

> It seems that a variable is available only once?!  How can the
> following shell session be explained:
> 
>     $ git config alias.test0
>     !echo $1
>     $ git test0 foo
>     foo
>     $ git config alias.test1
>     !echo $1 && echo $1
>     $ git test1 foo
> 
>     foo
>     $ git config alias.test2
>     !BRANCH=$1 && echo $BRANCH && echo $BRANCH
>     $ git test2 foo
> 
>     foo

Because in v1.7.4 and earlier, we literally just tack the arguments
(shell-quoted) onto the end of the string. So your alias ends up
expanding to:

  /bin/sh -c "!echo $1 && echo $1 'foo'"

So the first echo is empty, and then the second one echos foo. And what
you are trying to do doesn't work with a straight alias (at the bottom
I'll show you what you want).

But interestingly, that's _not_ the behavior as of Erik's 7f51f8b
(alias: use run_command api to execute aliases, 2011-01-07), which is in
master but not yet released. With that, we end up executing:

  sh -c 'echo $1 && echo $1 "$@"' 'echo $1 && echo $1' 'foo'

which prints "foo foo". So it is technically a regression. I don't know
how much we care; using positional parameters like this was already
nonsensical, as shown above.

For reference, what you actually want (in either system) is:

  $ git config alias.test1
  !sh -c 'echo $1 && echo $1' -
  $ git test1 foo
  foo
  foo

Make sure to include the "-" (or some other string) which ends up as $0.

-Peff

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> But interestingly, that's _not_ the behavior as of Erik's 7f51f8b
> (alias: use run_command api to execute aliases, 2011-01-07), which is in
> master but not yet released. With that, we end up executing:
>
>   sh -c 'echo $1 && echo $1 "$@"' 'echo $1 && echo $1' 'foo'
>
> which prints "foo foo". So it is technically a regression. I don't know
> how much we care; using positional parameters like this was already
> nonsensical, as shown above.
>
> For reference, what you actually want (in either system) is:
>
>   $ git config alias.test1
>   !sh -c 'echo $1 && echo $1' -

Oh, I should have been paying a bit more attention.  I've been assuming
that we were turning "!anything" into { "sh", "-c", "anything", "-" }
followed by the user supplied arguments.

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> But interestingly, that's _not_ the behavior as of Erik's 7f51f8b
>> (alias: use run_command api to execute aliases, 2011-01-07), which is in
>> master but not yet released. With that, we end up executing:
>>
>>   sh -c 'echo $1 && echo $1 "$@"' 'echo $1 && echo $1' 'foo'
>>
>> which prints "foo foo". So it is technically a regression. I don't know
>> how much we care; using positional parameters like this was already
>> nonsensical, as shown above.
>>
>> For reference, what you actually want (in either system) is:
>>
>>   $ git config alias.test1
>>   !sh -c 'echo $1 && echo $1' -
>
> Oh, I should have been paying a bit more attention.  I've been assuming
> that we were turning "!anything" into { "sh", "-c", "anything", "-" }
> followed by the user supplied arguments.

The attached quick hack gives

  $ git config alias.silly
  !echo hello $1; echo $# args, bye!
  $ GIT_TRACE=1 ./git silly world funny
  trace: exec: 'git-silly' 'world' 'funny'
  trace: run_command: 'git-silly' 'world' 'funny'
  trace: run_command: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!'' '-' 'world' 'funny'
  trace: exec: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!''
  '-' 'world' 'funny'
  hello world
  2 args, bye!

but it would penalize a properly written alias that uses "sh -c <it> -"
trick itself by double forking, which is not very nice and I am unhappy
about.

 git.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index ef598c3..8d54466 100644
--- a/git.c
+++ b/git.c
@@ -183,11 +183,14 @@ static int handle_alias(int *argcp, const char ***argv)
 			commit_pager_choice();
 
 			/* build alias_argv */
-			alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 1));
-			alias_argv[0] = alias_string + 1;
+			alias_argv = xmalloc(sizeof(*alias_argv) * (argc + 4));
+			alias_argv[0] = "sh";
+			alias_argv[1] = "-c";
+			alias_argv[2] = alias_string + 1;
+			alias_argv[3] = "-";
 			for (i = 1; i < argc; ++i)
-				alias_argv[i] = (*argv)[i];
-			alias_argv[argc] = NULL;
+				alias_argv[i + 3] = (*argv)[i];
+			alias_argv[argc + 3] = NULL;
 
 			ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
 			if (ret >= 0)   /* normal exit */

Re: Weird behavior of shell variables in git aliases

[Lasse Makholm]

On 21 March 2011 23:33, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>> Oh, I should have been paying a bit more attention.  I've been assuming
>> that we were turning "!anything" into { "sh", "-c", "anything", "-" }
>> followed by the user supplied arguments.
>
> The attached quick hack gives
>
>  $ git config alias.silly
>  !echo hello $1; echo $# args, bye!
>  $ GIT_TRACE=1 ./git silly world funny
>  trace: exec: 'git-silly' 'world' 'funny'
>  trace: run_command: 'git-silly' 'world' 'funny'
>  trace: run_command: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!'' '-' 'world' 'funny'
>  trace: exec: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!''
>  '-' 'world' 'funny'
>  hello world
>  2 args, bye!

That would IMHO be The Right Way to do it. Since the documentation for
aliases promises to to pass my alias to a shell if I prefix it with
"!", I shouldn't have to add the "sh -c ... -" myself...

> but it would penalize a properly written alias that uses "sh -c <it> -"
> trick itself by double forking, which is not very nice and I am unhappy
> about.

A properly written alias that uses a trick? I guess that sums up the
problem... :-)

Anyway, doesn't the existing way potentially break when passing funky
arguments containing spaces/quotes/something? We currently pass the
arguments to the alias command as a single quoted string. Passing them
as seperate elements on argv seems a lot more robust...

Do we risk breaking any existing "!sh -c ... -" aliases by applying this?

/Lasse

Re: Weird behavior of shell variables in git aliases

[Jeff King]

On Tue, Mar 22, 2011 at 11:38:06AM +0100, Lasse Makholm wrote:

> > The attached quick hack gives
> >
> >  $ git config alias.silly
> >  !echo hello $1; echo $# args, bye!
> >  $ GIT_TRACE=1 ./git silly world funny
> >  trace: exec: 'git-silly' 'world' 'funny'
> >  trace: run_command: 'git-silly' 'world' 'funny'
> >  trace: run_command: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!'' '-' 'world' 'funny'
> >  trace: exec: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!''
> >  '-' 'world' 'funny'
> >  hello world
> >  2 args, bye!
> 
> That would IMHO be The Right Way to do it. Since the documentation for
> aliases promises to to pass my alias to a shell if I prefix it with
> "!", I shouldn't have to add the "sh -c ... -" myself...

It does pass it to the shell. You can do:

  $ git config alias.autolog --oneline
  !repo=`find-git-repo-for $PWD` && git --git-dir="$repo" log

for an example of a shell-based alias. It just doesn't handle positional
parameters the way you want. The typical solution is to invoke another
shell, but you can also do:

  $ grep -B1 silly .git/config
  [alias]
          silly = "!foo() { echo hello $1; echo $# args, bye!\n}\nfoo"

which unsurprisingly looks like exactly the same solution one would use
in the shell to avoid the fact that shell aliases suck for handling
positional parameters.

> > but it would penalize a properly written alias that uses "sh -c <it> -"
> > trick itself by double forking, which is not very nice and I am unhappy
> > about.
> 
> A properly written alias that uses a trick? I guess that sums up the
> problem... :-)

Yeah. Though it also penalizes non-tricky aliases. See my other mail in
this thread.

> Anyway, doesn't the existing way potentially break when passing funky
> arguments containing spaces/quotes/something? We currently pass the
> arguments to the alias command as a single quoted string. Passing them
> as seperate elements on argv seems a lot more robust...

No, the arguments in the current scheme are properly shell-quoted before
they are appended to the shell snippet. So they are equally robust.

-Peff

Re: Weird behavior of shell variables in git aliases

[Lasse Makholm]

On 22 March 2011 12:28, Jeff King <peff@peff.net> wrote:
>  $ grep -B1 silly .git/config
>  [alias]
>          silly = "!foo() { echo hello $1; echo $# args, bye!\n}\nfoo"
>
> which unsurprisingly looks like exactly the same solution one would use
> in the shell to avoid the fact that shell aliases suck for handling
> positional parameters.

Atually, I think that's probably the most elegant way of dealing with
the issue... An example like this would be very useful in the alias
documentation along with an explanation on how positional parameters
are handled...

-- 
/Lasse

Re: Weird behavior of shell variables in git aliases

[Ævar Arnfjörð Bjarmason]

On Mon, Mar 21, 2011 at 23:33, Junio C Hamano <gitster@pobox.com> wrote:

> but it would penalize a properly written alias that uses "sh -c <it> -"
> trick itself by double forking, which is not very nice and I am unhappy
> about.

It would also be bizarre on Solaris where /bin/sh isn't a POSIX shell.

But we can substitute the relevant user-supplied shell during the build process.

Re: Weird behavior of shell variables in git aliases

[Jeff King]

On Mon, Mar 21, 2011 at 03:33:25PM -0700, Junio C Hamano wrote:

> >> But interestingly, that's _not_ the behavior as of Erik's 7f51f8b
> >> (alias: use run_command api to execute aliases, 2011-01-07), which is in
> >> master but not yet released. With that, we end up executing:
> >>
> >>   sh -c 'echo $1 && echo $1 "$@"' 'echo $1 && echo $1' 'foo'
> [...]
> >
> > Oh, I should have been paying a bit more attention.  I've been assuming
> > that we were turning "!anything" into { "sh", "-c", "anything", "-" }
> > followed by the user supplied arguments.

Yeah, I think that would be more useful in general, but...

> The attached quick hack gives
> 
>   $ git config alias.silly
>   !echo hello $1; echo $# args, bye!
>   $ GIT_TRACE=1 ./git silly world funny
>   trace: exec: 'git-silly' 'world' 'funny'
>   trace: run_command: 'git-silly' 'world' 'funny'
>   trace: run_command: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!'' '-' 'world' 'funny'
>   trace: exec: 'sh' '-c' 'echo hello $1; echo $# args, bye'\!''
>   '-' 'world' 'funny'
>   hello world
>   2 args, bye!
> 
> but it would penalize a properly written alias that uses "sh -c <it> -"
> trick itself by double forking, which is not very nice and I am unhappy
> about.

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.v1.7.4 log-nopager --oneline
  trace: exec: 'git-log-nopager' '--oneline'
  trace: run_command: 'git-log-nopager' '--oneline'
  trace: alias to shell cmd: log-nopager => git --no-pager log '--oneline'
  trace: built-in: git 'log' '--oneline'
  93c7d44 foo

  $ 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

I think the evolution of the alias code was something like:

 1. Let's have alias.foo, so that "git foo XXX" can be aliased to "git
    log --whatever XXX", just like shell aliases.

 2. Oops, git doesn't allow some things in (1) that we might want to do,
    like turning off the pager or choosing a new repo. For that we need
    to respawn git, so now we have "!git --whatever1 log --whatever2".
    It appends the arguments, just like the form in (1), so it is
    consistent.

 3. Oops, (2) doesn't allow complex mini-scripts that access the
    parameters in a non-sequential way. You have to do "!sh -c 'git
    --whatever $1 log --whatever2 $2'".

Knowing that step (3) exists, I think your solution is a better one. The
question is whether it is better enough to be worth breaking the people
who were helped by step (2).

Another way of looking at it is that types (1) and (2) are like shell
aliases. And they suck for complex things, just the way that shell
aliases do. The solution in the shell is to put your complex alias into
a shell function, or to push it into its own script. In git, you can
push things into their own script, but we have no equivalent to a shell
function. So one solution is:

  git config function.silly '!echo hello $1; echo $# args, bye!'

The name is of course terrible. But I think the idea is sound that there
are two different types of interface people may want. On the other hand,
this is getting kind of grossly complex. The separate script option is
always available.

-Peff

Re: Weird behavior of shell variables in git aliases

[Jeff King]

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

Re: Weird behavior of shell variables in git aliases

[Lasse Makholm]

On 22 March 2011 14:28, Jeff King <peff@peff.net> wrote:
> 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.

Then how about simply:

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6468a68..8097480 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -586,9 +586,16 @@ If the alias expansion is prefixed with an
exclamation point,
 it will be treated as a shell command.  For example, defining
 "alias.new = !gitk --all --not ORIG_HEAD", the invocation
 "git new" is equivalent to running the shell command
-"gitk --all --not ORIG_HEAD".  Note that shell commands will be
-executed from the top-level directory of a repository, which may
-not necessarily be the current directory.
+"gitk --all --not ORIG_HEAD". Note that any arguments you pass
+when running aliases are simply appended to the shell command.
+This means that "alias.foo = !echo $# args: $1, $2 and $3" will
+not do what you expect. To use alias arguments as positional
+parameters, wrap your command in a shell function:
+"alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo"
++
+Shell commands will be executed from the top-level directory
+of a repository, which may not necessarily be the current
+directory.

 am.keepcr::
 	If true, git-am will call git-mailsplit for patches in mbox format
-- 
/Lasse

Re: Weird behavior of shell variables in git aliases

[Jeff King]

On Tue, Mar 22, 2011 at 02:35:10PM +0100, Lasse Makholm wrote:

> On 22 March 2011 14:28, Jeff King <peff@peff.net> wrote:
> > 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.
> 
> Then how about simply:
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 6468a68..8097480 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -586,9 +586,16 @@ If the alias expansion is prefixed with an
> exclamation point,
>  it will be treated as a shell command.  For example, defining
>  "alias.new = !gitk --all --not ORIG_HEAD", the invocation
>  "git new" is equivalent to running the shell command
> -"gitk --all --not ORIG_HEAD".  Note that shell commands will be
> -executed from the top-level directory of a repository, which may
> -not necessarily be the current directory.
> +"gitk --all --not ORIG_HEAD". Note that any arguments you pass
> +when running aliases are simply appended to the shell command.
> +This means that "alias.foo = !echo $# args: $1, $2 and $3" will
> +not do what you expect. To use alias arguments as positional
> +parameters, wrap your command in a shell function:
> +"alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo"
> ++
> +Shell commands will be executed from the top-level directory
> +of a repository, which may not necessarily be the current
> +directory.

Yeah, that certainly improves the situation.

A small formatting nit: Those long commands with punctuation get hard to
read in the middle of a paragraph. Maybe something like this on top:

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3916665..d2b7515 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -582,10 +582,18 @@ it will be treated as a shell command.  For example, defining
 "git new" is equivalent to running the shell command
 "gitk --all --not ORIG_HEAD". Note that any arguments you pass
 when running aliases are simply appended to the shell command.
-This means that "alias.foo = !echo $# args: $1, $2 and $3" will
-not do what you expect. To use alias arguments as positional
+This means that
++
+----------------------
+alias.foo = !echo $# args: $1, $2 and $3
+----------------------
++
+will not do what you expect. To use alias arguments as positional
 parameters, wrap your command in a shell function:
-"alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo"
++
+----------------------
+alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo
+----------------------
 +
 Shell commands will be executed from the top-level directory
 of a repository, which may not necessarily be the current

-Peff

PS Your patch was wrapped on the @@ line. You might want to check your
   mailer settings.

Re: Weird behavior of shell variables in git aliases

[Lasse Makholm]

On 22 March 2011 14:43, Jeff King <peff@peff.net> wrote:
>
> Yeah, that certainly improves the situation.
>
> A small formatting nit: Those long commands with punctuation get hard to
> read in the middle of a paragraph. Maybe something like this on top:

Thought about it but wasn't sure of the asciidoc formatting... I'll
fix it and submit a proper patch...

> PS Your patch was wrapped on the @@ line. You might want to check your
>   mailer settings.

Yeah, gmail is sucky in that regard... :-/

-- 
/Lasse

Re: Weird behavior of shell variables in git aliases

[Dun Peal]

Thanks a lot for the explanations, I was really wondering about that.

Incidentally, I had a real reason to prefer using the `!echo...`
syntax over the `!sh -c 'echo...'` one;  I need my users to define the
following alias:

    git checkout $1 && git submodule foreach 'git checkout $sha1'

Due to the necessity of single-quoting the `foreach` expression, I
couldn't define it with the `!sh -c` syntax. And because of the
problems discussed in this thread, I still don't know how to have my
users define this alias.

Thanks, D.

P.S. Incidentally, this alias is addressing a pitfall in the use of
submodules that I think should not exist in the first place:

If a repo has a submodule, and head 'foo' of the repo has the
submodule's HEAD pointing to head 'foo' of its origin, while head
'bar' of the repo has the submodule HEAD pointing to head 'bar' of its
origin, checking out bar/foo on the repository leaves the submodule in
an outdated state (and `submodule update` doesn't help).

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Mar 22, 2011 at 02:35:10PM +0100, Lasse Makholm wrote:
>
>> On 22 March 2011 14:28, Jeff King <peff@peff.net> wrote:
>> > 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.
>> 
>> Then how about simply:
>> 
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index 6468a68..8097480 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -586,9 +586,16 @@ If the alias expansion is prefixed with an
>> exclamation point,
>>  it will be treated as a shell command.  For example, defining
>>  "alias.new = !gitk --all --not ORIG_HEAD", the invocation
>>  "git new" is equivalent to running the shell command
>> -"gitk --all --not ORIG_HEAD".  Note that shell commands will be
>> -executed from the top-level directory of a repository, which may
>> -not necessarily be the current directory.
>> +"gitk --all --not ORIG_HEAD". Note that any arguments you pass
>> +when running aliases are simply appended to the shell command.
>> +This means that "alias.foo = !echo $# args: $1, $2 and $3" will
>> +not do what you expect. To use alias arguments as positional
>> +parameters, wrap your command in a shell function:
>> +"alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo"
>> ++
>> +Shell commands will be executed from the top-level directory
>> +of a repository, which may not necessarily be the current
>> +directory.
>
> Yeah, that certainly improves the situation.

The first addition is indeed a huge improvement.

    Note that any argument you pass when running aliases are simply
    appended to the shell command.

The original didn't explicitly say it but it really should have.  The
example that comes before it, "alias.new = !...", should be updated with
an invocation that takes a parameter, perhaps like this:

    With this alias defined:

	[alias] since = "!gitk --all --since"

    you can view commits in the last week with:

        $ git since 7.days

    because this expands to "gitk --all --since 7.days" by concatenating
    the arguments supplied at runtime to the alias.

Then say that "Note ..." to stress that point.  The description at that
point has become much better.

With that understanding already there,

    This means that "alias.foo = !echo $# args: $1, $2 and $3" will
    not do what you expect.

is no longer true; nobody sane would expect that if you made them realize
that "simply appended" already.  Just dropping that sentence would make
the resulting text flow much better.

    If you want to refer to arguments given to the alias, you can
    wrap it as a shell script, e.g.

	[alias] reversed = "!sh -c 'echo $2 $1' -"

    or a shell function, e.g.

	[alias] reversed = "!reversed() { echo $2 $1 } && reversed"

    and invoke it like so:

	$ git reversed one two
        two one

I personally think the former "sh -c <str> -" is the more traditional and
well understood form (iow, an idiom) for people who breathe shells.

> +----------------------
> +alias.foo = !echo $# args: $1, $2 and $3
> +----------------------

While I totally agree with the formatting advice you gave here, we may
want to avoid this notation; neither "git config alias.foo = !echo ..."
nor writing "alias.foo = ..."  in .git/config file would work.

Re: Weird behavior of shell variables in git aliases

[Jeff King]

On Tue, Mar 22, 2011 at 10:57:08AM -0700, Junio C Hamano wrote:

> The first addition is indeed a huge improvement.
> 
>     Note that any argument you pass when running aliases are simply
>     appended to the shell command.
> 
> The original didn't explicitly say it but it really should have.  The
> example that comes before it, "alias.new = !...", should be updated with
> an invocation that takes a parameter, perhaps like this:
> 
>     With this alias defined:
> 
> 	[alias] since = "!gitk --all --since"
> 
>     you can view commits in the last week with:
> 
>         $ git since 7.days
> 
>     because this expands to "gitk --all --since 7.days" by concatenating
>     the arguments supplied at runtime to the alias.
> 
> Then say that "Note ..." to stress that point.  The description at that
> point has become much better.
> 
> With that understanding already there,
> 
>     This means that "alias.foo = !echo $# args: $1, $2 and $3" will
>     not do what you expect.
> 
> is no longer true; nobody sane would expect that if you made them realize
> that "simply appended" already.  Just dropping that sentence would make
> the resulting text flow much better.

Agreed, your version is better.

>     If you want to refer to arguments given to the alias, you can
>     wrap it as a shell script, e.g.
> 
> 	[alias] reversed = "!sh -c 'echo $2 $1' -"
> 
>     or a shell function, e.g.
> 
> 	[alias] reversed = "!reversed() { echo $2 $1 } && reversed"
> 
>     and invoke it like so:
> 
> 	$ git reversed one two
>         two one
> 
> I personally think the former "sh -c <str> -" is the more traditional and
> well understood form (iow, an idiom) for people who breathe shells.

Yeah, that is probably true. One advantage of the function idiom is that
it doesn't happen inside single-quotes, so it's one less layer of
quoting to deal with. And of course it saves a shell invocation. So I
think mentioning both is reasonable.

> > +----------------------
> > +alias.foo = !echo $# args: $1, $2 and $3
> > +----------------------
> 
> While I totally agree with the formatting advice you gave here, we may
> want to avoid this notation; neither "git config alias.foo = !echo ..."
> nor writing "alias.foo = ..."  in .git/config file would work.

Yeah, I didn't even think about that, but you are right.

-Peff

Re: Weird behavior of shell variables in git aliases

[Lasse Makholm]

On 22 March 2011 19:32, Jeff King <peff@peff.net> wrote:
> On Tue, Mar 22, 2011 at 10:57:08AM -0700, Junio C Hamano wrote:
>> is no longer true; nobody sane would expect that if you made them realize
>> that "simply appended" already.  Just dropping that sentence would make
>> the resulting text flow much better.
>
> Agreed, your version is better.

Good points...

>> I personally think the former "sh -c <str> -" is the more traditional and
>> well understood form (iow, an idiom) for people who breathe shells.
>
> Yeah, that is probably true. One advantage of the function idiom is that
> it doesn't happen inside single-quotes, so it's one less layer of
> quoting to deal with. And of course it saves a shell invocation. So I
> think mentioning both is reasonable.

As one of those shell-breathing people I prefer the function-style for
its lack of quotes:

git config alias.foo = !foo () { echo $# args: $1, $2 and $3; }; foo

Whlie we're at it, is "sh -c ... -" and "sh -c ... --" equally
portable, I wonder?

>> > +----------------------
>> > +alias.foo = !echo $# args: $1, $2 and $3
>> > +----------------------
>>
>> While I totally agree with the formatting advice you gave here, we may
>> want to avoid this notation; neither "git config alias.foo = !echo ..."
>> nor writing "alias.foo = ..."  in .git/config file would work.
>
> Yeah, I didn't even think about that, but you are right.

Good point, I was wondering about that but decided to take my clue
from existing examples of the same notation...

Right, I'll sleep() on it and cook a patch tomorow, attempting to take
all of the above into account...

-- 
/Lasse

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Lasse Makholm <lasse.makholm@gmail.com> writes:

> Whlie we're at it, is "sh -c ... -" and "sh -c ... --" equally
> portable, I wonder?

I suspect anything would work, but it is customary to pass the dummy $0 by
writing "-" there.  People who breathe shells would be able to recognize
the idiom more easily if you stick to that well-known form.

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... q&d hack to grep for $1 and the like without really parsing...
> But I think that is a little too magic for my taste.

Let's not go there.

Re: Weird behavior of shell variables in git aliases

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> > Oh, I should have been paying a bit more attention.  I've been assuming
>> > that we were turning "!anything" into { "sh", "-c", "anything", "-" }
>> > followed by the user supplied arguments.
>
> Yeah, I think that would be more useful in general, but...
> ...
> Doesn't it also break a lot of other more garden-variety aliases that
> rely on the automagic "$@", like:

Of course, silly me.

Because the most common usage of the alias mechanism is to package the
command name and common options and allow tacking other command line
arguments after them at runtime, and it is silly to require the user to
say "$@" (i.e. 'lg = "log --oneline"' example), any alias that wants to
use positional parameter should do the 'sh -c "$str" -' packaging itself.

So there is nothing to fix after all; I was just confused by seeing the
initial report which was just a usage error.

Sorry for the noise, and thanks.