[PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Nguyễn Thái Ngọc Duy]

Throwing something at the mailing list to see if anybody is
interested.

Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
handling path arguments hard because they are relative to the original
cwd. We set GIT_PREFIX to work around it, but I still think it's more
natural to keep cwd where it is.

We have a way to do that now after 441981b (git: simplify environment
save/restore logic - 2016-01-26). It's just a matter of choosing the
right syntax. I'm going with '!!'. I'm not very happy with it. But I
do like this type of alias.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/git.c b/git.c
index 296857a..4c1dcf4 100644
--- a/git.c
+++ b/git.c
@@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv)
 
 			alias_string++;
 			commit_pager_choice();
+			if (*alias_string == '!') {
+				keep_cwd = 0;
+				alias_string++;
+			}
 			restore_env(keep_cwd);
 
 			child.use_shell = 1;
-- 
2.8.2.524.g6ff3d78

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Throwing something at the mailing list to see if anybody is
> interested.
>
> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> handling path arguments hard because they are relative to the original
> cwd. We set GIT_PREFIX to work around it, but I still think it's more
> natural to keep cwd where it is.
>
> We have a way to do that now after 441981b (git: simplify environment
> save/restore logic - 2016-01-26). It's just a matter of choosing the
> right syntax. I'm going with '!!'. I'm not very happy with it. But I
> do like this type of alias.

I do not know why you are not happy with the syntax, but I
personally think it brilliant, both the idea and the preliminary
clean-up that made this possible with a simple patch like this.


> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  git.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/git.c b/git.c
> index 296857a..4c1dcf4 100644
> --- a/git.c
> +++ b/git.c
> @@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv)
>  
>  			alias_string++;
>  			commit_pager_choice();
> +			if (*alias_string == '!') {
> +				keep_cwd = 0;
> +				alias_string++;
> +			}
>  			restore_env(keep_cwd);
>  
>  			child.use_shell = 1;

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

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

Hi Junio,

On Thu, 6 Oct 2016, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> 
> > Throwing something at the mailing list to see if anybody is
> > interested.
> >
> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> > handling path arguments hard because they are relative to the original
> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
> > natural to keep cwd where it is.
> >
> > We have a way to do that now after 441981b (git: simplify environment
> > save/restore logic - 2016-01-26). It's just a matter of choosing the
> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
> > do like this type of alias.
> 
> I do not know why you are not happy with the syntax, but I
> personally think it brilliant, both the idea and the preliminary
> clean-up that made this possible with a simple patch like this.

I guess he is not happy with it because "!!" is quite unintuitive a
construct. I know that *I* would have been puzzled by it, asking "What the
heck does this do?".

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Junio,
>
> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>> > Throwing something at the mailing list to see if anybody is
>> > interested.
>> >
>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
>> > handling path arguments hard because they are relative to the original
>> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
>> > natural to keep cwd where it is.
>> >
>> > We have a way to do that now after 441981b (git: simplify environment
>> > save/restore logic - 2016-01-26). It's just a matter of choosing the
>> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
>> > do like this type of alias.
>>
>> I do not know why you are not happy with the syntax, but I
>> personally think it brilliant, both the idea and the preliminary
>> clean-up that made this possible with a simple patch like this.
>
> I guess he is not happy with it because "!!" is quite unintuitive a
> construct. I know that *I* would have been puzzled by it, asking "What the
> heck does this do?".

Yep. And I wouldn't want to set a tradition for the next alias type
'!!!'. There's no good choice to represent a new alias type with a
leading symbol. This just occurred to me, however, what do you think
about a new config group for it? With can have something like
externalAlias.* (or some other name) that lives in parallel with
alias.*. Then we don't need '!' (or '!!') at all.
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Matthieu Moy]

Duy Nguyen <pclouds@gmail.com> writes:

> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi Junio,
>>
>> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>>
>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>>
>>> > Throwing something at the mailing list to see if anybody is
>>> > interested.
>>> >
>>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
>>> > make
>>> > handling path arguments hard because they are relative to the
>>> > original
>>> > cwd. We set GIT_PREFIX to work around it, but I still think it's
>>> > more
>>> > natural to keep cwd where it is.
>>> >
>>> > We have a way to do that now after 441981b (git: simplify
>>> > environment
>>> > save/restore logic - 2016-01-26). It's just a matter of choosing
>>> > the
>>> > right syntax. I'm going with '!!'. I'm not very happy with it.
>>> > But I
>>> > do like this type of alias.
>>>
>>> I do not know why you are not happy with the syntax, but I
>>> personally think it brilliant, both the idea and the preliminary
>>> clean-up that made this possible with a simple patch like this.
>>
>> I guess he is not happy with it because "!!" is quite unintuitive a
>> construct. I know that *I* would have been puzzled by it, asking
>> "What the
>> heck does this do?".
>
> Yep. And I wouldn't want to set a tradition for the next alias type
> '!!!'. There's no good choice to represent a new alias type with a
> leading symbol. This just occurred to me, however, what do you think
> about a new config group for it? With can have something like
> externalAlias.* (or some other name) that lives in parallel with
> alias.*. Then we don't need '!' (or '!!') at all.

