{"thread":{"id":"9339","subject":"Interpreting EDITOR/VISUAL environment variables.","startedAt":"2007-08-01T13:36:48Z","lastAt":"2007-08-02T10:31:25Z","messageCount":16,"participants":["David Kastrup","Junio C Hamano","Yann Dirson","Matthias Lederhofer"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"49308","messageId":"86abtbnzpr.fsf@lola.quinscape.zz","threadId":"9339","inReplyTo":null,"subject":"Interpreting EDITOR/VISUAL environment variables.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T13:36:48Z","receivedAt":"2007-08-01T13:36:48Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\nWhatever I have been able to Google, is completely silent on this\nmatter. If anybody has an idea where to find authoritative\ninformation, holler.\n\nIn the meantime, in the Emacs manual there is the following bit of\ninformation:\n\n(info \"(emacs) Invoking Emacsclient\")\n\n The option `-a COMMAND' or `--alternate-editor=COMMAND' specifies a\n command to run if `emacsclient' fails to contact Emacs. This is useful\n when running `emacsclient' in a script. For example, the following\n setting for the `EDITOR' environment variable will always give you an\n editor, even if no Emacs server is running:\n\n EDITOR=\"emacsclient --alternate-editor emacs +%d %s\"\n\nThat makes it likely that the way to call an editor should be via\nsystem. However, there are certainly programs around which will not\ninterpret the +%d and %s thingies. My current setting is\n\n EDITOR=\"emacsclient --alternate-editor vi\"\n\nand this seems to do the trick with most applications. Not so with\ngit-commit and other git scripts. The easiest way out will be to\ncreate something like ~/bin/myemacsclient which does the respective\nargument splicing. I am just not sure this is the \"canonically\ncorrect way\" of interpreting $EDITOR.\n\nActually, splicing $EDITOR into a system command is a nuisance because\nit means having to shell-quote its arguments. So the current\ninterpretation is likely easier to maintain.\n\nIs it the correct one?\n\n-- \nDavid Kastrup\n"},{"id":"49326","messageId":"7vd4y75gcy.fsf@assigned-by-dhcp.cox.net","threadId":"9339","inReplyTo":"86abtbnzpr.fsf@lola.quinscape.zz","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-01T17:12:13Z","receivedAt":"2007-08-01T17:12:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Actually, splicing $EDITOR into a system command is a nuisance because\n> it means having to shell-quote its arguments. So the current\n> interpretation is likely easier to maintain.\n>\n> Is it the correct one?\n\nI've been torn on this one. From the point of view of\n\"specified behaviour in the documentation\", which is \"EDITOR and\nVISUAL name the editor of your choice\", not splicing is not\nviolating the letter (I am not talking about our documentation\nhere, but many other programs'). Splicing and shell quoting\nother parameters, while it is technically not a problem at all\ndoing that in scripts, feels \"dirty\". Maybe it's just me.\n\nBoth cvs and svn seems to splice, I suspect they just do a\nstraight system(3) invocation.\n\nWe recently normalized the script callers not to splice at all\n(the scripts were hand-rolling \"the VISUAL or EDITOR or vi\" and\nslightly differently). It obviously has negative (i.e. setting\nEDITOR to \"emacsclient --alternate-editor vi\" does not work) as\nwell as positive side (i.e. \"/home/dak/My Programs/editor\" would\nwork).\n"},{"id":"49336","messageId":"20070801185042.GB30277@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"9339","inReplyTo":"7vd4y75gcy.fsf@assigned-by-dhcp.cox.net","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-08-01T18:50:42Z","receivedAt":"2007-08-01T18:50:42Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"On Wed, Aug 01, 2007 at 10:12:13AM -0700, Junio C Hamano wrote:\n> We recently normalized the script callers not to splice at all\n> (the scripts were hand-rolling \"the VISUAL or EDITOR or vi\" and\n> slightly differently). It obviously has negative (i.e. setting\n> EDITOR to \"emacsclient --alternate-editor vi\" does not work) as\n> well as positive side (i.e. \"/home/dak/My Programs/editor\" would\n> work).\n\nAnd, indeed, --alternate-editor could be supplemented by another\nenvvar to be able to work in our situation. Maybe the various emacsen\nvendors would be willing to integrate such a patch ?\n\nBest regards,\n-- \nYann\n"},{"id":"49339","messageId":"85r6mnrs1z.fsf@lola.goethe.zz","threadId":"9339","inReplyTo":"7vd4y75gcy.fsf@assigned-by-dhcp.cox.net","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T19:08:40Z","receivedAt":"2007-08-01T19:08:40Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> David Kastrup writes:\n>\n>> Actually, splicing $EDITOR into a system command is a nuisance because\n>> it means having to shell-quote its arguments. So the current\n>> interpretation is likely easier to maintain.\n>>\n>> Is it the correct one?\n>\n> I've been torn on this one. From the point of view of\n> \"specified behaviour in the documentation\", which is \"EDITOR and\n> VISUAL name the editor of your choice\", not splicing is not\n> violating the letter (I am not talking about our documentation\n> here, but many other programs'). Splicing and shell quoting\n> other parameters, while it is technically not a problem at all\n> doing that in scripts, feels \"dirty\". Maybe it's just me.\n>\n> Both cvs and svn seems to splice, I suspect they just do a\n> straight system(3) invocation.\n>\n> We recently normalized the script callers not to splice at all\n> (the scripts were hand-rolling \"the VISUAL or EDITOR or vi\" and\n> slightly differently). It obviously has negative (i.e. setting\n> EDITOR to \"emacsclient --alternate-editor vi\" does not work) as\n> well as positive side (i.e. \"/home/dak/My Programs/editor\" would\n> work).\n\nWell, I just checked the behavior with \"less\", \"more\", \"mail\" and\n\"mailx\", quite traditional commands that would not have a reason to\ncomplicate things by using \"system\" and quoting instead of exec.\n\nless, mail and mailx apparently go via system, more (wtf?!?)\napparently via exec.\n\nTaken together with the behavior by cvs and svn, I think we should not\njust throw EDITOR/VISUAL into one exec arg.\n\nThen there are two implementations to pick from:\n\na) Using system and shell-quoting the filename. Advantage: one can\nset EDITOR='\"/home/dak/My Programs/editor\"' and have it work.\nDisadvantage: shell-quoting a file name seems shell- and\nsystem-dependent.\n\nIt turns out all three contestants still in the race apparently do a).\nIf nobody has a sensible idea how to shell-quote generally enough...\nUnder Unix, one has the option of using \"...\" and quoting the three of\n\"$\\ with \\ or using '...' and replacing every contained ' with '\\''.\nI don't think that there is a library function generally available.\nThe \" quote type would probably be more typical.\n\nIt turns out that gitk and gitk-wish _already_ do this. So the\nnormalization does not seem to have covered them if I read the code\ncorrectly:\n\nproc shellquote {str} {\n if {![string match \"*\\['\\\"\\\\ \\t]*\" $str]} {\n return $str\n }\n if {![string match \"*\\['\\\"\\\\]*\" $str]} {\n return \"\\\"$str\\\"\"\n }\n if {![string match \"*'*\" $str]} {\n return \"'$str'\"\n }\n return \"\\\"[string map {\\\" \\\\\\\" \\\\ \\\\\\\\} $str]\\\"\"\n}\n\nNote that the first case does not cover strings with newlines in them,\nthough, and the second does not help with dollar signs. And I have no\nclue what the last return does. Presumably maps \" to \\\" and \\ to \\\\\ninside of double quotes.\n\nb) splitting EDITOR/VISUAL at spaces and using exec. Nobody else\nappears to do this, so may neither should be.\n\n\nIt appears that the C code already has quote.c, so it is probably more\nor less doable.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49342","messageId":"85ir7zrr0u.fsf@lola.goethe.zz","threadId":"9339","inReplyTo":"20070801185042.GB30277@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T19:30:57Z","receivedAt":"2007-08-01T19:30:57Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Yann Dirson writes:\n\n> On Wed, Aug 01, 2007 at 10:12:13AM -0700, Junio C Hamano wrote:\n>> We recently normalized the script callers not to splice at all\n>> (the scripts were hand-rolling \"the VISUAL or EDITOR or vi\" and\n>> slightly differently). It obviously has negative (i.e. setting\n>> EDITOR to \"emacsclient --alternate-editor vi\" does not work) as\n>> well as positive side (i.e. \"/home/dak/My Programs/editor\" would\n>> work).\n>\n> And, indeed, --alternate-editor could be supplemented by another\n> envvar to be able to work in our situation.\n\nIt is already. But if git is pretty much alone in breaking a setup\nthat is working everywhere else, is having a workaround available\nreally a good excuse for not doing the right thing?\n\n> Maybe the various emacsen vendors would be willing to integrate such\n> a patch ?\n\nActually, it is a nuisance because nobody remembers this variable. It\nis called (looking in the Emacs manual, using the index to find\nemacsclient, following a link after two pages to the invocation, going\ndown two pages again) ALTERNATE_EDITOR. It does not even _mention_\nEmacs or emacsclient in its name. The \"-a\" option is easier to\nremember.\n\nSo yes, emacsclient has an environment hook making it possible to work\naround git's idiosyncratic behavior here. But should it really be\nnecessary?\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49345","messageId":"7v7iof3uc5.fsf@assigned-by-dhcp.cox.net","threadId":"9339","inReplyTo":"85r6mnrs1z.fsf@lola.goethe.zz","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-01T19:53:14Z","receivedAt":"2007-08-01T19:53:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Well, I just checked the behavior with \"less\", \"more\", \"mail\" and\n> \"mailx\", quite traditional commands that would not have a reason to\n> complicate things by using \"system\" and quoting instead of exec.\n> ...\n> It turns out all three contestants still in the race apparently do a).\n> If nobody has a sensible idea how to shell-quote generally enough...\n> Under Unix, one has the option of using \"...\" and quoting the three of\n> \"$\\ with \\ or using '...' and replacing every contained ' with '\\''.\n\nOur scripts and C-level both tend to prefer using '\\'' for its\nsimplicity (this also applies to rare case the user wants to\nunquote it by hand). Because it now is all in git-sh-setup, it\nshould be reasonably straightforward to implement the quoting of\nthe temporary file name and drop dq around ${EDITOR-VISUAL}.\n\nPlease make it so.\n"},{"id":"49352","messageId":"S1752294AbXHAWCj/20070801220239Z+281@vger.kernel.org","threadId":"9339","inReplyTo":"7v7iof3uc5.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T21:47:20Z","receivedAt":"2007-08-01T21:47:20Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"The previous code only allowed specifying a single executable rather\nthan a complete command like \"emacsclient --alternate-editor vi\" in\nthose variables. Since VISUAL/EDITOR appear to be traditionally\npassed to a shell for interpretation (as corroborated with \"less\",\n\"mail\" and \"mailx\", while the really ancient \"more\" indeed allows only\nan executable name), the shell function git_editor has been amended\nappropriately.\n\n\"eval\" is employed to have quotes and similar interpreted _after_\nexpansion, so that specifying\nEDITOR='\"/home/dak/My Commands/notepad.exe\"'\ncan be used for actually using commands with blanks.\n\nInstead of passing just the first argument of git_editor on, we pass\nall of them (so that +lineno might be employed at a later point of\ntime, or so that multiple files may be edited when appropriate).\n\nStrictly speaking, there is a change in behavior: when\ngit config core.editor\nreturns a valid but empty string, the fallbacks are still searched.\nThis is more consistent, and the old code was problematic with regard\nto multiple blanks. Putting in additional quotes might have worked,\nbut quotes inside of command substitution inside of quotes is nasty\nenough to not reliably work the same across \"Bourne shells\".\n\nSigned-off-by: David Kastrup \n---\n git-sh-setup.sh | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 3c0367d..3c50bc1 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -29,7 +29,8 @@ set_reflog_action() {\n }\n \n git_editor() {\n-\tGIT_EDITOR=${GIT_EDITOR:-$(git config core.editor || echo ${VISUAL:-${EDITOR}})}\n+\t: \"${GIT_EDITOR:=$(git config core.editor)}\"\n+\t: \"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}\"\n \tcase \"$GIT_EDITOR,$TERM\" in\n \t,dumb)\n \t\techo >&2 \"No editor specified in GIT_EDITOR, core.editor, VISUAL,\"\n@@ -40,7 +41,7 @@ git_editor() {\n \t\texit 1\n \t\t;;\n \tesac\n-\t${GIT_EDITOR:-vi} \"$1\"\n+\teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n }\n \n is_bare_repository () {\n-- \n1.5.3.rc2.86.gdc7ba\n"},{"id":"49368","messageId":"S1752099AbXHAXHc/20070801230732Z+342@vger.kernel.org","threadId":"9339","inReplyTo":"85ejimrjb2.fsf@lola.goethe.zz","subject":"[PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T21:47:20Z","receivedAt":"2007-08-01T21:47:20Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"The previous code only allowed specifying a single executable rather\nthan a complete command like \"emacsclient --alternate-editor vi\" in\nthose variables. Since VISUAL/EDITOR appear to be traditionally\npassed to a shell for interpretation (as corroborated with \"less\",\n\"mail\" and \"mailx\", while the really ancient \"more\" indeed allows only\nan executable name), the shell function git_editor has been amended\nappropriately.\n\n\"eval\" is employed to have quotes and similar interpreted _after_\nexpansion, so that specifying\nEDITOR='\"/home/dak/My Commands/notepad.exe\"'\ncan be used for actually using commands with blanks.\n\nInstead of passing just the first argument of git_editor on, we pass\nall of them (so that +lineno might be employed at a later point of\ntime, or so that multiple files may be edited when appropriate).\n\nStrictly speaking, there is a change in behavior: when\ngit config core.editor\nreturns a valid but empty string, the fallbacks are still searched.\nThis is more consistent, and the old code was problematic with regard\nto multiple blanks. Putting in additional quotes might have worked,\nbut quotes inside of command substitution inside of quotes is nasty\nenough to not reliably work the same across \"Bourne shells\".\n\nSigned-off-by: David Kastrup \n---\n git-sh-setup.sh | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex c51985e..3c50bc1 100755\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -29,7 +29,8 @@ set_reflog_action() {\n }\n \n git_editor() {\n-\tGIT_EDITOR=${GIT_EDITOR:-$(git config core.editor || echo ${VISUAL:-${EDITOR}})}\n+\t: \"${GIT_EDITOR:=$(git config core.editor)}\"\n+\t: \"${GIT_EDITOR:=${VISUAL:-${EDITOR}}}\"\n \tcase \"$GIT_EDITOR,$TERM\" in\n \t,dumb)\n \t\techo >&2 \"No editor specified in GIT_EDITOR, core.editor, VISUAL,\"\n@@ -40,7 +41,7 @@ git_editor() {\n \t\texit 1\n \t\t;;\n \tesac\n-\t\"${GIT_EDITOR:-vi}\" \"$1\"\n+\teval \"${GIT_EDITOR:=vi}\" '\"$@\"'\n }\n \n is_bare_repository () {\n-- \n1.5.3.rc2.86.gdc7ba\n"},{"id":"49358","messageId":"85ejimrjb2.fsf@lola.goethe.zz","threadId":"9339","inReplyTo":"S1752294AbXHAWCj/20070801220239Z+281@vger.kernel.org","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T22:17:37Z","receivedAt":"2007-08-01T22:17:37Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"David Kastrup writes:\n\n> The previous code only allowed specifying a single executable rather\n> than a complete command like \"emacsclient --alternate-editor vi\" in\n\nOops, won't apply cleanly. I found that I had already made a\ndifferent (trivial) patch previously. Let me try again and fold that\npatch in manually.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49370","messageId":"7vy7gu3kuh.fsf@assigned-by-dhcp.cox.net","threadId":"9339","inReplyTo":"85ejimrjb2.fsf@lola.goethe.zz","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-01T23:18:14Z","receivedAt":"2007-08-01T23:18:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> David Kastrup writes:\n>\n>> The previous code only allowed specifying a single executable rather\n>> than a complete command like \"emacsclient --alternate-editor vi\" in\n>\n> Oops, won't apply cleanly. I found that I had already made a\n> different (trivial) patch previously. Let me try again and fold that\n> patch in manually.\n\nIt is not just \"won't apply\". What if GIT_DIR had spaces (which\nis fine) and single-quotes in it? Wouldn't it percolate down to\n$@ because it becomes the leading directory of the temporary\nfile name? And you quote '\"$@\"' and eval it, now what happens?\n"},{"id":"49371","messageId":"7vtzri3kpr.fsf@assigned-by-dhcp.cox.net","threadId":"9339","inReplyTo":"7vy7gu3kuh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-01T23:21:04Z","receivedAt":"2007-08-01T23:21:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> It is not just \"won't apply\". What if GIT_DIR had spaces (which\n> is fine) and single-quotes in it? Wouldn't it percolate down to\n> $@ because it becomes the leading directory of the temporary\n> file name? And you quote '\"$@\"' and eval it, now what happens?\n\nAh, I spoke too fast. It is fine --- the shell that actually is\ndoing the eval then interpolates \"$@\". Clever.\n"},{"id":"49373","messageId":"85wswex11h.fsf@lola.goethe.zz","threadId":"9339","inReplyTo":"7vy7gu3kuh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T23:55:38Z","receivedAt":"2007-08-01T23:55:38Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> David Kastrup writes:\n>\n>> David Kastrup writes:\n>>\n>>> The previous code only allowed specifying a single executable rather\n>>> than a complete command like \"emacsclient --alternate-editor vi\" in\n>>\n>> Oops, won't apply cleanly. I found that I had already made a\n>> different (trivial) patch previously. Let me try again and fold that\n>> patch in manually.\n>\n> It is not just \"won't apply\". What if GIT_DIR had spaces (which\n> is fine) and single-quotes in it? Wouldn't it percolate down to\n> $@ because it becomes the leading directory of the temporary\n> file name? And you quote '\"$@\"' and eval it, now what happens?\n\nThe eval removes the outer single quotes and then evaluates the\nremaining \"$@\" which leaves the original argument structure completely\nintact: What got passed into git_editor as 3 arguments will remain as\n3 arguments, and even multiple embedded spaces in one argument will\nget preserved perfectly.\n\nIf you don't believe me, throw whatever you want at it.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49375","messageId":"85sl72x0x5.fsf@lola.goethe.zz","threadId":"9339","inReplyTo":"7vtzri3kpr.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-01T23:58:14Z","receivedAt":"2007-08-01T23:58:14Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> It is not just \"won't apply\". What if GIT_DIR had spaces (which\n>> is fine) and single-quotes in it? Wouldn't it percolate down to\n>> $@ because it becomes the leading directory of the temporary\n>> file name? And you quote '\"$@\"' and eval it, now what happens?\n>\n> Ah, I spoke too fast. It is fine --- the shell that actually is\n> doing the eval then interpolates \"$@\". Clever.\n\nSince eval folds all its arguments into a single string separated by\nsingle blanks, actual blanks (which can be multiple or interspersed\nwith newlines) must not yet be seen. The string '\"$@\"' contains no\nblanks and thus gets through unmolested.\n\n-- \nDavid Kastrup, Kriemhildstr. 15, 44793 Bochum\n"},{"id":"49380","messageId":"7vhcni3g39.fsf@assigned-by-dhcp.cox.net","threadId":"9339","inReplyTo":"85sl72x0x5.fsf@lola.goethe.zz","subject":"Re: [PATCH] git-sh-setup.sh: make GIT_EDITOR/core.editor/VISUAL/EDITOR accept commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-02T01:00:58Z","receivedAt":"2007-08-02T01:00:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Junio C Hamano writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> It is not just \"won't apply\". What if GIT_DIR had spaces (which\n>>> is fine) and single-quotes in it? Wouldn't it percolate down to\n>>> $@ because it becomes the leading directory of the temporary\n>>> file name? And you quote '\"$@\"' and eval it, now what happens?\n>>\n>> Ah, I spoke too fast. It is fine --- the shell that actually is\n>> doing the eval then interpolates \"$@\". Clever.\n>\n> Since eval folds all its arguments into a single string separated by\n> single blanks, actual blanks (which can be multiple or interspersed\n> with newlines) must not yet be seen. The string '\"$@\"' contains no\n> blanks and thus gets through unmolested.\n\nYup. I just misread the code.\n\nThanks, applied.\n"},{"id":"49407","messageId":"20070802101056.GA31182@moooo.ath.cx","threadId":"9339","inReplyTo":"85r6mnrs1z.fsf@lola.goethe.zz","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2007-08-02T10:10:56Z","receivedAt":"2007-08-02T10:10:56Z","isPatch":false,"sender":{"key":"matled@gmx.net","avatar":null},"body":"David Kastrup wrote:\n> a) Using system and shell-quoting the filename. Advantage: one can\n> set EDITOR='\"/home/dak/My Programs/editor\"' and have it work.\n> Disadvantage: shell-quoting a file name seems shell- and\n> system-dependent.\n\nWhat about this instead of quoting the argument?\n\n sh -c '$EDITOR \"$1\" \"$2\"' editor +5 /path/to/file\n\n(i.e. for C execvp(\"/bin/sh\", \"-c\", \"$EDITOR \\\"$1\\\" \\\"$2\\\"\", \"editor\",\n \"+5\", \"/path/to/file\"))\n"},{"id":"49410","messageId":"86fy32kz2a.fsf@lola.quinscape.zz","threadId":"9339","inReplyTo":"20070802101056.GA31182@moooo.ath.cx","subject":"Re: Interpreting EDITOR/VISUAL environment variables.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-08-02T10:31:25Z","receivedAt":"2007-08-02T10:31:25Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Matthias Lederhofer writes:\n\n> David Kastrup wrote:\n>> a) Using system and shell-quoting the filename. Advantage: one can\n>> set EDITOR='\"/home/dak/My Programs/editor\"' and have it work.\n>> Disadvantage: shell-quoting a file name seems shell- and\n>> system-dependent.\n\nActually I was talking C here, and the editor is never called from C\nin git but rather from the shell. So this problem is a non-problem\nfor us.\n\n> What about this instead of quoting the argument?\n>\n> sh -c '$EDITOR \"$1\" \"$2\"' editor +5 /path/to/file\n>\n> (i.e. for C execvp(\"/bin/sh\", \"-c\", \"$EDITOR \\\"$1\\\" \\\"$2\\\"\", \"editor\",\n> \"+5\", \"/path/to/file\"))\n\nIt suffers from the fault that it does not work as far as I can see.\n-c does not set the positional parameters.\n\n-- \nDavid Kastrup\n"}]}