All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
@ 2010-05-05  0:25 Jared Hance
  2010-05-05  0:51 ` Jeff King
  0 siblings, 1 reply; 23+ messages in thread
From: Jared Hance @ 2010-05-05  0:25 UTC (permalink / raw)
  To: gitster; +Cc: git

The environment variable GIT_PATHNAME_PREFIX passes on the
current working directory (where the git command was called from)
to shell aliases (aliases that begin with "!"). This allows these
shell aliases to know the directory that the git command was called
from.

Signed-off-by: Jared Hance <jaredhance@gmail.com>
---
 git.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git.c b/git.c
index 6bae305..836f753 100644
--- a/git.c
+++ b/git.c
@@ -167,6 +167,9 @@ static int handle_alias(int *argcp, const char ***argv)
 				free(alias_string);
 				alias_string = buf.buf;
 			}
+			static char current_dir[PATH_MAX+1];
+			setenv("GIT_PATHNAME_PREFIX", getcwd(current_dir, sizeof(current_dir)), 1);
+
 			trace_printf("trace: alias to shell cmd: %s => %s\n",
 				     alias_command, alias_string + 1);
 			ret = system(alias_string + 1);
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
  2010-05-05  0:25 [PATCH] Set GIT_PATHNAME_PREFIX with aliases Jared Hance
@ 2010-05-05  0:51 ` Jeff King
  2010-05-05  6:53   ` Johannes Sixt
  0 siblings, 1 reply; 23+ messages in thread
From: Jeff King @ 2010-05-05  0:51 UTC (permalink / raw)
  To: Jared Hance; +Cc: gitster, git

On Tue, May 04, 2010 at 08:25:22PM -0400, Jared Hance wrote:

> The environment variable GIT_PATHNAME_PREFIX passes on the
> current working directory (where the git command was called from)
> to shell aliases (aliases that begin with "!"). This allows these
> shell aliases to know the directory that the git command was called
> from.

Seems like a reasonable goal, but...

> --- a/git.c
> +++ b/git.c
> @@ -167,6 +167,9 @@ static int handle_alias(int *argcp, const char ***argv)
>  				free(alias_string);
>  				alias_string = buf.buf;
>  			}
> +			static char current_dir[PATH_MAX+1];
> +			setenv("GIT_PATHNAME_PREFIX", getcwd(current_dir, sizeof(current_dir)), 1);
> +
>  			trace_printf("trace: alias to shell cmd: %s => %s\n",
>  				     alias_command, alias_string + 1);
>  			ret = system(alias_string + 1);

I see three problems:

  1. Don't declare variables in the middle of a function. It's a C99-ism
     that we avoid to retain portability with older compilers.

  2. On getcwd error, we setenv the value to NULL. Is that OK on all
     platforms (I am specifically thinking of our Windows *env wrappers,
     which have some restrictions, but I don't remember the details)?

  3. Most importantly, isn't this totally the wrong place to look at
     getcwd? We're just about to run system(), which means we will
     already have done our chdir() (which probably is the one happening
     in setup_git_directory). On even a simple test, it seems to always
     print the root of the repository for me.

     I think instead you want to pass in the prefix value computed by
     setup_git_directory to handle_alias.

-Peff

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
  2010-05-05  0:51 ` Jeff King
@ 2010-05-05  6:53   ` Johannes Sixt
  2010-05-05  7:01     ` Jeff King
  0 siblings, 1 reply; 23+ messages in thread
From: Johannes Sixt @ 2010-05-05  6:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Jared Hance, gitster, git

Am 5/5/2010 2:51, schrieb Jeff King:
> On Tue, May 04, 2010 at 08:25:22PM -0400, Jared Hance wrote:
> 
>> The environment variable GIT_PATHNAME_PREFIX passes on the
>> current working directory (where the git command was called from)
>> to shell aliases (aliases that begin with "!"). This allows these
>> shell aliases to know the directory that the git command was called
>> from.
> 
> Seems like a reasonable goal, but...

Sorry, I disagree.

The availability of this environment variable doesn't help the alias
writer a lot:

