{"thread":{"id":"17876","subject":"[PATCH] git-svn: Allow using arguments to the editor.","startedAt":"2009-02-18T13:47:49Z","lastAt":"2009-02-19T08:10:55Z","messageCount":5,"participants":["Gerfried Fuchs","Thomas Rast","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"105310","messageId":"1234964869-11159-1-git-send-email-rhonda@deb.at","threadId":"17876","inReplyTo":null,"subject":"[PATCH] git-svn: Allow using arguments to the editor.","fromName":"Gerfried Fuchs","fromEmail":"rhonda@deb.at","sentAt":"2009-02-18T13:47:49Z","receivedAt":"2009-02-18T13:47:49Z","isPatch":true,"sender":{"key":"rhonda@deb.at","avatar":null},"body":"When setting the EDITOR or VISUAL environment variable, one might want\nto hand over arguments (like e.g. for not backgrounding a GUI editor but\nwaiting for it to finish. This patch enables that posibility, before it\ndid look for a program with the content of the variable, including the\nspace as filename part. The change is in sync with regular behavior with\nvarious other tools, git itself included.\n\nSigned-off-by: Gerfried Fuchs <rhonda@deb.at>\n---\n git-svn.perl |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 83cb36f..d29664c 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1137,8 +1137,9 @@ sub get_commit_entry {\n \n \tif ($_edit || ($type eq 'tree')) {\n \t\tmy $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';\n+\t\tmy (@editor) = split /\\s+/, $editor;\n \t\t# TODO: strip out spaces, comments, like git-commit.sh\n-\t\tsystem($editor, $commit_editmsg);\n+\t\tsystem(@editor, $commit_editmsg);\n \t}\n \trename $commit_editmsg, $commit_msg or croak $!;\n \t{\n-- \n1.5.6.5\n"},{"id":"105311","messageId":"200902181515.59625.trast@student.ethz.ch","threadId":"17876","inReplyTo":"1234964869-11159-1-git-send-email-rhonda@deb.at","subject":"Re: [PATCH] git-svn: Allow using arguments to the editor.","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-18T14:15:53Z","receivedAt":"2009-02-18T14:15:53Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Gerfried Fuchs wrote:\n> When setting the EDITOR or VISUAL environment variable, one might want\n> to hand over arguments (like e.g. for not backgrounding a GUI editor but\n> waiting for it to finish. This patch enables that posibility, before it\n> did look for a program with the content of the variable, including the\n> space as filename part. The change is in sync with regular behavior with\n> various other tools, git itself included.\n[...]\n> +\t\tmy (@editor) = split /\\s+/, $editor;\n\nThis doesn't handle quoted spaces and such.  launch_editor() seems to\npass the $EDITOR through 'sh -c' if there are any special characters\ninside, wouldn't that be appropriate for git-svn too?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105317","messageId":"20090218144814.GA23938@anguilla.debian.or.at","threadId":"17876","inReplyTo":"200902181515.59625.trast@student.ethz.ch","subject":"Re: [PATCH] git-svn: Allow using arguments to the editor.","fromName":"Gerfried Fuchs","fromEmail":"rhonda@deb.at","sentAt":"2009-02-18T14:48:14Z","receivedAt":"2009-02-18T14:48:14Z","isPatch":true,"sender":{"key":"rhonda@deb.at","avatar":null},"body":"* Thomas Rast <trast@student.ethz.ch> [2009-02-18 15:15:53 CET]:\n> Gerfried Fuchs wrote:\n> > When setting the EDITOR or VISUAL environment variable, one might want\n> > to hand over arguments (like e.g. for not backgrounding a GUI editor but\n> > waiting for it to finish. This patch enables that posibility, before it\n> > did look for a program with the content of the variable, including the\n> > space as filename part. The change is in sync with regular behavior with\n> > various other tools, git itself included.\n> [...]\n> > +\t\tmy (@editor) = split /\\s+/, $editor;\n> \n> This doesn't handle quoted spaces and such.  launch_editor() seems to\n> pass the $EDITOR through 'sh -c' if there are any special characters\n> inside, wouldn't that be appropriate for git-svn too?\n\n I'm not sure where quoted spaces and such are handled very much\ndifferently. How would you quote it that wouldn't make tons of programs\nfail? EDITOR='\"/my space path/prog\"' doesn't work in most applications\nbecause they wouldn't strip the embedded quotes, neither does\nEDITOR='/my\\ space\\ path/prog' work. EDITOR='/my space path/prog' might\nwork in some (current git-svn) but not in all.\n\n I was just following common practise with the suggestion, unfortunately\nthere isn't a real standard for the content of EDITOR and different\npeople expect different beavior. Most easy way to work around is of\ncourse writing a wrapper script that does call your editor with the\noptions you want, some people though consider that inconvenient.\nPersonally I don't care too much either way but when applications\nsuggest setting options in EDITOR is proper it might be right to follow\nthat.\n\n I haven't looked into how git does it directly, but it offers that\nposibility:\nEDITOR=\"vim -c 'syn off'\" git commit -a\n\n I wasn't able to figure out any way to use a space-embedded filename in\nEDITOR for git - so why should git-svn behave differently here? But you\nare right, this very example shows clearly that my approach is\nincomplete because it can't handle the 'syn off' part.\n\n About just doing a system on the combined string\n(\"$editor $commit_editmsg\"): It might work somewhat safely if one uses\nsystem($editor . \" \" . quotemeta($commit_editmsg)) instead. Thinking\nabout it, seems to make sense. If you want me to update the patch for\nthat approach just tell me. :)\n\n Thanks for the response. :)\nRhonda\n"},{"id":"105318","messageId":"200902181643.59516.trast@student.ethz.ch","threadId":"17876","inReplyTo":"20090218144814.GA23938@anguilla.debian.or.at","subject":"Re: [PATCH] git-svn: Allow using arguments to the editor.","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-18T15:43:55Z","receivedAt":"2009-02-18T15:43:55Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Gerfried Fuchs wrote:\n>  I'm not sure where quoted spaces and such are handled very much\n> differently. How would you quote it that wouldn't make tons of programs\n> fail? EDITOR='\"/my space path/prog\"' doesn't work in most applications\n> because they wouldn't strip the embedded quotes, neither does\n> EDITOR='/my\\ space\\ path/prog' work. EDITOR='/my space path/prog' might\n> work in some (current git-svn) but not in all.\n\nI have EDITOR='emacs -nw' and it works for git, svn and hg.  (Which\nare roughly the ones I care about.)\n\n>  I haven't looked into how git does it directly, but it offers that\n> posibility:\n> EDITOR=\"vim -c 'syn off'\" git commit -a\n> \n>  I wasn't able to figure out any way to use a space-embedded filename in\n> EDITOR for git - so why should git-svn behave differently here? But you\n> are right, this very example shows clearly that my approach is\n> incomplete because it can't handle the 'syn off' part.\n\nlaunch_editor (in editor.c) does roughly the following in Perl:\n\n  # set $editor according to config/variables\n  my @editor = ($editor);\n  if ($editor =~ /[\"$\\t ]/) {\n      @editor = ('sh', '-c', $editor . ' \"$@\"', $editor);\n  }\n  system(@editor, @rest_of_args);\n\nI'm not a Perl expert but that's my current educated guess of what it\ntranslates to ;-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105407","messageId":"20090219081055.GB21499@dcvr.yhbt.net","threadId":"17876","inReplyTo":"1234964869-11159-1-git-send-email-rhonda@deb.at","subject":"Re: [PATCH] git-svn: Allow using arguments to the editor.","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2009-02-19T08:10:55Z","receivedAt":"2009-02-19T08:10:55Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Gerfried Fuchs <rhonda@deb.at> wrote:\n> When setting the EDITOR or VISUAL environment variable, one might want\n> to hand over arguments (like e.g. for not backgrounding a GUI editor but\n> waiting for it to finish. This patch enables that posibility, before it\n> did look for a program with the content of the variable, including the\n> space as filename part. The change is in sync with regular behavior with\n> various other tools, git itself included.\n> \n> Signed-off-by: Gerfried Fuchs <rhonda@deb.at>\n\nHi Gerfried,\n\nI wanted to do something like this a few weeks ago, but generically\nenough to be used by other scripts (sh/perl/python/ruby/whatever...).\n\nI haven't had time to follow up on it (and decide on a command-name),\nbut the thread starts here:\n\n  http://mid.gmane.org/20090201025349.GA22160@dcvr.yhbt.net\n\n-- \nEric Wong\n"}]}