[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: --editor-cmd option of the 'pe' subcommand did not work if the path of the external text editor contains space

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Sat, 06 Feb 2010 00:56:23 +0000

On Wed, 2010-02-03, Claude Xu wrote:
> I found the --editor-cmd option of the 'pe' subcommand did not work if the
> path of your external text editor contains space. Here is my environment:
>
> Operating System: Microsoft Windows XP Professional Version 2002 Service
> Pack 3
>
> Release of Subversion: version 1.6.4 (r38063)
[...]
> C:\temp\svn_trunk>svn pe --editor-cmd "C:\Program Files\Vim\vim72\gvim.exe"
> myproperty file1.txt
> 'C:\Program' is not recognized as an internal or external command,
> operable program or batch file.
> svn: system('C:\Program Files\Vim\vim72\gvim.exe svn-prop.tmp') returned 1
>
> The space was still treated as a special separator. Then I tried to escape
> the space with the character ^.
>
> C:\temp\svn_trunk>svn pe --editor-cmd "C:\Program^ Files\Vim\vim72\gvim.exe"
> myproperty file1.txt
>
> It did invoke the external text editor, in this case is vim, successfully
> but then I found another problem that it treated the content behind the
> space, in this case is Files\Vim\vim72\gvim.exe, as the temporary file to
> hold the contents of the property, so it still did not work. :)
>
> I also tested this feature on Windows 2000 Ad Server and the result was the
> same as the one I got on Windows XP. To temporarily solve the problem, we
> can define the external text editor by SVN_EDITOR environment. That works
> perfect even the path of the external text editor has space.
>
> I would like to know whether this is a bug. Thanks.

Yes, there is a bug here. Thank you for reporting it.

I am not sure what the best solution is.

In general I think Subversion should avoid using the C "system()" call
to invoke external programs, but in this case I think some people
already rely on it to be able to pass a complex command line rather than
simply the path to an executable, so we can't change that in a way that
breaks such usage. I think that means we have to continue to use the
system() call.

If anybody is interested in fixing this, the Subversion code is in
subversion/svn/util.c, svn_cl__edit_file_externally() (and note that
svn_cl__edit_string_externally() is very similar). It simply passes to
the system() call a concatenation of the "editor" value, a space and the
name of the temporary file. I think we should be quoting the two
arguments in the string before passing it to the system() call. It looks
to me like the system() call is being executed wrongly - but maybe it is
just being executed in a surprising and inconsistent manner that is not
expected to work unless we quote the arguments properly.

For a bit of a clue how the quoting could be done, see the function
_quote_arg() in subversion/tests/cmdline/svntest/main.py.

- Julian
Received on 2010-02-06 01:57:04 CET

This is an archived mail posted to the Subversion Dev mailing list.