patchgit-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 <rhonda@deb.at>
---
 git-svn.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Gerfried Fuchs wrote:

[...]

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

 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 have EDITOR='emacs -nw' and it works for git, svn and hg. (Which are roughly the ones I care about.)

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 ;-)

Gerfried Fuchs <rhonda@deb.at> wrote:

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