Re: svn commit: r33597 - branches/diff-repos-wc/subversion/libsvn_client

On Sun, 2008-10-12 at 05:12 +0200, Neels J. Hofmeyr wrote:
> This is a summary for those interested in discovering scores of broken diff
> cases (on trunk, marked "*** NOTE") or in the current state of summarizing
> diff re- implementation, and otherwise just a note to myself.

Hi Neels.

I fixed diff_tests-45 on this branch.

The next thing I looked at, to get more of the tests working, is the way
it accidentally prints a double path "path/to/diff/path/to/diff" for
each node:

[[[
$ svn diff --summarize -c33597 subversion/libsvn_client/
[...]
M subversion/libsvn_client/subversion/libsvn_client/diff.c
]]]

The attached patch solves this for WC paths, but it also makes URLs be
printed as relative to the target specified in the command:

[[[
$ svn diff -c33598 $SVN_REPOS/branches/diff-repos-wc/subversion
Index: libsvn_client/diff.c
===================================================================
--- libsvn_client/diff.c (revision 33597)
+++ libsvn_client/diff.c (revision 33598)
]]]

Pros: this is neat, and it is what plain "svn diff" does.
Cons: this is ambiguous with multiple targets, and is an unnecessary
change in behaviour.

The previous (trunk, svn-1.5.3) behaviour for a repos:repos change is:

  diff (normal): Paths relative to CWD. URLs relative to TARGET.
  diff --summarize: Paths relative to CWD. URLs ABSOLUTE.

I think we should keep that set of behaviours unchanged, at least for
now while we are extending "diff --summarize". I don't think we will
want to change "summarize" to print TARGET-relative URLs like plain
"diff" does. If we want to change plain diff to print absolute URLs like
"summarize" does, that is a separate issue.

So, my patch is no good.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org