Re: [PATCH] fix for interactive merge-callback not supporting spaces in file paths

On 7/17/07, Daniel Rall <dlr@collab.net> wrote:
> On Tue, 17 Jul 2007, David Glasser wrote:
>
> > On 7/17/07, Erik Huelsmann <ehuels@gmail.com> wrote:
>
> > > I've looked through it and I'm sad to have to say your approach won't
> > > really work: Subversion is a library in a (possibly) multithreaded
> > > program. Changing the working dir is not acceptable, because threads
> > > my influence each other when doing that, getting in each others way.
>
> This change is to the command-line binary, not to the libraries. We
> use apr_filepath_set() in three other places in this source file alone.

Good point.

> > It's a tricky little problem. Why don't we tokenize the editor
> > argument with apr_tokenize_to_argv and then use something like
> > apr_proc_create which takes an argv error instead? That's what
> > libsvn_ra_svn does (see its client.c(find_tunnel_agent)).
>
> That sounds like a good approach, assuming it's actually necessary
> give that Augie's patch looks fine already.

True, though Augie's patch (and the current implementation in general)
do make the undocumented assumption that every file that is edited via
the function has no special character in its name; you can check
through the code and discover that this doesn't happen currently, but
it would be nice to not have to worry.

Not that I'm volunteering to write the code this week :-) Augie's
patch is probably fine for now if nobody wants to do the
apr_tokenize_to_argv version.

--dave

-- 
David Glasser | glasser_at_mit.edu | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org