Another possibility: !(nocd), which leaves room
for !(keyword1,keyword2,...) if needed later. Also, it is consistent
with the :(word) syntax of pathspecs.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 7:47 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>>> Hi Junio,
>>>
>>> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>>>
>>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>>>
>>>> > Throwing something at the mailing list to see if anybody is
>>>> > interested.
>>>> >
>>>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
>>>> > make
>>>> > handling path arguments hard because they are relative to the
>>>> > original
>>>> > cwd. We set GIT_PREFIX to work around it, but I still think it's
>>>> > more
>>>> > natural to keep cwd where it is.
>>>> >
>>>> > We have a way to do that now after 441981b (git: simplify
>>>> > environment
>>>> > save/restore logic - 2016-01-26). It's just a matter of choosing
>>>> > the
>>>> > right syntax. I'm going with '!!'. I'm not very happy with it.
>>>> > But I
>>>> > do like this type of alias.
>>>>
>>>> I do not know why you are not happy with the syntax, but I
>>>> personally think it brilliant, both the idea and the preliminary
>>>> clean-up that made this possible with a simple patch like this.
>>>
>>> I guess he is not happy with it because "!!" is quite unintuitive a
>>> construct. I know that *I* would have been puzzled by it, asking
>>> "What the
>>> heck does this do?".
>>
>> Yep. And I wouldn't want to set a tradition for the next alias type
>> '!!!'. There's no good choice to represent a new alias type with a
>> leading symbol. This just occurred to me, however, what do you think
>> about a new config group for it? With can have something like
>> externalAlias.* (or some other name) that lives in parallel with
>> alias.*. Then we don't need '!' (or '!!') at all.
>
> Another possibility: !(nocd), which leaves room
> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
> with the :(word) syntax of pathspecs.

This seems to solve my problem nicely.
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

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

Hi Matthieu,

On Fri, 7 Oct 2016, Matthieu Moy wrote:

> Duy Nguyen <pclouds@gmail.com> writes:
> 
> > On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> >> Hi Junio,
> >>
> >> On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >>
> >>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> >>>
> >>> > Throwing something at the mailing list to see if anybody is
> >>> > interested.
> >>> >
> >>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could
> >>> > make
> >>> > handling path arguments hard because they are relative to the
> >>> > original
> >>> > cwd. We set GIT_PREFIX to work around it, but I still think it's
> >>> > more
> >>> > natural to keep cwd where it is.
> >>> >
> >>> > We have a way to do that now after 441981b (git: simplify
> >>> > environment
> >>> > save/restore logic - 2016-01-26). It's just a matter of choosing
> >>> > the
> >>> > right syntax. I'm going with '!!'. I'm not very happy with it.
> >>> > But I
> >>> > do like this type of alias.
> >>>
> >>> I do not know why you are not happy with the syntax, but I
> >>> personally think it brilliant, both the idea and the preliminary
> >>> clean-up that made this possible with a simple patch like this.
> >>
> >> I guess he is not happy with it because "!!" is quite unintuitive a
> >> construct. I know that *I* would have been puzzled by it, asking
> >> "What the
> >> heck does this do?".
> >
> > Yep. And I wouldn't want to set a tradition for the next alias type
> > '!!!'. There's no good choice to represent a new alias type with a
> > leading symbol. This just occurred to me, however, what do you think
> > about a new config group for it? With can have something like
> > externalAlias.* (or some other name) that lives in parallel with
> > alias.*. Then we don't need '!' (or '!!') at all.
> 
> Another possibility: !(nocd), which leaves room
> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
> with the :(word) syntax of pathspecs.

But is this backwards-compatible? Don't we execute everything that comes
after the exclamation mark as a command-line via shell, where the
parentheses mean "open a subshell"?

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Matthieu Moy]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Matthieu,
>
> On Fri, 7 Oct 2016, Matthieu Moy wrote:
>
>> Another possibility: !(nocd), which leaves room
>> for !(keyword1,keyword2,...) if needed later. Also, it is consistent
>> with the :(word) syntax of pathspecs.
>
> But is this backwards-compatible? Don't we execute everything that comes
> after the exclamation mark as a command-line via shell, where the
> parentheses mean "open a subshell"?

Good point. I can imagine someone already having

[alias]
	foo = !(cd bar && $something) && $something_else

Your proposed (keyword)! is better.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

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

Hi Duy,

On Fri, 7 Oct 2016, Duy Nguyen wrote:

> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >
> >> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> >>
> >> > Throwing something at the mailing list to see if anybody is
> >> > interested.
> >> >
> >> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> >> > handling path arguments hard because they are relative to the original
> >> > cwd. We set GIT_PREFIX to work around it, but I still think it's more
> >> > natural to keep cwd where it is.
> >> >
> >> > We have a way to do that now after 441981b (git: simplify environment
> >> > save/restore logic - 2016-01-26). It's just a matter of choosing the
> >> > right syntax. I'm going with '!!'. I'm not very happy with it. But I
> >> > do like this type of alias.
> >>
> >> I do not know why you are not happy with the syntax, but I
> >> personally think it brilliant, both the idea and the preliminary
> >> clean-up that made this possible with a simple patch like this.
> >
> > I guess he is not happy with it because "!!" is quite unintuitive a
> > construct. I know that *I* would have been puzzled by it, asking "What the
> > heck does this do?".
> 
> Yep. And I wouldn't want to set a tradition for the next alias type
> '!!!'. There's no good choice to represent a new alias type with a
> leading symbol. This just occurred to me, however, what do you think
> about a new config group for it? With can have something like
> externalAlias.* (or some other name) that lives in parallel with
> alias.*. Then we don't need '!' (or '!!') at all.

