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

[Gerfried Fuchs <rhonda@deb.at>]

* 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