On Mon, Sep 13, 2010 at 05:50:05PM +0100, Julian Foad wrote:
> On Mon, 2010-09-13, stsp_at_apache.org wrote:
> > * subversion/libsvn_diff/parse-diff.c
> > (svn_diff_parse_next_patch): When generating a reverse diff, do not swap
> > the old and new filenames of the patch. The old filename in a unidiff
> > is often useless (e.g. it contains a ".orig" extension), but the new
> > filename should always be valid.
>
> Sounds odd. I don't doubt this change is correct, but I'm sure the
> reasoning for it shouldn't have anything to do with what kind of
> filenames you think people might use :-)
Ah, good point. Let's see. The usual use cases for reverse diffs are:
1) someone accidentally produced the patch in the wrong order
2) you've applied a patch and you now want to revert it
So far, svn patch has been addressing use case number 1.
But of course I forgot about use case 1 when I made this commit, and I was
thinking of use case 2.
You might say that "svn revert" is what people should use for use case 2.
But the underlying problem I'm trying to fix is that we should have a way
to revert changes that svn patch has made to locally addded nodes. At the
moment "svn revert" doesn't deal with this nicely. The two ways I see are:
a) make svn revert smart enough to deal with it, e.g. by having
svn patch store the pristine added file/directory in the pristine
store/wc.db before patching it, and then have "svn revert" revert to
the pristine added state, and only if run twice revert the addition
itself (or make it depend on a command line option)
b) make svn patch --reverse-diff work for locally added files,
which currently doesn't work because of a bug -- svn patch
says the change had already been applied and does nothing.
I was aiming to fix the bug I found in case b) and ran into a few issues,
one of which was this filename problem. But it seems we'll need a better
approach, because my commit broke use case 1) above (the diff produced
in wrong order by accident) :(
Thanks for pointing this out, I'll think more about it.
Stefan
Received on 2010-09-13 19:55:35 CEST