Re: svn commit: r26034 - in trunk/subversion: include libsvn_delta libsvn_ra_svn

Hi Mike! A few questions/comments...

On Fri, 10 Aug 2007, cmpilato@tigris.org wrote:
...
> Fix a regression I caused in ra_svn with the sparse directories compat
> code.
>
> * subversion/include/svn_delta.h,
> * subversion/libsvn_delta/depth_filter_editor.c
> (svn_delta_depth_filter_editor): Allow svn_depth_unknown (as a no-op).
>
> * subversion/libsvn_ra_svn/client.c
> (DEPTH_FILTER_EDITOR): New.

I don't see such a symbol in this commit. Looks like you meant to
refer to the new DEPTH_TO_RECURSE() macro instead.

> (normalize_depth_values): Remove as unused.
> (ra_svn_update, ra_svn_switch, ra_svn_diff, ra_svn_status): Stop using
> normalize_depth_values(), stop clobbering depth, and setup the
> recurse value ourselves now.

How about "Replace usage of normalize_depth_values() with
DEPTH_TO_RECURSE(), ..."?

...
> --- trunk/subversion/libsvn_delta/depth_filter_editor.c (original)
> +++ trunk/subversion/libsvn_delta/depth_filter_editor.c Fri Aug 10 13:21:00 2007
> @@ -397,8 +397,11 @@
> struct edit_baton *eb;
>
> /* Easy out: if the caller wants infinite depth, there's nothing to
> - filter, so just return the editor we were supposed to wrap. */
> - if (requested_depth == svn_depth_infinity)
> + filter, so just return the editor we were supposed to wrap. And
> + if they've asked for an unknown depth, we can't possibly know
> + what that means, so why bother? */
> + if ((requested_depth == svn_depth_unknown)
> + || (requested_depth == svn_depth_infinity))

This conditional is the functional equivalent of the
SVN_DEPTH_TO_RECURSE() macro (from svn_types.h). Would it make sense
to use the macro instead?

> {
> *editor = wrapped_editor;
> *edit_baton = wrapped_edit_baton;
...
> --- trunk/subversion/libsvn_ra_svn/client.c (original)
> +++ trunk/subversion/libsvn_ra_svn/client.c Fri Aug 10 13:21:00 2007
> @@ -52,6 +52,9 @@
> #define DO_AUTH svn_ra_svn__do_internal_auth
> #endif
>
> +#define DEPTH_TO_RECURSE(d) \
> + (((d) == svn_depth_unknown || (d) > svn_depth_files) ? TRUE : FALSE)
...

This macro definition differs from SVN_DEPTH_TO_RECURSE(). Might be
worth mentioning why here in an inline comment.