1. The alias is still burdened with the task to check *when* to use the
variable, i.e., whether an argument passed is absolute or relative, and
apply the variable only to a relative argument.

2. When more than one pathspec is passed to the alias, it is tedious to
apply $GIT_PATHNAME_PREFIX to each of them.

The only way where this variable could be used in a useful manner is to
write the alias as

   !cd "${GIT_PATHNAME_PREFIX:-.}" && { do stuff... ; }

which is something that git should do before it invokes the alias.

-- Hannes

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
  2010-05-05  6:53   ` Johannes Sixt
@ 2010-05-05  7:01     ` Jeff King
  2010-05-05  7:52       ` Eli Barzilay
  0 siblings, 1 reply; 23+ messages in thread
From: Jeff King @ 2010-05-05  7:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jared Hance, gitster, git

On Wed, May 05, 2010 at 08:53:35AM +0200, Johannes Sixt wrote:

> Am 5/5/2010 2:51, schrieb Jeff King:
> > On Tue, May 04, 2010 at 08:25:22PM -0400, Jared Hance wrote:
> > 
> >> The environment variable GIT_PATHNAME_PREFIX passes on the
> >> current working directory (where the git command was called from)
> >> to shell aliases (aliases that begin with "!"). This allows these
> >> shell aliases to know the directory that the git command was called
> >> from.
> > 
> > Seems like a reasonable goal, but...
> 
> Sorry, I disagree.
> [reasons why it sucks]

Yes, I agree it sucks. The problem is that this information is totally
lost now for shell aliases, so you can't even do these painful things.
Your alias simply doesn't have access to that information at all. I am
open to better interfaces (my "reasonable" above was not a ringing
endorsement, but rather "I can see why you might want to do this").

> The only way where this variable could be used in a useful manner is to
> write the alias as
> 
>    !cd "${GIT_PATHNAME_PREFIX:-.}" && { do stuff... ; }

Agreed that is the only sane thing to do with it, but...

> which is something that git should do before it invokes the alias.

Wouldn't we then be breaking existing aliases which do not expect this
new behavior?

-Peff

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
  2010-05-05  7:01     ` Jeff King
@ 2010-05-05  7:52       ` Eli Barzilay
  2010-05-05 16:07         ` Junio C Hamano
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05  7:52 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Jared Hance, gitster, git

