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