On Wed, Mar 31, 2010 at 11:34:35AM +0200, Bert Huijben wrote: > > > > -----Original Message----- > > From: Greg Stein [mailto:[email protected]] > > Sent: woensdag 31 maart 2010 2:47 > > To: [email protected] > > Subject: redundant path functions > > > > The following functions seem very redundant. Because each is slightly > > different, I always have to compare/contrast them to isolate their > > differences. It would be much better if we could pick JUST ONE, and > > run with that: > > > > svn_*_is_child() > > svn_*_is_ancestor() > > svn_*_skip_ancestor() > > > > I do realize that we published the svn_dirent_* functions in 1.6, but > > we don't have to carry their analogues to the new functions. (and > > possibly even deprecate the 1.6 funcs) > > > > It seems that the is_child variant is the only function needed. The > > is_ancestor() is merely is_child() != NULL, and the skip_ancestor is > > simply pool=NULL. > > > > Am I missing something, and/or can/should we remove this redundancy? > > There is one difference between is_child and the ancestors functions: How > they handle the case where the path is the ancestor itself. > > The is_child function returns NULL, while is_ancestor returns true. > Skip ancestor removes the ancestor (prefix) and returns "" for the ancestor > itself and leaves paths that don't have the ancestor as-is.
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. Stefan