On May  5, Jeff King wrote:
> On Wed, May 05, 2010 at 08:53:35AM +0200, Johannes Sixt wrote:
> 
> > Am 5/5/2010 2:51, schrieb Jeff King:
> > > On Tue, May 04, 2010 at 08:25:22PM -0400, Jared Hance wrote:
> > > 
> > >> The environment variable GIT_PATHNAME_PREFIX passes on the
> > >> current working directory (where the git command was called from)
> > >> to shell aliases (aliases that begin with "!"). This allows these
> > >> shell aliases to know the directory that the git command was called
> > >> from.
> > > 
> > > Seems like a reasonable goal, but...
> > 
> > Sorry, I disagree.
> > [reasons why it sucks]
> 
> Yes, I agree it sucks. The problem is that this information is
> totally lost now for shell aliases, so you can't even do these
> painful things.  Your alias simply doesn't have access to that
> information at all. I am open to better interfaces (my "reasonable"
> above was not a ringing endorsement, but rather "I can see why you
> might want to do this").

Something that Jonathan suggested earlier is a different magic
character instead of "!" that will do the cd -- perhaps a second
character would be more acceptable, something like "!!"...

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Set GIT_PATHNAME_PREFIX with aliases.
  2010-05-05  7:52       ` Eli Barzilay
@ 2010-05-05 16:07         ` Junio C Hamano
  2010-05-05 20:28           ` [PATCH] An alias that starts with "!!" runs in the current directory Eli Barzilay
  0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2010-05-05 16:07 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: Jeff King, Johannes Sixt, Jared Hance, git

Eli Barzilay <eli@barzilay.org> writes:

> On May  5, Jeff King wrote:
>> 
>> Yes, I agree it sucks.

I would not use such a strong word but I agree that it would have been
nicer if the original directory were used.

> Something that Jonathan suggested earlier is a different magic
> character instead of "!" that will do the cd -- perhaps a second
> character would be more acceptable, something like "!!"...

That sounds a reasonable compromise.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] An alias that starts with "!!" runs in the current directory.
  2010-05-05 16:07         ` Junio C Hamano
@ 2010-05-05 20:28           ` Eli Barzilay
  2010-05-05 20:52             ` [PATCH] An alias that starts with &quot;!!&quot; " Jared Hance
                               ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 20:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, Jared Hance, git

With "!"-prefixed shell aliases, the shell command not only gets
executed at the repository top level, but there is no way to know the
current directory of the original call.  This adds "!!"-prefixed aliases
as a similar variant for "!"-prefixed ones, but the commands are
executed in the original directory instead of the top level.

Signed-off-by: Eli Barzilay <eli@barzilay.org>
---

(Sending as a reply to the earlier message, keeping CCs.)

It looks like setup_git_directory_gently() returns the original CWD, but
since it's not documented or commented, I don't know if this is reliable
or not, so it might need to change.

Also, it might make more sense to document the "!!" variant first, since
it is generally more useful, but the way things evolved with "!!" being
the longer prefix, it seems to me that documenting it after "!" is more
sensible.

 Documentation/config.txt |    9 +++++++--
 git.c                    |    8 ++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 92f851e..055f4e3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -549,13 +549,18 @@ alias.*::
 	spaces, the usual shell quoting and escaping is supported.
 	quote pair and a backslash can be used to quote them.
 +
-If the alias expansion is prefixed with an exclamation point,
+If the alias expansion is prefixed with a single 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
+"gitk --all --not ORIG_HEAD".  Note that such shell commands will be
 executed from the top-level directory of a repository, which may
 not necessarily be the current directory.
++
+If the alias expansion is prefixed with two exclamation points,
+it will be treader similarly to the above, except that the shell commands
+are executed at the current directory.
+
 
 am.keepcr::
 	If true, git-am will call git-mailsplit for patches in mbox format
diff --git a/git.c b/git.c
index 6bae305..f3f8346 100644
--- a/git.c
+++ b/git.c
@@ -167,6 +167,14 @@ static int handle_alias(int *argcp, const char ***argv)
 				free(alias_string);
 				alias_string = buf.buf;
 			}
+			/* going to exit anyway, so it's fine to change
+			 * alias_string to the actual command */
+			alias_string += 1;
+			if (alias_string[0] == '!') {
+				alias_string += 1;
+				if (subdir && chdir(subdir))
+					die_errno("Cannot change to '%s'", subdir);
+			}
 			trace_printf("trace: alias to shell cmd: %s => %s\n",
 				     alias_command, alias_string + 1);
 			ret = system(alias_string + 1);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] An alias that starts with &quot;!!&quot; runs in the current directory.
  2010-05-05 20:28           ` [PATCH] An alias that starts with "!!" runs in the current directory Eli Barzilay
@ 2010-05-05 20:52             ` Jared Hance
  2010-05-05 20:58               ` Eli Barzilay
  2010-05-05 21:31             ` [PATCH v2] An alias that starts with "!!" runs in the current directory Eli Barzilay
  2010-05-05 22:02             ` [PATCH] " Junio C Hamano
  2 siblings, 1 reply; 23+ messages in thread
From: Jared Hance @ 2010-05-05 20:52 UTC (permalink / raw)
  To: git

Eli Barzilay <eli <at> barzilay.org> writes:

>  			ret = system(alias_string + 1);

I don't think that this is correct. You already changed alias_string to the
actual code earlier in the command, so I think that this will actually chop off
the first letter of the command.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] An alias that starts with &quot;!!&quot; runs in the current directory.
  2010-05-05 20:52             ` [PATCH] An alias that starts with &quot;!!&quot; " Jared Hance
@ 2010-05-05 20:58               ` Eli Barzilay
  2010-05-05 21:00                 ` Eli Barzilay
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 20:58 UTC (permalink / raw)
  To: Jared Hance; +Cc: git

