{"thread":{"id":"27087","subject":"[RFC/PATCH] handle_alias: provide GIT_CWD to !alias","startedAt":"2011-04-13T13:05:20Z","lastAt":"2011-04-27T09:36:28Z","messageCount":12,"participants":["Michael J Gruber","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"165745","messageId":"e8c9aa9160f922f728d56387e5e86eb50220774f.1302699792.git.git@drmicha.warpmail.net","threadId":"27087","inReplyTo":"4DA59B27.50506@ge.infn.it","subject":"[RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-13T13:05:20Z","receivedAt":"2011-04-13T13:05:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Provide an environment variable GIT_CWD which contains the subdirectory\nfrom which a !alias was called since these cd to the to level directory\nbefore they are executed.\n\nSigned-off-by: Michael J Gruber \n---\nRFC for lack of tests and doc.\n\nI would say this is something realistic and useful which can be done now, no\nmatter how our efforts with !(top) and the like go.\n\n git.c | 10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex ef598c3..c8b9189 100644\n--- a/git.c\n+++ b/git.c\n@@ -179,6 +179,8 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\tif (alias_string[0] == '!') {\n \t\t\tconst char **alias_argv;\n \t\t\tint argc = *argcp, i;\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tconst char *env[2];\n \n \t\t\tcommit_pager_choice();\n \n@@ -189,7 +191,13 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\t\t\talias_argv[i] = (*argv)[i];\n \t\t\talias_argv[argc] = NULL;\n \n-\t\t\tret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n+\t\t\tstrbuf_addstr(&sb, \"GIT_CWD=\");\n+\t\t\tif (subdir)\n+\t\t\t\tstrbuf_addstr(&sb, subdir);\n+\t\t\tenv[0] = sb.buf;\n+\t\t\tenv[1] = NULL;\n+\t\t\tret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n+\t\t\tstrbuf_release(&sb);\n \t\t\tif (ret >= 0) /* normal exit */\n \t\t\t\texit(ret);\n \n-- \n1.7.5.rc0.365.g23929a\n"},{"id":"165746","messageId":"BANLkTimubmaV437LvdG8QcmFFjyWujQRzw@mail.gmail.com","threadId":"27087","inReplyTo":"e8c9aa9160f922f728d56387e5e86eb50220774f.1302699792.git.git@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-13T13:12:40Z","receivedAt":"2011-04-13T13:12:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber\n wrote:\n> -                       ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n> +                       strbuf_addstr(&sb, \"GIT_CWD=\");\n> +                       if (subdir)\n> +                               strbuf_addstr(&sb, subdir);\n> +                       env[0] = sb.buf;\n> +                       env[1] = NULL;\n> +                       ret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n> +                       strbuf_release(&sb);\n>                        if (ret >= 0)   /* normal exit */\n>                                exit(ret);\n\nsubdir can be NULL. I'm not sure if it can be empty string though. May\nneed a check and set it to '.' so \"cd $GIT_CWD\" does not go back to\n$HOME.\n-- \nDuy\n"},{"id":"165747","messageId":"4DA5A228.2040105@drmicha.warpmail.net","threadId":"27087","inReplyTo":"BANLkTimubmaV437LvdG8QcmFFjyWujQRzw@mail.gmail.com","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-13T13:16:24Z","receivedAt":"2011-04-13T13:16:24Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 13.04.2011 15:12:\n> On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber\n> wrote:\n>> - ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n>> + strbuf_addstr(&sb, \"GIT_CWD=\");\n>> + if (subdir)\n>> + strbuf_addstr(&sb, subdir);\n>> + env[0] = sb.buf;\n>> + env[1] = NULL;\n>> + ret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n>> + strbuf_release(&sb);\n>> if (ret >= 0) /* normal exit */\n>> exit(ret);\n> \n> subdir can be NULL. I'm not sure if it can be empty string though. May\n\nI'm pretty sure that my \"if (subdir)\" would catch that...\n\n> need a check and set it to '.' so \"cd $GIT_CWD\" does not go back to\n> $HOME.\n\nWell, with the current implementation you get empty or the subdir so\nthat you can easily \"test -n $GIT_CWD\". If you want to cd around you can\ndo \"cd ./$GIT_CWD\" unconditionally. I think this is more useful than\nhaving a \".\" there.\n\nMichael\n"},{"id":"165750","messageId":"BANLkTikKyJb283ZTaX0q=BYkCtp_48Yocg@mail.gmail.com","threadId":"27087","inReplyTo":"4DA5A228.2040105@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-13T13:24:44Z","receivedAt":"2011-04-13T13:24:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 13, 2011 at 8:16 PM, Michael J Gruber\n wrote:\n> Nguyen Thai Ngoc Duy venit, vidit, dixit 13.04.2011 15:12:\n>> On Wed, Apr 13, 2011 at 8:05 PM, Michael J Gruber\n>> wrote:\n>>> -                       ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n>>> +                       strbuf_addstr(&sb, \"GIT_CWD=\");\n>>> +                       if (subdir)\n>>> +                               strbuf_addstr(&sb, subdir);\n>>> +                       env[0] = sb.buf;\n>>> +                       env[1] = NULL;\n>>> +                       ret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n>>> +                       strbuf_release(&sb);\n>>>                        if (ret >= 0)   /* normal exit */\n>>>                                exit(ret);\n>>\n>> subdir can be NULL. I'm not sure if it can be empty string though. May\n>\n> I'm pretty sure that my \"if (subdir)\" would catch that...\n\nYes. I skimmed too quick and realized after sending my email.\n\n>> need a check and set it to '.' so \"cd $GIT_CWD\" does not go back to\n>> $HOME.\n>\n> Well, with the current implementation you get empty or the subdir so\n> that you can easily \"test -n $GIT_CWD\". If you want to cd around you can\n> do \"cd ./$GIT_CWD\" unconditionally. I think this is more useful than\n> having a \".\" there.\n\nIn that case should the variable have a another name? CWD/PWD is\nusually absolute path. \"cd ./$GIT_CWD\" looks odd to me. Or you can\ngetcwd() and set $OLDPWD.\n-- \nDuy\n"},{"id":"166416","messageId":"7v1v0od557.fsf@alter.siamese.dyndns.org","threadId":"27087","inReplyTo":"e8c9aa9160f922f728d56387e5e86eb50220774f.1302699792.git.git@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-26T22:22:44Z","receivedAt":"2011-04-26T22:22:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Provide an environment variable GIT_CWD which contains the subdirectory\n> from which a !alias was called since these cd to the to level directory\n> before they are executed.\n\nIsn't this what we usually call \"prefix\", though?\n"},{"id":"166463","messageId":"4DB7BBEF.8010409@drmicha.warpmail.net","threadId":"27087","inReplyTo":"7v1v0od557.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-27T06:47:11Z","receivedAt":"2011-04-27T06:47:11Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 27.04.2011 00:22:\n> Michael J Gruber writes:\n> \n>> Provide an environment variable GIT_CWD which contains the subdirectory\n>> from which a !alias was called since these cd to the to level directory\n>> before they are executed.\n> \n> Isn't this what we usually call \"prefix\", though?\n\nIn code it is called \"prefix\". But I have to ask: So what? Are you\nsuggesting GIT_PREFIX? User expectation is probably more tied to CWD,\nunless a user knows \"rev-parse --show-prefix\". I don't care about the\nname, and GIT_PREFIX may be more systematic.\n\nA more important point is the prefix-less case (i.e. we are in topdir):\nShould GIT_YOUNAMEIT be empty then? I would say yes (just like\n\"rev-parse --show-prefix\"), Duy said no. We need a third vote :)\n\nAlso, we may want to scrap this altogether and do !!-aliases instead\n(without chdir).\n\nMichael\n"},{"id":"166467","messageId":"BANLkTinEiT1_8czdY2DtBnY-5CqN1XAVcw@mail.gmail.com","threadId":"27087","inReplyTo":"4DB7BBEF.8010409@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-27T07:40:00Z","receivedAt":"2011-04-27T07:40:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 27, 2011 at 1:47 PM, Michael J Gruber\n wrote:\n> A more important point is the prefix-less case (i.e. we are in topdir):\n> Should GIT_YOUNAMEIT be empty then? I would say yes (just like\n> \"rev-parse --show-prefix\"), Duy said no. We need a third vote :)\n\nI said no because you named it GIT_CWD. If it's GIT_PREFIX, then empty\nprefix is normal, no objections.\n-- \nDuy\n"},{"id":"166469","messageId":"4DB7CF72.3080205@drmicha.warpmail.net","threadId":"27087","inReplyTo":"BANLkTinEiT1_8czdY2DtBnY-5CqN1XAVcw@mail.gmail.com","subject":"Re: [RFC/PATCH] handle_alias: provide GIT_CWD to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-27T08:10:26Z","receivedAt":"2011-04-27T08:10:26Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 27.04.2011 09:40:\n> On Wed, Apr 27, 2011 at 1:47 PM, Michael J Gruber\n> wrote:\n>> A more important point is the prefix-less case (i.e. we are in topdir):\n>> Should GIT_YOUNAMEIT be empty then? I would say yes (just like\n>> \"rev-parse --show-prefix\"), Duy said no. We need a third vote :)\n> \n> I said no because you named it GIT_CWD. If it's GIT_PREFIX, then empty\n> prefix is normal, no objections.\n\nWe have a winner then! Patch upcoming.\n\nMichael\n"},{"id":"166480","messageId":"3422485b672000ecdc72d82ffce8c89f258831b9.1303893279.git.git@drmicha.warpmail.net","threadId":"27087","inReplyTo":"4DB7CF72.3080205@drmicha.warpmail.net","subject":"[PATCH 1/2] t1020: test !alias in subdirectory","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-27T08:36:26Z","receivedAt":"2011-04-27T08:36:26Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Signed-off-by: Michael J Gruber \n---\n t/t1020-subdirectory.sh | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh\nindex 1fd187c..66e40d3 100755\n--- a/t/t1020-subdirectory.sh\n+++ b/t/t1020-subdirectory.sh\n@@ -118,6 +118,17 @@ test_expect_success 'alias expansion' '\n \t\tgit ss\n \t)\n '\n+\n+test_expect_success '!alias expansion' '\n+\tpwd >expect &&\n+\t(\n+\t\tgit config alias.test !pwd &&\n+\t\tcd dir &&\n+\t\tgit test >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'no file/rev ambiguity check inside .git' '\n \tgit commit -a -m 1 &&\n \t(\n-- \n1.7.5.270.gafca7\n"},{"id":"166481","messageId":"5058a9853a75bff71187f553e67632997761de44.1303893279.git.git@drmicha.warpmail.net","threadId":"27087","inReplyTo":"4DB7CF72.3080205@drmicha.warpmail.net","subject":"[PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-27T08:36:27Z","receivedAt":"2011-04-27T08:36:27Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Provide an environment variable GIT_PREFIX which contains the subdirectory\nfrom which a !alias was called (i.e. 'git rev-parse --show-prefix') since\nthese cd to the to level directory before they are executed.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/config.txt | 2 ++\n git.c | 10 +++++++++-\n t/t1020-subdirectory.sh | 10 ++++++++++\n 3 files changed, 21 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 750c86d..66281dd 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -589,6 +589,8 @@ it will be treated as a shell command. For example, defining\n \"gitk --all --not ORIG_HEAD\". Note that shell commands will be\n executed from the top-level directory of a repository, which may\n not necessarily be the current directory.\n+'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'\n+from the original current directory. See linkgit:git-rev-parse[1].\n \n am.keepcr::\n \tIf true, git-am will call git-mailsplit for patches in mbox format\ndiff --git a/git.c b/git.c\nindex ef598c3..ed89951 100644\n--- a/git.c\n+++ b/git.c\n@@ -179,6 +179,8 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\tif (alias_string[0] == '!') {\n \t\t\tconst char **alias_argv;\n \t\t\tint argc = *argcp, i;\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tconst char *env[2];\n \n \t\t\tcommit_pager_choice();\n \n@@ -189,7 +191,13 @@ static int handle_alias(int *argcp, const char ***argv)\n \t\t\t\talias_argv[i] = (*argv)[i];\n \t\t\talias_argv[argc] = NULL;\n \n-\t\t\tret = run_command_v_opt(alias_argv, RUN_USING_SHELL);\n+\t\t\tstrbuf_addstr(&sb, \"GIT_PREFIX=\");\n+\t\t\tif (subdir)\n+\t\t\t\tstrbuf_addstr(&sb, subdir);\n+\t\t\tenv[0] = sb.buf;\n+\t\t\tenv[1] = NULL;\n+\t\t\tret = run_command_v_opt_cd_env(alias_argv, RUN_USING_SHELL, NULL, env);\n+\t\t\tstrbuf_release(&sb);\n \t\t\tif (ret >= 0) /* normal exit */\n \t\t\t\texit(ret);\n \ndiff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh\nindex 66e40d3..ddc3921 100755\n--- a/t/t1020-subdirectory.sh\n+++ b/t/t1020-subdirectory.sh\n@@ -129,6 +129,16 @@ test_expect_success '!alias expansion' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'GIT_PREFIX for !alias' '\n+\tprintf \"dir/\" >expect &&\n+\t(\n+\t\tgit config alias.test \"!sh -c \\\"printf \\$GIT_PREFIX\\\"\" &&\n+\t\tcd dir &&\n+\t\tgit test >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'no file/rev ambiguity check inside .git' '\n \tgit commit -a -m 1 &&\n \t(\n-- \n1.7.5.270.gafca7\n"},{"id":"166486","messageId":"BANLkTi=gjuOqLEArk_0Nj1faEu62dzjbZA@mail.gmail.com","threadId":"27087","inReplyTo":"5058a9853a75bff71187f553e67632997761de44.1303893279.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-27T09:06:09Z","receivedAt":"2011-04-27T09:06:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/4/27 Michael J Gruber :\n> @@ -589,6 +589,8 @@ it will be treated as a shell command.  For example, defining\n>  \"gitk --all --not ORIG_HEAD\".  Note that shell commands will be\n>  executed from the top-level directory of a repository, which may\n>  not necessarily be the current directory.\n> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'\n> +from the original current directory. See linkgit:git-rev-parse[1].\n\nIt made me wonder \"but then why should not we just do GIT_PREFIX=`git\nrev-parse --show-prefix` in the alias?\" then realized by the time we\ndo that, it's already too late. Maybe\n\n> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'\n> +before current directory is changed to top-level directory. See linkgit:git-rev-parse[1].\n\n?\n-- \nDuy\n"},{"id":"166489","messageId":"4DB7E39C.1040609@drmicha.warpmail.net","threadId":"27087","inReplyTo":"BANLkTi=gjuOqLEArk_0Nj1faEu62dzjbZA@mail.gmail.com","subject":"Re: [PATCH 2/2] handle_alias: provide GIT_PREFIX to !alias","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-27T09:36:28Z","receivedAt":"2011-04-27T09:36:28Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 27.04.2011 11:06:\n> 2011/4/27 Michael J Gruber :\n>> @@ -589,6 +589,8 @@ it will be treated as a shell command. For example, defining\n>> \"gitk --all --not ORIG_HEAD\". Note that shell commands will be\n>> executed from the top-level directory of a repository, which may\n>> not necessarily be the current directory.\n>> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'\n>> +from the original current directory. See linkgit:git-rev-parse[1].\n> \n> It made me wonder \"but then why should not we just do GIT_PREFIX=`git\n> rev-parse --show-prefix` in the alias?\" then realized by the time we\n> do that, it's already too late. Maybe\n> \n>> +'GIT_PREFIX' is set as returned by running 'git rev-parse --show-prefix'\n>> +before current directory is changed to top-level directory. See linkgit:git-rev-parse[1].\n> \n> ?\n\nWell, the text says already that the alias is not run from the \"original\ncurrent directory\", so it's two ways of saying the same thing.\n\nMichael\n"}]}