You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Michael Sinz <Mi...@sinz.org> on 2005/11/08 13:52:55 UTC
Re: [Issue 2434] Spaces in path to EDITOR cause trouble
On 11/8/05, Julian Foad <ju...@btopenworld.com> wrote:
> > kfogel@tigris.org writes:
> >
> >>http://subversion.tigris.org/issues/show_bug.cgi?id=2434
> >
> >>------- Additional comments from kfogel@tigris.org Mon Nov 7 09:59:04 -0800 2005 -------
> >>So, anyone know if there's a Cygwin (or APR) function for escaping a
> >>path so that it can be invoked as an executable reliably on Cygwin?
> >>Should we just put the executable path in single quotes or
> >>something?
>
> This is a cross-platform issue, not specific to CygWin.
>
> The implementation uses "system()" which requires that the program name and
> arguments are represented by a single string with spaces delimiting program
> name and arguments. We should use svn_io_run_command which keeps the program
> name and arguments separately intact.
Please don't change this - there is a perfectly good way to address
the spaces issue.
> Philip Martin wrote:
> > There was a patch to do something like that in the past. The current
> > code allows things like
> >
> > SVN_EDITOR="LD_LIBRARY_PATH=/some/lib /some/program"
> >
> > and the patch would have stopped that working.
>
> It also allows initial arguments to be specified, like SVN_EDITOR="/bin/editor
> --option=x".
>
> I doubt it was intended to allow things like that; that looks like an
> accidental artifact of the implementation. In the absence of any further
> argument for keeping the current behaviour I recommend calling it a bug and
> fixing it, i.e. treat the whole variable as the name of a program.
No, that is needed - some editors on some platforms detach by default
and you need
to provide some options to the editor.
All that you need to do is "quote" the program name part of the variable and it
will work just as expected. I tried this on Windows and Linux and all is well.
For example, on Windows I did this test to see if I can make the editor start
in read-only mode:
set SVN_EDITOR="C:\Program Files\Posix Toolkit\bin\vi.exe" -R
svn commit
On Linux the way you get quotes into the variable depends on your shell
escaping mechanisms or if your shell supports dual quote types. For example:
setenv SVN_EDITOR '"/home/mks/path space/bin/vi" -R'
svn commit
Both worked exactly as I would expect and handled the space in the command
without changing SVN or removing the ability to provide command line options
to the editor.
--
Michael Sinz Technology and Engineering Director/Consultant
"Starting Startups" mailto:Michael.Sinz@sinz.org
My place on the web http://www.sinz.org/Michael.Sinz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 2434] Spaces in path to EDITOR cause trouble
Posted by kf...@collab.net.
Michael Sinz <Mi...@sinz.org> writes:
> Attached is a rough patch to the faq.html about this issue. I am not
> sure about the specific wording or the level of the intended audience.
> But it does show the way to do this in both Windows and Unix/bash.
> (I figure most Unix users can handle the translation to other shell
> constructs.)
Applied in r17311, with only minor tweaks. Thanks!
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 2434] Spaces in path to EDITOR cause trouble
Posted by Michael Sinz <Mi...@sinz.org>.
kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>
>>Excellent. That may be the best solution for invoking an editor:
>>leave the code alone and document the feature. Does anybody see a
>>problem with that?
>
>
> No, +1, I think it's by far the best solution! Thanks, Michael, for
> pointing it out.
Attached is a rough patch to the faq.html about this issue. I am not
sure about the specific wording or the level of the intended audience.
But it does show the way to do this in both Windows and Unix/bash.
(I figure most Unix users can handle the translation to other shell
constructs.)
--
Michael Sinz Technology and Engineering Director/Consultant
"Starting Startups" mailto:michael.sinz@sinz.org
My place on the web http://www.sinz.org/Michael.Sinz
Re: [Issue 2434] Spaces in path to EDITOR cause trouble
Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> Excellent. That may be the best solution for invoking an editor:
> leave the code alone and document the feature. Does anybody see a
> problem with that?
No, +1, I think it's by far the best solution! Thanks, Michael, for
pointing it out.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 2434] Spaces in path to EDITOR cause trouble
Posted by Julian Foad <ju...@btopenworld.com>.
Michael Sinz wrote:
> On 11/8/05, Julian Foad <ju...@btopenworld.com> wrote:
>
>>The implementation uses "system()" which requires that the program name and
>>arguments are represented by a single string with spaces delimiting program
>>name and arguments. We should use svn_io_run_command which keeps the program
>>name and arguments separately intact.
>
> Please don't change this - there is a perfectly good way to address
> the spaces issue.
>
>
>>Philip Martin wrote:
>>
>>>There was a patch to do something like that in the past. The current
>>>code allows things like
>>>
>>>SVN_EDITOR="LD_LIBRARY_PATH=/some/lib /some/program"
>>>
>>>and the patch would have stopped that working.
>>
>>It also allows initial arguments to be specified, like SVN_EDITOR="/bin/editor
>>--option=x".
>>
>>I doubt it was intended to allow things like that; that looks like an
>>accidental artifact of the implementation. In the absence of any further
>>argument for keeping the current behaviour I recommend calling it a bug and
>>fixing it, i.e. treat the whole variable as the name of a program.
>
> No, that is needed - some editors on some platforms detach by default
> and you need
> to provide some options to the editor.
Strictly speaking, that ability to specify a multi-word command line is USEFUL,
but not NECESSARY. The same issue is present for invoking external "diff"
command, but we handle that a different way, requiring an intermediate program
or script if it needs to modify arguments or environment variables.
> All that you need to do is "quote" the program name part of the variable and it
> will work just as expected. I tried this on Windows and Linux and all is well.
Excellent. That may be the best solution for invoking an editor: leave the
code alone and document the feature. Does anybody see a problem with that?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org