On May  5, Jared Hance wrote:
> Eli Barzilay <eli <at> barzilay.org> writes:
> 
> >  			ret = system(alias_string + 1);
> 
> I don't think that this is correct. You already changed alias_string
> to the actual code earlier in the command, so I think that this will
> actually chop off the first letter of the command.

(*sigh*)  That's correct, I fixed this, but improperly remade the
patch.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] An alias that starts with &quot;!!&quot; runs in the current directory.
  2010-05-05 20:58               ` Eli Barzilay
@ 2010-05-05 21:00                 ` Eli Barzilay
  2010-05-05 21:24                   ` Re-submitting patches Jonathan Nieder
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 21:00 UTC (permalink / raw)
  To: Jared Hance, git

On May  5, Eli Barzilay wrote:
> On May  5, Jared Hance wrote:
> > Eli Barzilay <eli <at> barzilay.org> writes:
> > 
> > >  			ret = system(alias_string + 1);
> > 
> > I don't think that this is correct. You already changed alias_string
> > to the actual code earlier in the command, so I think that this will
> > actually chop off the first letter of the command.
> 
> (*sigh*)  That's correct, I fixed this, but improperly remade the
> patch.

Is there some convention for sending a fixed patch?

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re-submitting patches
  2010-05-05 21:00                 ` Eli Barzilay
@ 2010-05-05 21:24                   ` Jonathan Nieder
  2010-05-05 21:28                     ` Eli Barzilay
  0 siblings, 1 reply; 23+ messages in thread
From: Jonathan Nieder @ 2010-05-05 21:24 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: Jared Hance, git

Hi Eli,

Eli Barzilay wrote:
> On May  5, Eli Barzilay wrote:

>> (*sigh*)  That's correct, I fixed this, but improperly remade the
>> patch.
>
> Is there some convention for sending a fixed patch?

With a small patch like this one, you can just send the fixed patch
as a reply to the thread.  Putting “[PATCH v2]” in the subject would
make it clear that this is the newer and better version.

With larger patches, doing that too often can overload people.
My preferred solution: describe the changes as soon as you want, but
then take some time to polish them before resubmitting.  This gives
people time to breathe. ;-)

The “ideal patch flow” section in Documentation/SubmittingPatches
says:

-------
 (3) Polish, refine, and re-send to the list and the people who
     spend their time to improve your patch.  Go back to step (2).
-------

Maybe this could be clearer.  In particular, sometimes it is not
obvious to people that even the patch submitter can pretend to be a
reviewer and discuss small incremental changes.

Thanks for bringing it up,
Jonathan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Re-submitting patches
  2010-05-05 21:24                   ` Re-submitting patches Jonathan Nieder
@ 2010-05-05 21:28                     ` Eli Barzilay
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 21:28 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Jared Hance, git

On May  5, Jonathan Nieder wrote:
> Hi Eli,
> 
> Eli Barzilay wrote:
> > On May  5, Eli Barzilay wrote:
> 
> >> (*sigh*)  That's correct, I fixed this, but improperly remade the
> >> patch.
> >
> > Is there some convention for sending a fixed patch?
> 
> With a small patch like this one, you can just send the fixed patch
> as a reply to the thread.  Putting “[PATCH v2]” in the subject would
> make it clear that this is the newer and better version.

Ah -- it's the "v2" that I missed, thanks.


> With larger patches, doing that too often can overload people.
> My preferred solution: describe the changes as soon as you want, but
> then take some time to polish them before resubmitting.  This gives
> people time to breathe. ;-)

Yes, I'd be much more careful if this was not a quick thing.  (BTW, it
would be nice if there is a way to get various publishing commands
like `format-patch' and `pull' to spit out some warning if there are
uncommitted changes -- that would have prevented me from sending the
bogus one.)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 20:28           ` [PATCH] An alias that starts with "!!" runs in the current directory Eli Barzilay
  2010-05-05 20:52             ` [PATCH] An alias that starts with &quot;!!&quot; " Jared Hance
