Julian Foad wrote on Wed, Jun 12, 2013 at 12:22:31 +0100: > Daniel Shahaf wrote: > > > danie...@apache.org wrote on Wed, Jun 12, 2013 at 10:29:35 -0000: > >> Author: danielsh > >> Date: Wed Jun 12 10:29:34 2013 > >> New Revision: 1492134 > >> > >> URL: http://svn.apache.org/r1492134 > >> Log: > >> Add an XFail test for 'svn blame -r 3:1' (where 3 > 1). > >> > >> Not filing an issue since I plan to commit a fix soon. > > What behaviour change are you planning for 'blame'? > > I ask because I don't think it's obvious. > > For example, it would be useful to have a mode that looks forward in > time from a given revision, to find the *next* revision at which each > line is changed after the starting revision. Is that the behaviour > you are planning? >
What I was thinking was to allow blame to consider the sequence of revisions in opposite order: instead of considering the oldest revision the first one in the chain, so `svn blame` output prints the youngest revision that touched a given line, allow 'svn blame' (when invoked as -r M:N with M<N, which is currently an error) to consider the _youngest_ revision as the first one in the chain, so that `svn blame` output will print the _oldest_ revision that touched a given line. In other words: 'svn blame -r 3:1 iota' will, for each line in iota@1, print the oldest revision which changed or removed that line. Does that make sense? Thanks for asking, Daniel > - Julian > > > > Update: Bert found a potential problem with peg revisions here that > > might require revving an RA API, so it'll take me longer to get to > > committing a fix than I planned. > > > > Current WIP patch: > > > > Index: subversion/libsvn_client/blame.c > > =================================================================== > > --- subversion/libsvn_client/blame.c (revision 1492120) > > +++ subversion/libsvn_client/blame.c (working copy) > > @@ -466,7 +466,7 @@ file_rev_handler(void *baton, const char *path, sv > > /* Create the rev structure. */ > > frb->rev = apr_pcalloc(frb->mainpool, sizeof(struct rev)); > > > > - if (revnum < frb->start_rev) > > + if (revnum < MIN(frb->start_rev, frb->end_rev)) > > { > > /* We shouldn't get more than one revision before the starting > > revision (unless of including merged revisions). */ > > @@ -479,7 +479,8 @@ file_rev_handler(void *baton, const char *path, sv > > } > > else > > { > > - SVN_ERR_ASSERT(revnum <= frb->end_rev); > > + /* 1+ for the "youngest to oldest" blame */ > > + SVN_ERR_ASSERT(revnum <= 1 + MAX(frb->end_rev, frb->start_rev)); > > > > /* Set values from revision props. */ > > frb->rev->revision = revnum; > > @@ -578,6 +579,7 @@ svn_client_blame5(const char *target, > > svn_stream_t *last_stream; > > svn_stream_t *stream; > > const char *target_abspath_or_url; > > + svn_revnum_t youngest; > > > > if (start->kind == svn_opt_revision_unspecified > > || end->kind == svn_opt_revision_unspecified) > > @@ -599,11 +601,6 @@ svn_client_blame5(const char *target, > > target_abspath_or_url, > > ra_session, > > start, pool)); > > > > - if (end_revnum < start_revnum) > > - return svn_error_create > > - (SVN_ERR_CLIENT_BAD_REVISION, NULL, > > - _("Start revision must precede end revision")); > > - > > /* We check the mime-type of the yougest revision before getting all > > the older revisions. */ > > if (!ignore_mime_type) > > @@ -674,8 +671,14 @@ svn_client_blame5(const char *target, > > We need to ensure that we get one revision before the start_rev, > > if available so that we can know what was actually changed in the > > start > > revision. */ > > + SVN_ERR(svn_ra_get_latest_revnum(ra_session, &youngest, frb.currpool)); > > SVN_ERR(svn_ra_get_file_revs2(ra_session, "", > > - start_revnum - (start_revnum > 0 ? 1 : 0), > > + start_revnum > > + - (0 < start_revnum && start_revnum > > < end_revnum ? 1 : 0) > > + + (youngest > start_revnum && > > start_revnum > end_revnum ? 1 : 0), > > + /* ### pass MAX(start_revnum, end_revnum) > > as > > peg thru to svn_fs_revision_root() */ > > + /* ### blame 5 fails. */ > > + /* ### check svn_repos_get_file_revs2 > > tests. */ > > end_revnum, include_merged_revisions, > > file_rev_handler, &frb, pool)); > > > > > >> * subversion/tests/cmdline/blame_tests.py > >> (blame_youngest_to_oldest): New test, XFail. > >> (test_list): Run it. > >