{"thread":{"id":"11363","subject":"[PATCH] builtin-tag.c: allow arguments in $EDITOR","startedAt":"2007-12-19T23:23:26Z","lastAt":"2007-12-22T14:50:07Z","messageCount":7,"participants":["Luciano Rocha","Johannes Schindelin","Junio C Hamano","Steven Grimm"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"63815","messageId":"20071219232326.GA4135@bit.office.eurotux.com","threadId":"11363","inReplyTo":null,"subject":"[PATCH] builtin-tag.c: allow arguments in $EDITOR","fromName":"Luciano Rocha","fromEmail":"luciano@eurotux.com","sentAt":"2007-12-19T23:23:26Z","receivedAt":"2007-12-19T23:23:26Z","isPatch":true,"sender":{"key":"luciano@eurotux.com","avatar":null},"body":"The previous sh version of git-commit evaluated the value of the defined\neditor, thus allowing arguments.\n\nMake the builtin version work the same, by adding an explicit check for\narguments in the editor command, and extract them to an additional argument.\n\nSigned-off-by: Luciano Rocha <luciano@eurotux.com>\n---\n builtin-tag.c |   13 ++++++++++++-\n 1 files changed, 12 insertions(+), 1 deletions(-)\n\nI personally use EDITOR=\"gvim -f\", thus this patch.\n\nCreated on top of ce85b053d827e2f7c2ee2683cc09393e4768cc22, \ngit-describe is now: v1.5.4-rc0-75-g5f791e5\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 274901a..57dcfe0 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -46,7 +46,18 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \tif (!editor)\n \t\teditor = \"vi\";\n \n-\tif (strcmp(editor, \":\")) {\n+\tif (strstr(editor, \" -\")) {\n+\t\tchar *editor_cmd = xstrdup(editor);\n+\t\tchar *editor_sep = strstr(editor_cmd, \" -\");\n+\t\tconst char *args[] = { editor_cmd, editor_sep + 1,\n+\t\t\tpath, NULL };\n+\n+\t\t*editor_sep = '\\0';\n+\n+\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n+\t\t\tdie(\"There was a problem with the editor %s.\",\n+\t\t\t\t\teditor_cmd);\n+\t} else if (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n \n \t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-- \nLuciano Rocha <luciano@eurotux.com>\nEurotux Informática, S.A. <http://www.eurotux.com/>\n"},{"id":"63847","messageId":"20071220095706.GA9685@bit.office.eurotux.com","threadId":"11363","inReplyTo":"20071219232326.GA4135@bit.office.eurotux.com","subject":"[PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Luciano Rocha","fromEmail":"luciano@eurotux.com","sentAt":"2007-12-20T09:57:06Z","receivedAt":"2007-12-20T09:57:06Z","isPatch":true,"sender":{"key":"luciano@eurotux.com","avatar":null},"body":"\nThe previous sh version of git-commit evaluated the value of the defined\neditor, thus allowing arguments.\n\nMake the builtin version work the same, by adding an explicit check for\narguments in the editor command, and extract them to an additional argument.\n\nSigned-off-by: Luciano Rocha <luciano@eurotux.com>\n---\n builtin-tag.c |   14 +++++++++++++-\n 1 files changed, 13 insertions(+), 1 deletions(-)\n\nI personally use EDITOR=\"gvim -f\", thus this patch.\nNow with free() of temporary buffer.\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 274901a..0e8575e 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -46,7 +46,19 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \tif (!editor)\n \t\teditor = \"vi\";\n \n-\tif (strcmp(editor, \":\")) {\n+\tif (strstr(editor, \" -\")) {\n+\t\tchar *editor_cmd = xstrdup(editor);\n+\t\tchar *editor_sep = strstr(editor_cmd, \" -\");\n+\t\tconst char *args[] = { editor_cmd, editor_sep + 1,\n+\t\t\tpath, NULL };\n+\n+\t\t*editor_sep = '\\0';\n+\n+\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n+\t\t\tdie(\"There was a problem with the editor %s.\",\n+\t\t\t\t\teditor_cmd);\n+\t\tfree(editor_cmd);\n+\t} else if (strcmp(editor, \":\")) {\n \t\tconst char *args[] = { editor, path, NULL };\n \n \t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-- \nLuciano Rocha <luciano@eurotux.com>\nEurotux Informática, S.A. <http://www.eurotux.com/>\n"},{"id":"63854","messageId":"Pine.LNX.4.64.0712201255510.14355@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"11363","inReplyTo":"20071220095706.GA9685@bit.office.eurotux.com","subject":"Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-20T11:58:59Z","receivedAt":"2007-12-20T11:58:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 20 Dec 2007, Luciano Rocha wrote:\n\n> The previous sh version of git-commit evaluated the value of the defined \n> editor, thus allowing arguments.\n> \n> Make the builtin version work the same, by adding an explicit check for \n> arguments in the editor command, and extract them to an additional \n> argument.\n\nAnything wrong with that patch?\n\nhttp://article.gmane.org/gmane.comp.version-control.git/68444\n\nCiao,\nDscho\n"},{"id":"63855","messageId":"20071220120601.GA15290@bit.office.eurotux.com","threadId":"11363","inReplyTo":"Pine.LNX.4.64.0712201255510.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Luciano Rocha","fromEmail":"luciano@eurotux.com","sentAt":"2007-12-20T12:06:01Z","receivedAt":"2007-12-20T12:06:01Z","isPatch":true,"sender":{"key":"luciano@eurotux.com","avatar":null},"body":"On Thu, Dec 20, 2007 at 12:58:59PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 20 Dec 2007, Luciano Rocha wrote:\n> \n> > The previous sh version of git-commit evaluated the value of the defined \n> > editor, thus allowing arguments.\n> > \n> > Make the builtin version work the same, by adding an explicit check for \n> > arguments in the editor command, and extract them to an additional \n> > argument.\n> \n> Anything wrong with that patch?\n> \n> http://article.gmane.org/gmane.comp.version-control.git/68444\n\nNo, I just missed it in the mailing list. That patch also supports any\nnumber of whitespace/arguments.\n\n-- \nLuciano Rocha <luciano@eurotux.com>\nEurotux Informática, S.A. <http://www.eurotux.com/>\n"},{"id":"63880","messageId":"7vhcidovxt.fsf@gitster.siamese.dyndns.org","threadId":"11363","inReplyTo":"Pine.LNX.4.64.0712201255510.14355@wbgn129.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-20T22:14:38Z","receivedAt":"2007-12-20T22:14:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Anything wrong with that patch?\n>\n> http://article.gmane.org/gmane.comp.version-control.git/68444\n\nI think Steven stopped after you poked holes in that patch.\n\nThe way scripted commands spawned editor is:\n\n\teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n\nwhich meant that $IFS characters in $GIT_EDITOR separated words\nand $environment_variables were substituted.\n\nIOW, this is possible:\n\n\tGIT_EDITOR='emacs -l $HOME/my-customization.el'\n\nI think something like this patch is probably more appropriate.\nIt avoids potential bugs in splitting arguments by hand and lets the\nshell deal with the issue.\n\n---\n builtin-tag.c |   14 +++++++++++++-\n 1 files changed, 13 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 274901a..fae2487 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -47,7 +47,19 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \t\teditor = \"vi\";\n \n \tif (strcmp(editor, \":\")) {\n-\t\tconst char *args[] = { editor, path, NULL };\n+\t\tsize_t len = strlen(editor);\n+\t\tint i = 0;\n+\t\tconst char *args[6];\n+\n+\t\tif (strcspn(editor, \"$ \\t'\") != len) {\n+\t\t\t/* there are specials */\n+\t\t\targs[i++] = \"sh\";\n+\t\t\targs[i++] = \"-c\";\n+\t\t\targs[i++] = \"$0 \\\"$@\\\"\";\n+\t\t}\n+\t\targs[i++] = editor;\n+\t\targs[i++] = path;\n+\t\targs[i] = NULL;\n \n \t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n \t\t\tdie(\"There was a problem with the editor %s.\", editor);\n\n\t\n"},{"id":"63894","messageId":"C84F3F74-6FB1-46E9-8627-09FC636D256E@midwinter.com","threadId":"11363","inReplyTo":"7vhcidovxt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-12-21T00:33:12Z","receivedAt":"2007-12-21T00:33:12Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"On Dec 20, 2007, at 2:14 PM, Junio C Hamano wrote:\n> I think Steven stopped after you poked holes in that patch.\n\nNah, just entered a particularly busy period in my day job and haven't  \nhad time to do much more git stuff than occasionally skim the mailing  \nlist. I do plan to revisit that at some point unless the patch in your  \nmail ends up being what we go with. (It seems like a sensible approach  \nto me.)\n\n-Steve\n"},{"id":"63997","messageId":"Pine.LNX.4.64.0712221548540.14355@wbgn129.biozentrum.uni-wuerzburg.de","threadId":"11363","inReplyTo":"7vhcidovxt.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2] builtin-tag.c: allow arguments in $EDITOR","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-22T14:50:07Z","receivedAt":"2007-12-22T14:50:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 20 Dec 2007, Junio C Hamano wrote:\n\n> I think something like this patch is probably more appropriate.\n\nLooks obviously fine, especially thinking about this:\n\n> \tGIT_EDITOR='emacs -l $HOME/my-customization.el'\n\nCiao,\nDscho\n"}]}