@ 2010-05-05 21:31             ` Eli Barzilay
  2010-05-05 22:22               ` Will Palmer
  2010-05-05 22:02             ` [PATCH] " Junio C Hamano
  2 siblings, 1 reply; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 21:31 UTC (permalink / raw)
  To: Junio C Hamano, Jeff King, Johannes Sixt, Jared Hance, git

With "!"-prefixed shell aliases, the shell command not only gets
executed at the repository top level, but there is no way to know the
current directory of the original call.  This adds "!!"-prefixed aliases
as a similar variant for "!"-prefixed ones, but the commands are
executed in the original directory instead of the top level.

Signed-off-by: Eli Barzilay <eli@barzilay.org>
---

It looks like setup_git_directory_gently() returns the original CWD, but
since it's not documented or commented, I don't know if this is reliable
or not, so it might need to change.

Also, it might make more sense to document the "!!" variant first, since
it is generally more useful, but the way things evolved with "!!" being
the longer prefix, it seems to me that documenting it after "!" is more
sensible.

This fixes the broken use of alias_command that Jared Hance caught.

 Documentation/config.txt |    9 +++++++--
 git.c                    |   14 +++++++++++---
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 92f851e..055f4e3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -549,13 +549,18 @@ alias.*::
 	spaces, the usual shell quoting and escaping is supported.
 	quote pair and a backslash can be used to quote them.
 +
-If the alias expansion is prefixed with an exclamation point,
+If the alias expansion is prefixed with a single 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
+"gitk --all --not ORIG_HEAD".  Note that such shell commands will be
 executed from the top-level directory of a repository, which may
 not necessarily be the current directory.
++
+If the alias expansion is prefixed with two exclamation points,
+it will be treader similarly to the above, except that the shell commands
+are executed at the current directory.
+
 
 am.keepcr::
 	If true, git-am will call git-mailsplit for patches in mbox format
diff --git a/git.c b/git.c
index 6bae305..3d8ed20 100644
--- a/git.c
+++ b/git.c
@@ -167,14 +167,22 @@ static int handle_alias(int *argcp, const char ***argv)
 				free(alias_string);
 				alias_string = buf.buf;
 			}
+			/* going to exit anyway, so it's fine to change
+			 * alias_string to the actual command */
+			alias_string += 1;
+			if (alias_string[0] == '!') {
+				alias_string += 1;
+				if (subdir && chdir(subdir))
+					die_errno("Cannot change to '%s'", subdir);
+			}
 			trace_printf("trace: alias to shell cmd: %s => %s\n",
-				     alias_command, alias_string + 1);
-			ret = system(alias_string + 1);
+				     alias_command, alias_string);
+			ret = system(alias_string);
 			if (ret >= 0 && WIFEXITED(ret) &&
 			    WEXITSTATUS(ret) != 127)
 				exit(WEXITSTATUS(ret));
 			die("Failed to run '%s' when expanding alias '%s'",
-			    alias_string + 1, alias_command);
+			    alias_string, alias_command);
 		}
 		count = split_cmdline(alias_string, &new_argv);
 		if (count < 0)
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] An alias that starts with "!!" runs in the current directory.
  2010-05-05 20:28           ` [PATCH] An alias that starts with "!!" runs in the current directory Eli Barzilay
  2010-05-05 20:52             ` [PATCH] An alias that starts with &quot;!!&quot; " Jared Hance
  2010-05-05 21:31             ` [PATCH v2] An alias that starts with "!!" runs in the current directory Eli Barzilay
@ 2010-05-05 22:02             ` Junio C Hamano
  2010-05-05 22:13               ` Eli Barzilay
  2 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2010-05-05 22:02 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: Junio C Hamano, Jeff King, Johannes Sixt, Jared Hance, git

Eli Barzilay <eli@barzilay.org> writes:

> It looks like setup_git_directory_gently() returns the original CWD,...

It is designed to return what we internally call "prefix".  You however
have to be careful as it can return NULL or an empty string when you are
already at the top of the working tree.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] An alias that starts with "!!" runs in the current  directory.
  2010-05-05 22:02             ` [PATCH] " Junio C Hamano
@ 2010-05-05 22:13               ` Eli Barzilay
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 22:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Sixt, Jared Hance, git

