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

Re: svn commit: r997249 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/patch.c svn/cl.h svn/main.c svn/patch-cmd.c tests/cmdline/patch_tests.py tests/libsvn_client/client-test.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Fri, 17 Sep 2010 02:01:27 +0200

Stefan Sperling wrote on Wed, Sep 15, 2010 at 20:42:55 +0200:
> On Wed, Sep 15, 2010 at 04:51:15PM +0200, Daniel Shahaf wrote:
> > stsp_at_apache.org wrote on Wed, Sep 15, 2010 at 10:05:15 -0000:
> > > Author: stsp
> > > Date: Wed Sep 15 10:05:14 2010
> > > New Revision: 997249
> > >
> > > URL: http://svn.apache.org/viewvc?rev=997249&view=rev
> > > Log:
> > > Add an --old-patch-target-names option to svn patch.
> > > This option is useful in two cases:
> > >
> > > 1) A patch contains names like
> > > --- foo.c
> > > +++ foo.c.new
> > > and should be applied to foo.c.
> > >
> > > 2) A patch contains names like
> > > --- foo.c.orig
> > > +++ foo.c
> > > and should be applied in reverse to foo.c, e.g. to undo prior application
> > > of the patch.
> >
> > What happens if a user forgets to supply the new option? (Does svn
> > complain that 'foo.c.new is nonexistent/unversioned'?)
>
> $ svn patch test.diff
> Skipped missing target: 'foo.new'
> Summary of conflicts:
> Skipped paths: 1

Okay. So it always uses the /^+++/ path. Pro: simple and predictable,
con: sometimes it's possible to guess the correct path via other means
(the Index: line, or extension stripping, etc; see the other thread).

>
> > For case (2): suppose I have such a patch (was emailed to me). I
> > applied it using 'svn patch $patchfile'. Now I want to unapply it; so
> > so I use 'svn patch --rd $patchfile' or 'svn patch --rd --optn $patchfile'?
> > AIUI, currently only the latter will work?
>
> Which one works depends on the way path names appear in the patch.
>
> Say you have a diff produced with svn diff.
> That will work with either combination of options.
>
> $ svn diff
> Index: alpha
> ===================================================================
> --- alpha (revision 2)
> +++ alpha (working copy)
> @@ -1 +1,2 @@
> alpha
> +aaa
>
> You need the new option only to handle cases where people used e.g. the
> UNIX diff tool to create a patch using left/right names they made up
> while producing the diff.
>
> > Is the UI "'svn patch' always uses the filename in the /^+++/ line"?
>
> Yes. By default, that's the name that is used.
>
> If you reverse the diff, the filenames get swapped as well.
> So if you use the --optn option, the filenames get swapped again, or not at
> all, depending which way you look at it :)
>

My point was that I think --rd shouldn't swap the filenames.

> This UI may not be ideal, and I'm open for suggestions on how we could
> improve it. Maybe there's a way to handle this without user interaction,
> or a better name for the new option?
>
> The UNIX patch tool prompts for a filename when it cannot find a target.
> We could do the same, defining yet another prompt callback type.
>
> But I don't like the fact that UNIX patch does not do tab-completion in its
> prompt. I think the lack of tab-completion really affects usability.
> So if svn had a prompt for this, I'd like it to support tab-completion.
>

Tab completion? Can we do that using APR or similar?

> So the new --optn option is an easier way to implement this.
> Maybe it's sufficient? Note that having the option around doesn't hurt
> even if we decide to implement prompting later.

Devil's advocate: It's a very narrow-scoped option, and might become
obsolete if we extend this part of the code a bit.

>
> Stefan
Received on 2010-09-17 01:02:40 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.