But what would the precedence be? externalAlias.xyz wins over alias.xyz?

And we still would need '!' support: tons of people (including myself)
rely on it.

Possibly a better idea would be to use *another* special symbol, one that
makes intuitive sense as a modifier, such as:

	[alias]
		# This works as before
		xyz = !pwd
		# As does this
		stat = -p status
		# This, however, is different:
		duy = (nocd)!pwd

This is backwards compatible as "(" is not a part of any Git command, nor
of a valid alias, nor is it commonly used as part of a git-*
executable/script.

It is also kind of a bit more intuitive, I'd wager, and it is also
extensible to future options we may want to introduce.

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Fri, Oct 07, 2016 at 04:11:34PM +0200, Johannes Schindelin wrote:

> Possibly a better idea would be to use *another* special symbol, one that
> makes intuitive sense as a modifier, such as:
> 
> 	[alias]
> 		# This works as before
> 		xyz = !pwd
> 		# As does this
> 		stat = -p status
> 		# This, however, is different:
> 		duy = (nocd)!pwd
> 
> This is backwards compatible as "(" is not a part of any Git command, nor
> of a valid alias, nor is it commonly used as part of a git-*
> executable/script.
> 
> It is also kind of a bit more intuitive, I'd wager, and it is also
> extensible to future options we may want to introduce.

I like this much better (like you, I am concerned about things like
"!(foo)" as conflicting with the shell). And I think your "(nocd)!pwd"
example is quite readable.

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Sixt]

Am 07.10.2016 um 14:27 schrieb Duy Nguyen:
> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> Hi Junio,
>>
>> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>>
>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>>
>>>> Throwing something at the mailing list to see if anybody is
>>>> interested.
>>>>
>>>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
>>>> handling path arguments hard because they are relative to the original
>>>> cwd. We set GIT_PREFIX to work around it, but I still think it's more
>>>> natural to keep cwd where it is.
>>>>
>>>> We have a way to do that now after 441981b (git: simplify environment
>>>> save/restore logic - 2016-01-26). It's just a matter of choosing the
>>>> right syntax. I'm going with '!!'. I'm not very happy with it. But I
>>>> do like this type of alias.
>>>
>>> I do not know why you are not happy with the syntax, but I
>>> personally think it brilliant, both the idea and the preliminary
>>> clean-up that made this possible with a simple patch like this.
>>
>> I guess he is not happy with it because "!!" is quite unintuitive a
>> construct. I know that *I* would have been puzzled by it, asking "What the
>> heck does this do?".
>
> Yep. And I wouldn't want to set a tradition for the next alias type
> '!!!'. There's no good choice to represent a new alias type with a
> leading symbol. This just occurred to me, however, what do you think
> about a new config group for it? With can have something like
> externalAlias.* (or some other name) that lives in parallel with
> alias.*. Then we don't need '!' (or '!!') at all.

Maybe it's time to aim for

   git config alias.d2u.shell \
        'f() { git ls-files "$@" | xargs dos2unix; }; f'
   git config alias.d2u.cdup false
   git d2u *.c   # yada!

-- Hannes

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote:

> Maybe it's time to aim for
> 
>   git config alias.d2u.shell \
>        'f() { git ls-files "$@" | xargs dos2unix; }; f'
>   git config alias.d2u.cdup false
>   git d2u *.c   # yada!

That would be nice. It would also allow "alias.foo_bar.shell"; right now
"alias.foo_bar" is forbidden because of the config syntax, which does
not allow underscores outside of the "subsection" name.

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Peff & Hannes,

On Fri, 7 Oct 2016, Jeff King wrote:

> On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote:
> 
> > Maybe it's time to aim for
> > 
> >   git config alias.d2u.shell \
> >        'f() { git ls-files "$@" | xargs dos2unix; }; f'
> >   git config alias.d2u.cdup false
> >   git d2u *.c   # yada!
> 
> That would be nice. It would also allow "alias.foo_bar.shell"; right now
> "alias.foo_bar" is forbidden because of the config syntax, which does
> not allow underscores outside of the "subsection" name.

So what about this?

	[alias]
		d2u = !dos2unix
	[alias "d2u"]
		shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
		exec = C:/cygwin64/bin/dos2unix.exe

You introduce all kinds of ambiguities here that did not exist before...

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote:

> > > Maybe it's time to aim for
> > > 
> > >   git config alias.d2u.shell \
> > >        'f() { git ls-files "$@" | xargs dos2unix; }; f'
> > >   git config alias.d2u.cdup false
> > >   git d2u *.c   # yada!
> > 
> > That would be nice. It would also allow "alias.foo_bar.shell"; right now
> > "alias.foo_bar" is forbidden because of the config syntax, which does
> > not allow underscores outside of the "subsection" name.
> 
> So what about this?
> 
> 	[alias]
> 		d2u = !dos2unix
> 	[alias "d2u"]
> 		shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
> 		exec = C:/cygwin64/bin/dos2unix.exe
> 
> You introduce all kinds of ambiguities here that did not exist before...

