[PATCH] git-svn: Allow using arguments to the editor.

When setting the EDITOR or VISUAL environment variable, one might want
to hand over arguments (like e.g. for not backgrounding a GUI editor but
waiting for it to finish. This patch enables that posibility, before it
did look for a program with the content of the variable, including the
space as filename part. The change is in sync with regular behavior with
various other tools, git itself included.

Signed-off-by: Gerfried Fuchs <redacted>
---
 git-svn.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 83cb36f..d29664c 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -1137,8 +1137,9 @@ sub get_commit_entry {
 
 	if ($_edit || ($type eq 'tree')) {
 		my $editor = $ENV{VISUAL} || $ENV{EDITOR} || 'vi';
+		my (@editor) = split /\s+/, $editor;
 		# TODO: strip out spaces, comments, like git-commit.sh
-		system($editor, $commit_editmsg);
+		system(@editor, $commit_editmsg);
 	}
 	rename $commit_editmsg, $commit_msg or croak $!;
 	{

-- 
1.5.6.5

Gerfried Fuchs wrote:

When setting the EDITOR or VISUAL environment variable, one might want
to hand over arguments (like e.g. for not backgrounding a GUI editor but
waiting for it to finish. This patch enables that posibility, before it
did look for a program with the content of the variable, including the
space as filename part. The change is in sync with regular behavior with
various other tools, git itself included.

[...]

+		my (@editor) = split /\s+/, $editor;

This doesn't handle quoted spaces and such.  launch_editor() seems to
pass the $EDITOR through 'sh -c' if there are any special characters
inside, wouldn't that be appropriate for git-svn too?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

* Thomas Rast [off-list ref] [2009-02-18 15:15:53 CET]:

Gerfried Fuchs wrote:

quoted

When setting the EDITOR or VISUAL environment variable, one might want
to hand over arguments (like e.g. for not backgrounding a GUI editor but
waiting for it to finish. This patch enables that posibility, before it
did look for a program with the content of the variable, including the
space as filename part. The change is in sync with regular behavior with
various other tools, git itself included.

[...]

quoted

+		my (@editor) = split /\s+/, $editor;

This doesn't handle quoted spaces and such.  launch_editor() seems to
pass the $EDITOR through 'sh -c' if there are any special characters
inside, wouldn't that be appropriate for git-svn too?

 I'm not sure where quoted spaces and such are handled very much
differently. How would you quote it that wouldn't make tons of programs
fail? EDITOR='"/my space path/prog"' doesn't work in most applications
because they wouldn't strip the embedded quotes, neither does
EDITOR='/my\ space\ path/prog' work. EDITOR='/my space path/prog' might
work in some (current git-svn) but not in all.

 I was just following common practise with the suggestion, unfortunately
there isn't a real standard for the content of EDITOR and different
people expect different beavior. Most easy way to work around is of
course writing a wrapper script that does call your editor with the
options you want, some people though consider that inconvenient.
Personally I don't care too much either way but when applications
suggest setting options in EDITOR is proper it might be right to follow
that.

 I haven't looked into how git does it directly, but it offers that
posibility:
EDITOR="vim -c 'syn off'" git commit -a

 I wasn't able to figure out any way to use a space-embedded filename in
EDITOR for git - so why should git-svn behave differently here? But you
are right, this very example shows clearly that my approach is
incomplete because it can't handle the 'syn off' part.

 About just doing a system on the combined string
("$editor $commit_editmsg"): It might work somewhat safely if one uses
system($editor . " " . quotemeta($commit_editmsg)) instead. Thinking
about it, seems to make sense. If you want me to update the patch for
that approach just tell me. :)

 Thanks for the response. :)
Rhonda

Gerfried Fuchs wrote:

 I'm not sure where quoted spaces and such are handled very much
differently. How would you quote it that wouldn't make tons of programs
fail? EDITOR='"/my space path/prog"' doesn't work in most applications
because they wouldn't strip the embedded quotes, neither does
EDITOR='/my\ space\ path/prog' work. EDITOR='/my space path/prog' might
work in some (current git-svn) but not in all.

I have EDITOR='emacs -nw' and it works for git, svn and hg.  (Which
are roughly the ones I care about.)

 I haven't looked into how git does it directly, but it offers that
posibility:
EDITOR="vim -c 'syn off'" git commit -a

 I wasn't able to figure out any way to use a space-embedded filename in
EDITOR for git - so why should git-svn behave differently here? But you
are right, this very example shows clearly that my approach is
incomplete because it can't handle the 'syn off' part.

launch_editor (in editor.c) does roughly the following in Perl:

  # set $editor according to config/variables
  my @editor = ($editor);
  if ($editor =~ /["$\t ]/) {
      @editor = ('sh', '-c', $editor . ' "$@"', $editor);
  }
  system(@editor, @rest_of_args);

I'm not a Perl expert but that's my current educated guess of what it
translates to ;-)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Gerfried Fuchs [off-list ref] wrote:

When setting the EDITOR or VISUAL environment variable, one might want
to hand over arguments (like e.g. for not backgrounding a GUI editor but
waiting for it to finish. This patch enables that posibility, before it
did look for a program with the content of the variable, including the
space as filename part. The change is in sync with regular behavior with
various other tools, git itself included.

Signed-off-by: Gerfried Fuchs <redacted>

Hi Gerfried,

I wanted to do something like this a few weeks ago, but generically
enough to be used by other scripts (sh/perl/python/ruby/whatever...).

I haven't had time to follow up on it (and decide on a command-name),
but the thread starts here:

  http://mid.gmane.org/20090201025349.GA22160@dcvr.yhbt.net

-- 
Eric Wong