Daniel Rall <firstname.lastname@example.org> writes:
> Karl, attached is a patch attempting to implement the outstanding
> suggestions from Peter's "Default depth in reporter?" review mail:
> I've dropped the DEPTH parameter from the svn_repos_begin_report3()
> API and its callers, in favor of writing the DEPTH passed to
> set_path() to the report, and subsequently using that depth during the
> editor drive. An unknown value passed to write_path_info() will
> default to report_baton_t->default_depth (a field which I renamed from
> depth). I've got svn_repos_begin_report3() setting
> report_baton_t->default_depth to svn_depth_infinity, but perhaps it
> should be smuggling in a depth of svn_depth_unknown?
> You'll notice that I had to change svn_wc_crawl_revisions3() to use
> the passed-in DEPTH parameter (when known) for the first call to
> set_path() (there's another call to set_path() near the beginning of
> this routine which may need similar treatment).
Dan, thanks for doing this! I will try to take it further tomorrow
(bad timing -- long doctor's appointment in the middle of the day),
and Thursday is fortunately more free. Sorry for the delay; I want to
get this merged too, but I think Malcolm and Peter are right about
fixing this bit of API lossage first.
I've looked over the patch, it seems like the right idea. Very +1 on
the rename of "baton->depth" to "baton->default_depth" :-). When you
wrote that you've "got svn_repos_begin_report3() setting
report_baton_t->default_depth to svn_depth_infinity, but perhaps it
should be smuggling in a depth of svn_depth_unknown?", I got a bit
confused, though. Here's the situation in the patch as I understand it:
There is an svn_repos_begin_report2() (no "3", right?), and the
difference between it and svn_repos_begin_report() is that the newer
one takes neither recurse nor depth (because set_path will do it).
Right now, you're implementing svn_repos_begin_report2() as a wrapper
around svn_repos_begin_report() (in other words, backwards from the
usual way we do it), and you're passing TRUE for the 'recurse' flag to
svn_repos_begin_report(), which translates to svn_depth_infinity on
this [new] line in svn_repos_begin_report():
b->default_depth = SVN_DEPTH_FROM_RECURSE(recurse);
So your intention is that that's all placeholder code until we get
things figured out, and that we'll do the compatibility wrapping in
the usual direction in the final cut, right?
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Wed Mar 21 08:02:26 2007