If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
then yes, I think that's an unavoidable part of the transition.  IMHO,
the new should take precedence over the old, and people will gradually
move from one to the other.

If you mean the ambiguity between alias.X.shell and alias.X.exec in your
example, the problem is that you have keys with overlapping meanings.
One solution is "don't do that" (so have a key like "cmd", and another
to select "shell or git-cmd", etc). Another is to define some rule, like
"last one wins" (so "exec" overrides "shell" in your example).

I'd prefer the "don't do that" path. The config you showed is
nonsensical, and it doesn't really matter that much how we behave. But
it is better still if we have a config scheme that makes it hard to
write nonsensical things in the first place.

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Sun, Oct 09, 2016 at 02:01:49AM -0400, Jeff King wrote:

> > So what about this?
> > 
> > 	[alias]
> > 		d2u = !dos2unix
> > 	[alias "d2u"]
> > 		shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
> > 		exec = C:/cygwin64/bin/dos2unix.exe
> > 
> > You introduce all kinds of ambiguities here that did not exist before...
> 
> If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> then yes, I think that's an unavoidable part of the transition.  IMHO,
> the new should take precedence over the old, and people will gradually
> move from one to the other.
> 
> If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> example, the problem is that you have keys with overlapping meanings.
> One solution is "don't do that" (so have a key like "cmd", and another
> to select "shell or git-cmd", etc). Another is to define some rule, like
> "last one wins" (so "exec" overrides "shell" in your example).
> 
> I'd prefer the "don't do that" path. The config you showed is
> nonsensical, and it doesn't really matter that much how we behave. But
> it is better still if we have a config scheme that makes it hard to
> write nonsensical things in the first place.

Just to be clear on my "don't do that", I don't mean "the user should
not do that", but "do not design a config scheme with keys that have
overlapping meanings".

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Sun, Oct 9, 2016 at 1:01 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote:
>
>> > > Maybe it's time to aim for
>> > >
>> > >   git config alias.d2u.shell \
>> > >        'f() { git ls-files "$@" | xargs dos2unix; }; f'
>> > >   git config alias.d2u.cdup false
>> > >   git d2u *.c   # yada!
>> >
>> > That would be nice. It would also allow "alias.foo_bar.shell"; right now
>> > "alias.foo_bar" is forbidden because of the config syntax, which does
>> > not allow underscores outside of the "subsection" name.
>>
>> So what about this?
>>
>>       [alias]
>>               d2u = !dos2unix
>>       [alias "d2u"]
>>               shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
>>               exec = C:/cygwin64/bin/dos2unix.exe
>>
>> You introduce all kinds of ambiguities here that did not exist before...
>
> If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> then yes, I think that's an unavoidable part of the transition.  IMHO,
> the new should take precedence over the old, and people will gradually
> move from one to the other.

Do we really need to treat this differently than

[alias]
    d2u = !dos2unix
    d2u = C:/cygwin/bin/dos3unix.exe

?

Another similar case is one d2u (could be either old syntax or new) is
defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
either case, the "latest" d2u definition wins.

> If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> example, the problem is that you have keys with overlapping meanings.
> One solution is "don't do that" (so have a key like "cmd", and another
> to select "shell or git-cmd", etc). Another is to define some rule, like
> "last one wins" (so "exec" overrides "shell" in your example).
>
> I'd prefer the "don't do that" path. The config you showed is
> nonsensical, and it doesn't really matter that much how we behave. But
> it is better still if we have a config scheme that makes it hard to
> write nonsensical things in the first place.

Any suggestion? I suppose we can have _one_ key for the command. How
to execute that command (exec, shell, nocd...) are boolean options.
People can still write conflicting things. We have been nice so far,
always dying when the user specify conflicting command line options.
We could do the same here, I guess.
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:

> > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> > then yes, I think that's an unavoidable part of the transition.  IMHO,
> > the new should take precedence over the old, and people will gradually
> > move from one to the other.
> 
> Do we really need to treat this differently than
> 
> [alias]
>     d2u = !dos2unix
>     d2u = C:/cygwin/bin/dos3unix.exe
> 
> ?
> 
> Another similar case is one d2u (could be either old syntax or new) is
> defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
> either case, the "latest" d2u definition wins.

Yeah, that's reasonable, too. So:

  [alias]
    d2u = "!dos2unix"

acts exactly as if:

  [alias "d2u"]
    command = dos2unix
    type = shell

was specified at that point, which is easy to understand.

> > If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> > example, the problem is that you have keys with overlapping meanings.
> > One solution is "don't do that" (so have a key like "cmd", and another
> > to select "shell or git-cmd", etc). Another is to define some rule, like
> > "last one wins" (so "exec" overrides "shell" in your example).
> [...]
> 
> Any suggestion? I suppose we can have _one_ key for the command. How
> to execute that command (exec, shell, nocd...) are boolean options.
> People can still write conflicting things. We have been nice so far,
> always dying when the user specify conflicting command line options.
> We could do the same here, I guess.

Having separate exec/shell boolean options just punts the overlap from
the command key to those keys. If you have two mutually exclusive
options, I think the best thing is a single option, like:

  type = <shell | exec | whatever>

and then it is obvious that a second appearance of "type" overrides an
earlier one, by our usual "last one wins" convention. As opposed to:

  shell = true
  exec = true