On May  5, Junio C Hamano wrote:
> Eli Barzilay <eli@barzilay.org> writes:
> 
> > It looks like setup_git_directory_gently() returns the original CWD,...
> 
> It is designed to return what we internally call "prefix".  You
> however have to be careful as it can return NULL or an empty string
> when you are already at the top of the working tree.

OK, in this case it looks like it should work as expected.  At least
provided that chdir("") stays in the same directory -- and given the
call at the end of handle_alias(), it looks like this assumption is
already made.  Does this sound right?

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 21:31             ` [PATCH v2] An alias that starts with "!!" runs in the current directory Eli Barzilay
@ 2010-05-05 22:22               ` Will Palmer
  2010-05-05 22:33                 ` Eli Barzilay
  2010-05-05 23:43                 ` Junio C Hamano
  0 siblings, 2 replies; 23+ messages in thread
From: Will Palmer @ 2010-05-05 22:22 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: Junio C Hamano, Jeff King, Johannes Sixt, Jared Hance, git

On Wed, 2010-05-05 at 17:31 -0400, Eli Barzilay wrote:
> With "!"-prefixed shell aliases, the shell command not only gets
> executed at the repository top level, but there is no way to know the
> current directory of the original call.  This adds "!!"-prefixed aliases
> as a similar variant for "!"-prefixed ones, but the commands are
> executed in the original directory instead of the top level.
> 
> Signed-off-by: Eli Barzilay <eli@barzilay.org>

Is there any precedent for the "!!" syntax? Something like ".!", "./!",
or "!(.)" would make the intention more clear, I'd think, as well as
leaving room for other extensions to be added later, and some
explicit-opposite, like "/!", to complement it. (It's not like people
are going around with binaries called "!", is it?)

Ignore my suggestions if it turns out that "!!" is used by other things,
but this one seems to be begging for someone else to come along with a
"me too!"


-- 
-- Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 22:22               ` Will Palmer
@ 2010-05-05 22:33                 ` Eli Barzilay
  2010-05-05 23:43                 ` Junio C Hamano
  1 sibling, 0 replies; 23+ messages in thread
From: Eli Barzilay @ 2010-05-05 22:33 UTC (permalink / raw)
  To: wmpalmer; +Cc: Junio C Hamano, Jeff King, Johannes Sixt, Jared Hance, git

On May  5, Will Palmer wrote:
> On Wed, 2010-05-05 at 17:31 -0400, Eli Barzilay wrote:
> > With "!"-prefixed shell aliases, the shell command not only gets
> > executed at the repository top level, but there is no way to know the
> > current directory of the original call.  This adds "!!"-prefixed aliases
> > as a similar variant for "!"-prefixed ones, but the commands are
> > executed in the original directory instead of the top level.
> > 
> > Signed-off-by: Eli Barzilay <eli@barzilay.org>
> 
> Is there any precedent for the "!!" syntax? Something like ".!",
> "./!", or "!(.)" would make the intention more clear, I'd think, as
> well as leaving room for other extensions to be added later, and
> some explicit-opposite, like "/!", to complement it. (It's not like
> people are going around with binaries called "!", is it?)

[

The first time I saw the "!" syntax, it was slightly confusing to me
in that it's similar to various shells using "!" for history expansion
-- especially given that these things are popular with shell aliases
made it hard to remember.

An alternative character that wouldn't have confused me, and even
suggests shell commands is ";".  So my preference would be for shell
aliases to start with it and to be invoked in the original working
directory, perhaps deprecating the "!" syntax.

But I'm new enough to be safely ignored...  So this is all a
parenthetical comment...

]

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 22:22               ` Will Palmer
  2010-05-05 22:33                 ` Eli Barzilay
@ 2010-05-05 23:43                 ` Junio C Hamano
  2010-05-06  0:05                   ` Adam Brewster
                                     ` (3 more replies)
  1 sibling, 4 replies; 23+ messages in thread
From: Junio C Hamano @ 2010-05-05 23:43 UTC (permalink / raw)
  To: wmpalmer; +Cc: Eli Barzilay, Jeff King, Johannes Sixt, Jared Hance, git

Will Palmer <wmpalmer@gmail.com> writes:

