On Wed, 2010-03-31 at 08:15 -0400, C. Michael Pilato wrote:
> Stefan Sperling wrote:
> > I too remember being confused as to which interface I should be using.
> > And I think having is_ancestor not be the reverse of is_child is a bad idea.
> > We'll likely find mis-use of either interface in the code base, where
> > the author didn't realise the subtle semantics of the interface.
> >
> > +1 on settling on one variant (I don't really care which), deprecating
> > the others, and updating callers.
>
> We had one variant before -- svn_path_is_child(). But I think people got
> tired of having to implement _is_ancestor() as a pool-requiring wrapper
> around that function and a new public API was born.
>
> The is_child() variant is clearly the most powerful one, having not just the
> ability to answer the question but also to return the path relative to the
> ancestor (which is used in various places in our codebase). But perhaps
> there's a unified API that do this for us:
>
> svn_*_check_ancestry(svn_boolean_t *is_ancestor,
> const char **child_relpath, /* may be NULL */
> const char *path1,
> const char *path2,
> apr_pool_t *result_pool) /* may be NULL */
Ew, ugly! There's no problem having multiple functions in the API (one
to ask the question, one to return the relpath) if their semantics are
identical. It's the differing semantics and poor naming (is_xxx for a
non-bool) that was the main problem.
Plus: let's call the potential-parent and potential-child args 'parent'
and 'child' rather than '1' and '2', as that's impossible to infer
otherwise.
- Julian
> The function always answers the is-ancestor question, and -- if you provide
> a non-NULL child_relpath and result_pool, it will fill in the child path
> relative to the ancestor for you.
>
> Is that sane?
>
Received on 2010-03-31 14:35:14 CEST