where you have to understand the meaning of each option to know that
"exec" overrides "shell".

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Having separate exec/shell boolean options just punts the overlap from
> the command key to those keys. If you have two mutually exclusive
> options, I think the best thing is a single option, like:
>
>   type = <shell | exec | whatever>
>
> and then it is obvious that a second appearance of "type" overrides an
> earlier one, by our usual "last one wins" convention. As opposed to:
>
>   shell = true
>   exec = true
>
> where you have to understand the meaning of each option to know that
> "exec" overrides "shell".

Good.  

Duy's "do we want to chdir or stay?" would be an orthogonal axis to
"what does the command line look like?" and "how is the command line
run?" so it adds one member to the "alias.<string>.*" family of
variables, I guess.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Mon, Oct 10, 2016 at 10:52:26AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Having separate exec/shell boolean options just punts the overlap from
> > the command key to those keys. If you have two mutually exclusive
> > options, I think the best thing is a single option, like:
> >
> >   type = <shell | exec | whatever>
> >
> > and then it is obvious that a second appearance of "type" overrides an
> > earlier one, by our usual "last one wins" convention. As opposed to:
> >
> >   shell = true
> >   exec = true
> >
> > where you have to understand the meaning of each option to know that
> > "exec" overrides "shell".
> 
> Good.  
> 
> Duy's "do we want to chdir or stay?" would be an orthogonal axis to
> "what does the command line look like?" and "how is the command line
> run?" so it adds one member to the "alias.<string>.*" family of
> variables, I guess.

Yeah, exactly. There may be other orthogonal axes to add, too, but I do
not have any in mind myself. My main motive in switching to the
"alias.$cmd.key" syntax is that it fixes the ancient mistake of putting
arbitrary content into the key (just like pager.*, as we've discussed
elsewhere).

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... My main motive in switching to the
> "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting
> arbitrary content into the key (just like pager.*, as we've discussed
> elsewhere).

Yup, we are on the same page.  It's not too grave a mistake (we said
"these are command names and do not have to be able to contain
random letters" and deliberately did without the usual three-level
hierarchy; I think the reasoning is even more valid for pager.*),
but it does imply case insensitivity not to use three-level names,
and I think it is a good move.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi,

On Sun, 9 Oct 2016, Jeff King wrote:

> On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:
> 
> > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> > > then yes, I think that's an unavoidable part of the transition.  IMHO,
> > > the new should take precedence over the old, and people will gradually
> > > move from one to the other.
> > 
> > Do we really need to treat this differently than
> > 
> > [alias]
> >     d2u = !dos2unix
> >     d2u = C:/cygwin/bin/dos3unix.exe
> > 
> > ?
> > 
> > Another similar case is one d2u (could be either old syntax or new) is
> > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
> > either case, the "latest" d2u definition wins.
> 
> Yeah, that's reasonable, too. So:
> 
>   [alias]
>     d2u = "!dos2unix"
> 
> acts exactly as if:
> 
>   [alias "d2u"]
>     command = dos2unix
>     type = shell
> 
> was specified at that point, which is easy to understand.

It is easy to understand, and even easier to get wrong or out of sync. I
really liked the ease of *one* `git config` call to add new aliases. Not
sure that I like the need for more such calls just to add *one* alias (one
config call for "shell", one for "don't cd up", etc).

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Tue, Oct 11, 2016 at 4:44 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 9 Oct 2016, Jeff King wrote:
>
>> On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:
>>
>> > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
>> > > then yes, I think that's an unavoidable part of the transition.  IMHO,
>> > > the new should take precedence over the old, and people will gradually
>> > > move from one to the other.
>> >
>> > Do we really need to treat this differently than
>> >
>> > [alias]
>> >     d2u = !dos2unix
>> >     d2u = C:/cygwin/bin/dos3unix.exe
>> >
>> > ?
>> >
>> > Another similar case is one d2u (could be either old syntax or new) is
>> > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
>> > either case, the "latest" d2u definition wins.
>>
>> Yeah, that's reasonable, too. So:
>>
>>   [alias]
>>     d2u = "!dos2unix"
>>
>> acts exactly as if:
>>
>>   [alias "d2u"]
>>     command = dos2unix
>>     type = shell
>>
>> was specified at that point, which is easy to understand.
>
> It is easy to understand, and even easier to get wrong or out of sync. I
> really liked the ease of *one* `git config` call to add new aliases.

I was about to bring this up. Although to me, "git config --global
alias.foo bar" is more convenient, but not using it is not exactly
easy to get wrong or out of sync. For adding alias.$name.* I was
thinking about "git config --global --edit", not executing "git
config" multiple times.

> Not sure that I like the need for more such calls just to add *one* alias (one
> config call for "shell", one for "don't cd up", etc).

We could add git-alias if more alias types pop up (and in my opinion
git-alias is the right call, we've been abusing git-config for alias
manipulation for a long time).
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

Hi Duy,

On Tue, 11 Oct 2016, Duy Nguyen wrote:

