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

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

[Gerfried Fuchs]

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 <rhonda@deb.at>
---
 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

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

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[Gerfried Fuchs]

* Thomas Rast <trast@student.ethz.ch> [2009-02-18 15:15:53 CET]:
> 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?

 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

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

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[Eric Wong]

Gerfried Fuchs <rhonda@deb.at> 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 <rhonda@deb.at>

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