[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: API review - svn_dirent_uri.h

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 15 Jun 2011 17:52:28 +0100

On Wed, 2011-06-15 at 12:42 -0400, Greg Stein wrote:
> On Wed, Jun 15, 2011 at 12:32, Julian Foad <julian.foad_at_wandisco.com> wrote:
> > On Fri, 2011-06-10 at 09:06 +0100, Julian Foad wrote:
> >> I (Julian Foad) wrote:
> >> > On Thu, 2011-06-09, I (Julian Foad) wrote:
> >> > > svn_dirent_uri.h
> >> > > svn_relpath_internal_style()
> >> > > svn_relpath_local_style()
> >> > > # These two are inappropriate: only dirents have a 'local' style.
> >> >
> >> > I removed the former and made the latter private in r1133964.
> >
> > Bert pointed out that there is a desire and precedent for the Windows
> > client to display in-repository relpaths using the Windows "\"
> > separator, so svn_relpath_local_style() may be wanted. It was currently
> URLs use forward slashes. Backslashes are not proper separators. The
> above does not make sense.
> >...
> > Many callers of svn_dirent_skip_ancestor(), on the other hand, expect
> > the 'child' input to be returned if it is not in fact a child. I am
> > splitting the function into two:
> >
> > svn_dirent_skip_ancestor() - return NULL if not a child
> > [some other function name] - return 'child' input if not a child
> PLEASE not another function. There are already too many variants of
> the same thing:
> svn_dirent_is_child()
> svn_dirent_skip_ancestor()
> svn_dirent_is_ancestor()
> These are all trivial variants which leads the reader to say "umm...
> which one is this?" and the developer to say "umm... which do I use
> here?". Please do not add a fourth... it will only exacerbate an
> already awful situation.
> If anything, please reduce the number to ONE.

Absolutely. I intend to reduce the number. But the first and more
important thing is to reduce the number of *semantic* variations, rather
than strictly the number of exposed API symbols. After this round of
changes, the semantics of *_skip_ancestor will match *_is_ancestor
exactly, so the latter becomes strictly redundant and I'll certainly
consider removing it, and even if I don't then its implementation will
be a simple wrapper rather than the current long-winded and separate

As a second change, I think *_is_child() should go, because that's a
semantic variant that can easily be derived from _skip_ancestor.
svn_dirent_is_child() was already public in 1.6, though, so that one
must stay, but we could deprecate that and prefer using

I guess, to speed this up in readiness for 1.7 branching, the sensible
thing would be to make the _is_child functions private at first.
Adapting their callers to use _skip_ancestor instead will be a bigger
change so that can come later.

So that's what I intend to do.

And this step of splitting _dirent_skip_ancestor into two variants with
the two different semantic meanings which are currently conflated - does
that now make sense to you as it does to me?

- Julian
Received on 2011-06-15 18:53:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.