> On Tue, Oct 11, 2016 at 4:44 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Sun, 9 Oct 2016, Jeff King wrote:
> >
> >> On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:
> >>
> >> > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> >> > > then yes, I think that's an unavoidable part of the transition.  IMHO,
> >> > > the new should take precedence over the old, and people will gradually
> >> > > move from one to the other.
> >> >
> >> > Do we really need to treat this differently than
> >> >
> >> > [alias]
> >> >     d2u = !dos2unix
> >> >     d2u = C:/cygwin/bin/dos3unix.exe
> >> >
> >> > ?
> >> >
> >> > Another similar case is one d2u (could be either old syntax or new) is
> >> > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
> >> > either case, the "latest" d2u definition wins.
> >>
> >> Yeah, that's reasonable, too. So:
> >>
> >>   [alias]
> >>     d2u = "!dos2unix"
> >>
> >> acts exactly as if:
> >>
> >>   [alias "d2u"]
> >>     command = dos2unix
> >>     type = shell
> >>
> >> was specified at that point, which is easy to understand.
> >
> > It is easy to understand, and even easier to get wrong or out of sync. I
> > really liked the ease of *one* `git config` call to add new aliases.
> 
> I was about to bring this up. Although to me, "git config --global
> alias.foo bar" is more convenient, but not using it is not exactly
> easy to get wrong or out of sync. For adding alias.$name.* I was
> thinking about "git config --global --edit", not executing "git
> config" multiple times.

Right, but many of my aliases get set by scripts, so your `--edit` idea
won't work for me.

> > Not sure that I like the need for more such calls just to add *one* alias (one
> > config call for "shell", one for "don't cd up", etc).
> 
> We could add git-alias if more alias types pop up (and in my opinion
> git-alias is the right call, we've been abusing git-config for alias
> manipulation for a long time).

Maybe.

It is also possible that this issue is a good indicator that we are
complicating things [*1*] more than necessary...

Ciao,
Dscho

Footnote *1*:
http://thedailywtf.com/articles/The_Complicator_0x27_s_Gloves

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Tue, Oct 11, 2016 at 11:44:50AM +0200, Johannes Schindelin wrote:

> > Yeah, that's reasonable, too. So:
> > 
> >   [alias]
> >     d2u = "!dos2unix"
> > 
> > acts exactly as if:
> > 
> >   [alias "d2u"]
> >     command = dos2unix
> >     type = shell
> > 
> > was specified at that point, which is easy to understand.
> 
> It is easy to understand, and even easier to get wrong or out of sync. I
> really liked the ease of *one* `git config` call to add new aliases. Not
> sure that I like the need for more such calls just to add *one* alias (one
> config call for "shell", one for "don't cd up", etc).

Could we simply support alias.d2u indefinitely, and you could use
whichever format you felt like (the shorter, more limited one if you
wanted, or the more verbose but flexible one)?

-Peff

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Tue, Oct 11, 2016 at 10:01 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 11, 2016 at 11:44:50AM +0200, Johannes Schindelin wrote:
>
>> > Yeah, that's reasonable, too. So:
>> >
>> >   [alias]
>> >     d2u = "!dos2unix"
>> >
>> > acts exactly as if:
>> >
>> >   [alias "d2u"]
>> >     command = dos2unix
>> >     type = shell
>> >
>> > was specified at that point, which is easy to understand.
>>
>> It is easy to understand, and even easier to get wrong or out of sync. I
>> really liked the ease of *one* `git config` call to add new aliases. Not
>> sure that I like the need for more such calls just to add *one* alias (one
>> config call for "shell", one for "don't cd up", etc).
>
> Could we simply support alias.d2u indefinitely, and you could use
> whichever format you felt like (the shorter, more limited one if you
> wanted, or the more verbose but flexible one)?

Before this thread goes completely dead... Since there's a lot more
work involved with the new alias.<name>.<property> approach (short
term would be git completion support, longer term would be the ability
to manipulate a config group more conveniently), I'm going with the
"(properties)!command" approach. But even then a new series is not
going to pop up, like, in the next two months.

I don't object the alias.<name>.<property> approach though. It's
definitely a cleaner one in my opinion. It just needs people who can
spend time to follow up until the end. But if someone decides to do
that now, I'll drop the "(properties)!command" and try to support
him/her.
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> I don't object the alias.<name>.<property> approach though. It's
> definitely a cleaner one in my opinion. It just needs people who can
> spend time to follow up until the end. But if someone decides to do
> that now, I'll drop the "(properties)!command" and try to support
> him/her.

I don't object to either approach, but what I would love to see
people avoid is to end up with both.

Thanks.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jakub Narębski]

Hello, Johannes

W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze:
> On Thu, 6 Oct 2016, Junio C Hamano wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> Throwing something at the mailing list to see if anybody is
>>> interested.
>>>
>>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
>>> handling path arguments hard because they are relative to the original
>>> cwd. We set GIT_PREFIX to work around it, but I still think it's more
>>> natural to keep cwd where it is.
>>>
>>> We have a way to do that now after 441981b (git: simplify environment
>>> save/restore logic - 2016-01-26). It's just a matter of choosing the
>>> right syntax. I'm going with '!!'. I'm not very happy with it. But I
>>> do like this type of alias.
>>
>> I do not know why you are not happy with the syntax, but I
>> personally think it brilliant, both the idea and the preliminary
>> clean-up that made this possible with a simple patch like this.
> 
> I guess he is not happy with it because "!!" is quite unintuitive a
> construct. I know that *I* would have been puzzled by it, asking "What the
> heck does this do?".