> Is there any precedent for the "!!" syntax? Something like ".!", "./!",
> or "!(.)" would make the intention more clear, I'd think, as well as
> leaving room for other extensions to be added later...

While I don't think !! is particularly good (or bad), possible future
extension is always a good reason to think about the notation carefully.

It probably is a good idea to switch aliases to start at the $cwd in 1.8.0
(or perhaps one major release after it), and using a notation that is more
descriptive, something like "!(cwd)" vs "!(root)", may give us a better
transtion strategy than casting cryptic "!!" in the stone.

What other variants might we want to be able to specify while defining and
using aliases?  If cwd vs root is the only distinction, then !(cwd) would
be a bit overkill, but if we used !! for this new feature, I suspect that
it would make it much harder to switch the default in the future.

Comments?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current  directory.
  2010-05-05 23:43                 ` Junio C Hamano
@ 2010-05-06  0:05                   ` Adam Brewster
  2010-05-06  6:21                     ` Will Palmer
  2010-05-06  6:26                   ` Will Palmer
                                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 23+ messages in thread
From: Adam Brewster @ 2010-05-06  0:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: wmpalmer, Eli Barzilay, Jeff King, Johannes Sixt, Jared Hance, git

On 5/5/10, Junio C Hamano <gitster@pobox.com> wrote:
>
> It probably is a good idea to switch aliases to start at the $cwd in 1.8.0
> (or perhaps one major release after it), and using a notation that is more
> descriptive, something like "!(cwd)" vs "!(root)", may give us a better
> transtion strategy than casting cryptic "!!" in the stone.
>

For what it's worth, I like "!cmd".

Would it be good enough to set some environment variables?

    [alias]
    command_in_cwd = "!cd $GIT_CWD; ..."

Adam

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current  directory.
  2010-05-06  0:05                   ` Adam Brewster
@ 2010-05-06  6:21                     ` Will Palmer
  0 siblings, 0 replies; 23+ messages in thread
From: Will Palmer @ 2010-05-06  6:21 UTC (permalink / raw)
  To: Adam Brewster
  Cc: Junio C Hamano, Eli Barzilay, Jeff King, Johannes Sixt, Jared Hance, git

On Wed, 2010-05-05 at 20:05 -0400, Adam Brewster wrote:
> On 5/5/10, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > It probably is a good idea to switch aliases to start at the $cwd in 1.8.0
> > (or perhaps one major release after it), and using a notation that is more
> > descriptive, something like "!(cwd)" vs "!(root)", may give us a better
> > transtion strategy than casting cryptic "!!" in the stone.
> >
> 
> For what it's worth, I like "!cmd".
> 
> Would it be good enough to set some environment variables?
> 
>     [alias]
>     command_in_cwd = "!cd $GIT_CWD; ..."
> 
> Adam

I'd be tempted to say that setting $GIT_CWD was an ideal solution, if
Junio hadn't mentioned the possibility of cwd being the default in
1.8.0, which to me sounds much more sane than defaulting to the
repository root. Defaulting to the root has never made sense to me and
has some perfectly good built-ins to emulate if needed. I know I have at
least two scripts-which-should-be-aliases sitting around because of this
restriction.

-- 
-- Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 23:43                 ` Junio C Hamano
  2010-05-06  0:05                   ` Adam Brewster
@ 2010-05-06  6:26                   ` Will Palmer
  2010-05-06  6:36                   ` Johannes Sixt
  2010-05-06  7:02                   ` Matthieu Moy
  3 siblings, 0 replies; 23+ messages in thread
From: Will Palmer @ 2010-05-06  6:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eli Barzilay, Jeff King, Johannes Sixt, Jared Hance, git

On Wed, 2010-05-05 at 16:43 -0700, Junio C Hamano wrote:
> What other variants might we want to be able to specify while defining and
> using aliases?  If cwd vs root is the only distinction, then !(cwd) would
> be a bit overkill, but if we used !! for this new feature, I suspect that
> it would make it much harder to switch the default in the future.
> 
> Comments?
> 
> 

What I had in mind when I mentioned !(...)  were some half-formed ideas
about shortcuts for manipulating the environment, !(nopager), for
example.

But these ideas were vague and I can't think of any off-hand which I
would use other than !(cwd). I'd say !(root:path/to/dir), but that's
obviously not necessary (any more than !(root) would be, if cwd were the
default), and honestly I'm quite happy using shell functions for that
sort of thing.

-- 
-- Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 23:43                 ` Junio C Hamano
  2010-05-06  0:05                   ` Adam Brewster
  2010-05-06  6:26                   ` Will Palmer
