Daniel Rall <dlr@collab.net> writes:
> Karl, attached is a patch attempting to implement the outstanding
> suggestions from Peter's "Default depth in reporter?" review mail:
>
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=124363
>
> 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?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 21 08:02:26 2007