Well, "!" as a prefix is not intuitive either.

Perhaps "!.", because "." is current directory, and the "." command
(that is, alias to "source") doesn't make sense in git aliases.

Note that we would have to teach git completion about new syntax;
or new configuration variable if we go that route.
-- 
Jakub Narębski

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Johannes Schindelin]

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

Hi Kuba,

On Fri, 7 Oct 2016, Jakub Narębski wrote:

> W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze:
> > On Thu, 6 Oct 2016, Junio C Hamano wrote:
> >> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> >>
> >>> Throwing something at the mailing list to see if anybody is
> >>> interested.
> >>>
> >>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> >>> handling path arguments hard because they are relative to the original
> >>> cwd. We set GIT_PREFIX to work around it, but I still think it's more
> >>> natural to keep cwd where it is.
> >>>
> >>> We have a way to do that now after 441981b (git: simplify environment
> >>> save/restore logic - 2016-01-26). It's just a matter of choosing the
> >>> right syntax. I'm going with '!!'. I'm not very happy with it. But I
> >>> do like this type of alias.
> >>
> >> I do not know why you are not happy with the syntax, but I
> >> personally think it brilliant, both the idea and the preliminary
> >> clean-up that made this possible with a simple patch like this.
> > 
> > I guess he is not happy with it because "!!" is quite unintuitive a
> > construct. I know that *I* would have been puzzled by it, asking "What the
> > heck does this do?".
> 
> Well, "!" as a prefix is not intuitive either.

You do not use vi, do you? :-P

In vi, if you enter command mode (typing a colon) and then want to
execute, say, `pwd`, you type !pwd<Enter>

> Perhaps "!.", because "." is current directory, and the "." command
> (that is, alias to "source") doesn't make sense in git aliases.

If you want to execute, say, `pwd` in the current directory, that would
mean you want to write

	!.pwd

But that already means "execute `.pwd`"...

> Note that we would have to teach git completion about new syntax;
> or new configuration variable if we go that route.

Why would we? Git's completion does not expand aliases, it only completes
the aliases' names, not their values.

Ciao,
Dscho

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jakub Narębski]

W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: 
> On Fri, 7 Oct 2016, Jakub Narębski wrote:

>> Note that we would have to teach git completion about new syntax;
>> or new configuration variable if we go that route.
> 
> Why would we? Git's completion does not expand aliases, it only completes
> the aliases' names, not their values.

Because Git completion finds out which _options_ and _arguments_
to complete for an alias based on its expansion.

Yes, this is nice bit of trickery...
-- 
Jakub Narębski

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Duy Nguyen]

On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze:
>> On Fri, 7 Oct 2016, Jakub Narębski wrote:
>
>>> Note that we would have to teach git completion about new syntax;
>>> or new configuration variable if we go that route.
>>
>> Why would we? Git's completion does not expand aliases, it only completes
>> the aliases' names, not their values.
>
> Because Git completion finds out which _options_ and _arguments_
> to complete for an alias based on its expansion.
>
> Yes, this is nice bit of trickery...

It's c63437c (bash: improve aliased command recognition - 2010-02-23)
isn't it? This may favor j6t's approach [1] because retrieving alias
attributes is much easier.

[1] https://public-inbox.org/git/20161006190720.4ecf3ptl6msztoya@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4
-- 
Duy

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[SZEDER Gábor]

Quoting Duy Nguyen <pclouds@gmail.com>:

> On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski <jnareb@gmail.com> wrote:
>> W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze:
>>> On Fri, 7 Oct 2016, Jakub Narębski wrote:
>>
>>>> Note that we would have to teach git completion about new syntax;
>>>> or new configuration variable if we go that route.
>>>
>>> Why would we? Git's completion does not expand aliases, it only completes
>>> the aliases' names, not their values.
>>
>> Because Git completion finds out which _options_ and _arguments_
>> to complete for an alias based on its expansion.
>>
>> Yes, this is nice bit of trickery...
>
> It's c63437c (bash: improve aliased command recognition - 2010-02-23)
> isn't it? This may favor j6t's approach [1] because retrieving alias
> attributes is much easier.
>
> [1]
> https://public-inbox.org/git/20161006190720.4ecf3ptl6msztoya@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4
> --
> Duy

The completion script is concerned in three ways:

   1. it has to get the names of all aliases, to offer them along with
      git commands for 'git <TAB>' or 'git help <TAB>',

   2. it has to get the command executed in place of the alias, and

   3. strip everything that can't be a git command, so it can offer the
      right options for the aliased command.


The '!!' syntax is the easiest, I think it will just work even with
the current completion code, no changes necessary.

The '(nocd)' form is still easy, we only have to add this '(nocd)' to
that list of stripped words for (3), but no further changes necessary
for (1) and (2).

'alias.d2u.command' is tricky.  Both (1) and (2) require adjustments,
preferably in a way that doesn't involve additional git processes, at
least for (1), as it is executed often, for every 'git <TAB>', for the
sake of users on platforms with not particularly stellar fork()+exec()
performance.  I think it would be possible to have only one 'git
config --get-regexp' and a little bit of post processing in each case,
but I didn't think too hard about possible pitfalls, especially when
dealing with ambiguity when both 'alias.d2u' and 'alias.d2u.command'
are set.  And I won't think any more about it until a conclusion is
reached that we'll go this route :)

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jakub Narębski]

