{"thread":{"id":"4970","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","startedAt":"2006-07-18T23:25:57Z","lastAt":"2006-07-25T20:34:29Z","messageCount":8,"participants":["Junio C Hamano","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"297420","messageId":"Pine.LNX.4.63.0607190125150.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4970","inReplyTo":null,"subject":"[PATCH] Allow an alias to start with \"-p\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-18T23:25:57Z","receivedAt":"2006-07-18T23:25:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nNow, something like\n\n\t[alias]\n\t\tpd = -p diff\n\nworks as expected.\n\nSigned-off-by: Johannes Schindelin \n---\n git.c | 6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 537af3a..a969b1b 100644\n--- a/git.c\n+++ b/git.c\n@@ -115,6 +115,12 @@ static int handle_alias(int *argcp, cons\n \n \t\t\tcount = split_cmdline(alias_string, &new_argv);\n \n+\t\t\tif (count > 0 && !strcmp(new_argv[0], \"-p\")) {\n+\t\t\t\tsetup_pager();\n+\t\t\t\tcount--;\n+\t\t\t\tnew_argv++;\n+\t\t\t}\n+\n \t\t\tif (count < 1)\n \t\t\t\tdie(\"empty alias for %s\", alias_command);\n \n-- \n1.4.1.g3c58e\n"},{"id":"24038","messageId":"7v8xmjh5sv.fsf@assigned-by-dhcp.cox.net","threadId":"4970","inReplyTo":"Pine.LNX.4.63.0607190125150.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-24T06:32:00Z","receivedAt":"2006-07-24T06:32:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Now, something like\n>\n> \t[alias]\n> \t\tpd = -p diff\n>\n> works as expected.\n\nI like what it wants to do but I am afraid this leads to an\nunmaintainable code (a micronit that already shows what I mean\nis that you can say \"git --paginate diff\", but you cannot say\n\"pd = --paginate diff\" in the configuration file).\n\nIs there a cleaner way to do it without duplicating the argument\nloop of git.c::main()?\n"},{"id":"24062","messageId":"Pine.LNX.4.63.0607241408170.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4970","inReplyTo":"7v8xmjh5sv.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Allow an alias to start with \"-p\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-24T12:10:45Z","receivedAt":"2006-07-24T12:10:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":" \nNow, something like\n\n\t[alias]\n\t\tpd = -p diff\n\nworks as expected.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\nOn Sun, 23 Jul 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Now, something like\n> >\n> > \t[alias]\n> > \t\tpd = -p diff\n> >\n> > works as expected.\n> \n> I like what it wants to do but I am afraid this leads to an\n> unmaintainable code (a micronit that already shows what I mean\n> is that you can say \"git --paginate diff\", but you cannot say\n> \"pd = --paginate diff\" in the configuration file).\n> \n> Is there a cleaner way to do it without duplicating the argument\n> loop of git.c::main()?\n\nThis patch uses a better approach: instead of duplicating the option\nparsing of the git wrapper, it refactors the code into the new function\nhandle_options().\n\nIn related news, this function would be the perfect candidate to set \nGIT_DIR without environment variables...\n\n git.c | 37 +++++++++++++++++++++++++++++++------\n 1 files changed, 31 insertions(+), 6 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex ee5a0e8..8d7c644 100644\n--- a/git.c\n+++ b/git.c\n@@ -35,6 +35,27 @@ static void prepend_to_path(const char *\n \tsetenv(\"PATH\", path, 1);\n }\n \n+static int handle_options(const char*** argv, int* argc)\n+{\n+\tint handled = 0;\n+\n+\twhile (*argc > 0) {\n+\t\tconst char *cmd = (*argv)[0];\n+\t\tif (cmd[0] != '-')\n+\t\t\tbreak;\n+\n+\t\tif (!strcmp(cmd, \"-p\") || !strcmp(cmd, \"--paginate\")) {\n+\t\t\tsetup_pager();\n+\t\t} else\n+\t\t\tdie (\"Unknown option: %s\", cmd);\n+\n+\t\t(*argv)++;\n+\t\t(*argc)--;\n+\t\thandled++;\n+\t}\n+\treturn handled;\n+}\n+\n static const char *alias_command;\n static char *alias_string = NULL;\n \n@@ -106,7 +127,7 @@ static int handle_alias(int *argcp, cons\n \n \tsubdir = setup_git_directory_gently(&nongit);\n \tif (!nongit) {\n-\t\tint count;\n+\t\tint count, option_count;\n \t\tconst char** new_argv;\n \n \t\talias_command = (*argv)[0];\n@@ -114,6 +135,10 @@ static int handle_alias(int *argcp, cons\n \t\tif (alias_string) {\n \n \t\t\tcount = split_cmdline(alias_string, &new_argv);\n+\t\t\toption_count = handle_options(&new_argv, &count);\n+\t\t\tmemmove(new_argv - option_count, new_argv,\n+\t\t\t\t\tcount * sizeof(char *));\n+\t\t\tnew_argv -= option_count;\n \n \t\t\tif (count < 1)\n \t\t\t\tdie(\"empty alias for %s\", alias_command);\n@@ -264,6 +289,7 @@ int main(int argc, const char **argv, ch\n \tif (!strncmp(cmd, \"git-\", 4)) {\n \t\tcmd += 4;\n \t\targv[0] = cmd;\n+\t\thandle_alias(&argc, &argv);\n \t\thandle_internal_command(argc, argv, envp);\n \t\tdie(\"cannot handle %s internally\", cmd);\n \t}\n@@ -273,13 +299,12 @@ int main(int argc, const char **argv, ch\n \n \t/* Look for flags.. */\n \twhile (argc > 1) {\n-\t\tcmd = *++argv;\n+\t\targv++;\n \t\targc--;\n \n-\t\tif (!strcmp(cmd, \"-p\") || !strcmp(cmd, \"--paginate\")) {\n-\t\t\tsetup_pager();\n-\t\t\tcontinue;\n-\t\t}\n+\t\thandle_options(&argv, &argc);\n+\t\t\t\n+\t\tcmd = *argv;\n \n \t\tif (strncmp(cmd, \"--\", 2))\n \t\t\tbreak;\n"},{"id":"24077","messageId":"7virlmfrl4.fsf@assigned-by-dhcp.cox.net","threadId":"4970","inReplyTo":"Pine.LNX.4.63.0607241408170.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-25T00:36:39Z","receivedAt":"2006-07-25T00:36:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Now, something like\n>\n> \t[alias]\n> \t\tpd = -p diff\n>\n> works as expected.\n>\n> Signed-off-by: Johannes Schindelin \n\nThis seems to break t5400 among other things (git-clone\ncomplains that it is not invoked in a git repository).\n"},{"id":"24079","messageId":"20060725050312.GA5618@coredump.intra.peff.net","threadId":"4970","inReplyTo":"Pine.LNX.4.63.0607241408170.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-07-25T05:03:12Z","receivedAt":"2006-07-25T05:03:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 24, 2006 at 02:10:45PM +0200, Johannes Schindelin wrote:\n\n> @@ -264,6 +289,7 @@ int main(int argc, const char **argv, ch\n> \tif (!strncmp(cmd, \"git-\", 4)) {\n> \t\tcmd += 4;\n> \t\targv[0] = cmd;\n> +\t\thandle_alias(&argc, &argv);\n> \t\thandle_internal_command(argc, argv, envp);\n> \t\tdie(\"cannot handle %s internally\", cmd);\n> \t}\n\nI believe this change is the source of the breakage in tests.\nGIT_DIR=foo git-init-db no longer works because handle_alias\nunconditionally calls setup_git_directory_gently(), which thinks that if\nGIT_DIR is set, it must exist.\n\nThis can be fixed by giving precedence to the internal command over\nalias checking. This makes sense, anyway, since later in the function,\nwe give precedence to internal commands in the \"git init-db\" form.\n\nPatch is below (wow, that +++ is kind of ugly!).\n\n-Peff\n\n+++\ngit: choose internal commands over aliases for git-*\n\nThis is especially important because some commands (like init-db) don't\nrequire a working GIT_DIR, and alias expansion tries to look at it. It\nalso matches the behavior of \"git cmd\".\n\nSigned-off-by: Jeff King \n---\n git.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex 8d7c644..68ce826 100644\n--- a/git.c\n+++ b/git.c\n@@ -289,8 +289,8 @@ int main(int argc, const char **argv, ch\n \tif (!strncmp(cmd, \"git-\", 4)) {\n \t\tcmd += 4;\n \t\targv[0] = cmd;\n-\t\thandle_alias(&argc, &argv);\n \t\thandle_internal_command(argc, argv, envp);\n+\t\thandle_alias(&argc, &argv);\n \t\tdie(\"cannot handle %s internally\", cmd);\n \t}\n \n-- \n1.4.2.rc1.gc470-dirty\n"},{"id":"24080","messageId":"Pine.LNX.4.63.0607250813450.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4970","inReplyTo":"20060725050312.GA5618@coredump.intra.peff.net","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-25T06:18:46Z","receivedAt":"2006-07-25T06:18:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 25 Jul 2006, Jeff King wrote:\n\n> On Mon, Jul 24, 2006 at 02:10:45PM +0200, Johannes Schindelin wrote:\n> \n> > @@ -264,6 +289,7 @@ int main(int argc, const char **argv, ch\n> > \tif (!strncmp(cmd, \"git-\", 4)) {\n> > \t\tcmd += 4;\n> > \t\targv[0] = cmd;\n> > +\t\thandle_alias(&argc, &argv);\n> > \t\thandle_internal_command(argc, argv, envp);\n> > \t\tdie(\"cannot handle %s internally\", cmd);\n> > \t}\n\nMy fault. This was a left-over from my original alias patch. (I did a \nmerge, and just used \"git-diff next\" instead of \"git-diff --merge next\". \n Nope, that would not have worked either.\n\n> Patch is below (wow, that +++ is kind of ugly!).\n\nSame here.\n\n> git: choose internal commands over aliases for git-*\n> \n> This is especially important because some commands (like init-db) don't\n> require a working GIT_DIR, and alias expansion tries to look at it. It\n> also matches the behavior of \"git cmd\".\n> \n> Signed-off-by: Jeff King \n\nAcked-by: Johannes Schindelin \n\n> @@ -289,8 +289,8 @@ int main(int argc, const char **argv, ch\n> \tif (!strncmp(cmd, \"git-\", 4)) {\n> \t\tcmd += 4;\n> \t\targv[0] = cmd;\n> -\t\thandle_alias(&argc, &argv);\n> \t\thandle_internal_command(argc, argv, envp);\n> +\t\thandle_alias(&argc, &argv);\n> \t\tdie(\"cannot handle %s internally\", cmd);\n> \t}\n\nAlternatively, you can just delete it. IIRC we decided that aliases with \n\"git-\" commands do not make sense.\n\nCiao,\nDscho\n"},{"id":"24081","messageId":"Pine.LNX.4.63.0607250822200.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4970","inReplyTo":"7virlmfrl4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-25T06:27:41Z","receivedAt":"2006-07-25T06:27:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 24 Jul 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Now, something like\n> >\n> > \t[alias]\n> > \t\tpd = -p diff\n> >\n> > works as expected.\n> >\n> > Signed-off-by: Johannes Schindelin \n> \n> This seems to break t5400 among other things (git-clone\n> complains that it is not invoked in a git repository).\n\nSee Peff's mail for a fix (hopefully: I do not see why git-clone should \nbe affected, as it is a script, not a hard link to the git wrapper). The \nfunny thing: t5400 does not break here.\n\nCiao,\nDscho\n"},{"id":"24103","messageId":"7v8xmhe84q.fsf@assigned-by-dhcp.cox.net","threadId":"4970","inReplyTo":"Pine.LNX.4.63.0607250813450.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Allow an alias to start with \"-p\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-25T20:34:29Z","receivedAt":"2006-07-25T20:34:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Patch is below (wow, that +++ is kind of ugly!).\n>\n> Same here.\n\nSame here ;-).\n\n> Acked-by: Johannes Schindelin \n>\n>> @@ -289,8 +289,8 @@ int main(int argc, const char **argv, ch\n>> \tif (!strncmp(cmd, \"git-\", 4)) {\n>> \t\tcmd += 4;\n>> \t\targv[0] = cmd;\n>> -\t\thandle_alias(&argc, &argv);\n>> \t\thandle_internal_command(argc, argv, envp);\n>> +\t\thandle_alias(&argc, &argv);\n>> \t\tdie(\"cannot handle %s internally\", cmd);\n>> \t}\n>\n> Alternatively, you can just delete it. IIRC we decided that aliases with \n> \"git-\" commands do not make sense.\n\nI think that is reasonable and simpler.\n"}]}