@ 2010-05-06  6:36                   ` Johannes Sixt
  2010-05-06  7:02                   ` Matthieu Moy
  3 siblings, 0 replies; 23+ messages in thread
From: Johannes Sixt @ 2010-05-06  6:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: wmpalmer, Eli Barzilay, Jeff King, Jared Hance, git

Am 5/6/2010 1:43, schrieb Junio C Hamano:
> It probably is a good idea to switch aliases to start at the $cwd in 1.8.0
> (or perhaps one major release after it), and using a notation that is more
> descriptive, something like "!(cwd)" vs "!(root)", may give us a better
> transtion strategy than casting cryptic "!!" in the stone.

As always, we should start with the verbose tag, and if one turns out to
be used frequently, use !! for it as a shortcut.

> What other variants might we want to be able to specify while defining and
> using aliases?  If cwd vs root is the only distinction, then !(cwd) would
> be a bit overkill, but if we used !! for this new feature, I suspect that
> it would make it much harder to switch the default in the future.

The syntax should use a symbol that is less likely to appear at the start
of a shell script. Perhaps "!@cwd".

-- Hannes

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] An alias that starts with "!!" runs in the current directory.
  2010-05-05 23:43                 ` Junio C Hamano
                                     ` (2 preceding siblings ...)
  2010-05-06  6:36                   ` Johannes Sixt
@ 2010-05-06  7:02                   ` Matthieu Moy
  3 siblings, 0 replies; 23+ messages in thread
From: Matthieu Moy @ 2010-05-06  7:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: wmpalmer, Eli Barzilay, Jeff King, Johannes Sixt, Jared Hance, git

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

> It probably is a good idea to switch aliases to start at the $cwd in 1.8.0
> (or perhaps one major release after it), and using a notation that is more
> descriptive, something like "!(cwd)" vs "!(root)", may give us a better
> transtion strategy than casting cryptic "!!" in the stone.

I like this: Allow !(cwd) and !(root) today, with !(root) being just
like "!", and change plain "!" to mean !(cwd) in a future major
release.

If other people have ideas for other features, the syntax can also be
extended to !(cwd,some-option) or so.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2010-05-06  7:03 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-05  0:25 [PATCH] Set GIT_PATHNAME_PREFIX with aliases Jared Hance
2010-05-05  0:51 ` Jeff King
2010-05-05  6:53   ` Johannes Sixt
2010-05-05  7:01     ` Jeff King
2010-05-05  7:52       ` Eli Barzilay
2010-05-05 16:07         ` Junio C Hamano
2010-05-05 20:28           ` [PATCH] An alias that starts with "!!" runs in the current directory Eli Barzilay
2010-05-05 20:52             ` [PATCH] An alias that starts with &quot;!!&quot; " Jared Hance
2010-05-05 20:58               ` Eli Barzilay
2010-05-05 21:00                 ` Eli Barzilay
2010-05-05 21:24                   ` Re-submitting patches Jonathan Nieder
2010-05-05 21:28                     ` Eli Barzilay
2010-05-05 21:31             ` [PATCH v2] An alias that starts with "!!" runs in the current directory Eli Barzilay
2010-05-05 22:22               ` Will Palmer
2010-05-05 22:33                 ` Eli Barzilay
2010-05-05 23:43                 ` Junio C Hamano
2010-05-06  0:05                   ` Adam Brewster
2010-05-06  6:21                     ` Will Palmer
2010-05-06  6:26                   ` Will Palmer
2010-05-06  6:36                   ` Johannes Sixt
2010-05-06  7:02                   ` Matthieu Moy
2010-05-05 22:02             ` [PATCH] " Junio C Hamano
2010-05-05 22:13               ` Eli Barzilay

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.