W dniu 11.10.2016 o 13:51, SZEDER Gábor pisze:
> Quoting Duy Nguyen <pclouds@gmail.com>:
>> On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski <jnareb@gmail.com> wrote:
>>> W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze:
>>>> On Fri, 7 Oct 2016, Jakub Narębski wrote:
>>>
>>>>> Note that we would have to teach git completion about new syntax;
>>>>> or new configuration variable if we go that route.
>>>>
>>>> Why would we? Git's completion does not expand aliases, it only completes
>>>> the aliases' names, not their values.
>>>
>>> Because Git completion finds out which _options_ and _arguments_
>>> to complete for an alias based on its expansion.
>>>
>>> Yes, this is nice bit of trickery...
>>
>> It's c63437c (bash: improve aliased command recognition - 2010-02-23)
>> isn't it? This may favor j6t's approach [1] because retrieving alias
>> attributes is much easier.
>>
>> [1] https://public-inbox.org/git/20161006190720.4ecf3ptl6msztoya@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4
>> -- 
>> Duy
> 
> The completion script is concerned in three ways:
> 
>   1. it has to get the names of all aliases, to offer them along with
>      git commands for 'git <TAB>' or 'git help <TAB>',
> 
>   2. it has to get the command executed in place of the alias, and
> 
>   3. strip everything that can't be a git command, so it can offer the
>      right options for the aliased command.

There is also a possibility to tell the completion script which git
command to use for option completion via some trickery, even if the
first git command of many used in script is not right (e.g. when
"$@" is passed somewhere in the middle).

I don't remember exact details, but let's look at source:

  # If you use complex aliases of form '!f() { ... }; f', you can use the null
  # command ':' as the first command in the function body to declare the desired
  # completion style.  For example '!f() { : git commit ; ... }; f' will
  # tell the completion to use commit completion.  This also works with aliases
  # of form "!sh -c '...'".  For example, "!sh -c ': git commit ; ... '".

Very nice.

> 
> The '!!' syntax is the easiest, I think it will just work even with
> the current completion code, no changes necessary.
> 
> The '(nocd)' form is still easy, we only have to add this '(nocd)' to
> that list of stripped words for (3), but no further changes necessary
> for (1) and (2).

Shouldn't the '!' vs '!!' / '(nocd)!' affect pathname completion?
Or do tab completion in subdir offer wrong completion of filenames
for aliases?

Best,
-- 
Jakub Narębski

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Thu, Oct 06, 2016 at 06:41:24PM +0700, Nguyễn Thái Ngọc Duy wrote:

> Throwing something at the mailing list to see if anybody is
> interested.
> 
> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make
> handling path arguments hard because they are relative to the original
> cwd. We set GIT_PREFIX to work around it, but I still think it's more
> natural to keep cwd where it is.
> 
> We have a way to do that now after 441981b (git: simplify environment
> save/restore logic - 2016-01-26). It's just a matter of choosing the
> right syntax. I'm going with '!!'. I'm not very happy with it. But I
> do like this type of alias.

Hmm. I wonder if any commands will be fooled by not being moved to
GIT_WORK_TREE. I know that there is a bug already in this case:

  export HOME=$PWD

  for i in one two; do
  	git init $i &&
  	(cd $i &&
  	 echo "* diff=$i" >.gitattributes &&
  	 git add . &&
  	 git commit -m $i) &&
  	git config --global diff.$i.textconv "sed s/^/$i:/"
  done

  cd two &&
  git --git-dir=$(pwd)/../one/.git --work-tree=$(pwd)/../one show

This shows the contents of repo one using the .gitattributes from repo
two. I am not sure if the bug is that when GIT_WORK_TREE is set,
git-show doesn't position itself at the toplevel of that tree, or if the
attributes machinery should be more careful about looking up paths in
the work-tree versus the current directory.

This bug is tangential to your patch (it has nothing to do with aliases,
and should be fixed regardless). But I wonder if your "!!" aliases will
expose this bug more frequently.

-Peff

PS I think your "!!" syntax conflicts with something like:

    git -c alias.has-changes="!! git diff --quiet' has-changes

   I don't know if that is worth worrying about or not. I also notice
   that using "!!git diff" with no space seems broken, as it seems to
   skip using the shell. I wonder if that is a bug in run-command.

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

[Jeff King]

On Thu, Oct 06, 2016 at 03:00:14PM -0400, Jeff King wrote:

> PS I think your "!!" syntax conflicts with something like:
> 
>     git -c alias.has-changes='!! git diff --quiet' has-changes
> 
>    I don't know if that is worth worrying about or not. I also notice
>    that using "!!git diff" with no space seems broken, as it seems to
>    skip using the shell. I wonder if that is a bug in run-command.

Nevermind this last bit. It is the shell that is complaining
(rightfully) about "!git"; the error message is just confusing because
it mentions $0, which contains the whole script:

  $ git -c alias.has-changes='!!git diff --quiet' has-changes
  !git diff --quiet: 1: !git diff --quiet: !git: not found

The "!! git diff" syntax sill has the conflict I mentioned (though note
I screwed up the quotes in what I wrote above).

-Peff