[PATCH 1 of 2] Optionally display chains of empty

[Ry4an Brase]

On Tue, Oct 21, 2008 at 09:16:38 +0000, dirkjan at ochtman.nl wrote:
> Ry4an Brase <ry4an-hg <at> ry4an.org> writes:
> > +      self.decendempties = self.configbool("web", "decendempties", False)
> 
> As Matt mentioned, let's not make it optional. I think we can make it efficient
> enough that the performance cost is not that important.

I'm all for that idea; I was just trying to increase the likelihood of
patch adoption.

> > +        elements = remain.split('/')
> > +        if len(elements) == 1:
> > +            files[remain] = (f, n)
> > +        elif not web.decendempties:
> > +            dirs[elements[0]] = {}
> > +        else:
> > +            h = dirs
> > +            for index in range(0, len(elements)-1):
> > +                if elements[index] not in h:
> > +                    h[elements[index]] = {}
> > +                h = h[elements[index]]
> > +            h[None] = 1
> 
> I have a feeling we might be able to do this a little more efficiently. As we
> walk the entries from the manifest, we can keep a dict with dir paths (of
> arbitrary depth) that have only one file in them. As soon as we encounter
> another file in the same (arbitrary depth) dir, we prune that dir from our
> list. Does that sound sensible? (I still have to take a good look at our
> manifest-walking code in webcommands to see if this would work.)

I started down that implementation path initially (prototyping in Perl
I'll admit) but found I had to keep the dirpaths in the dict w/ a value
indicating "more than one" to avoid re-adding them the next time they
were seen.  By the time I was handling that case and a few other edge
cases correctly I'd settled on building a full tree of directories.

Notice that by stopping the for loop one position before the end of the
'elements' list I avoid creating leaf-nodes for files and instead use
the key 'None' with a value of '1' to indicate "some files live here"),
which reduces the dict-tree size greatly.  There are probably still a
few cases that could be short-circuited with an early exit from that
'for' loop, but since I didn't assume I could count on the manifest
being sorted (and didn't want to needlessly do so) and because the
preceding 'remain' code saves on the descending of the whole tree for
all but repo-root views I made my peace with the code above.  If you'd
like me to revisit it I'm happy to do so.

> Also, this is strictly a UI thing, right? HTTP requests to
> /file/tip/some/intermediate/dir/ would still work?

Positively.  That's handled by the 'remain' stuff earlier, which lops
off any preceding path.  I've got a repo w/ this patch applied which I
can push outside of my firewall if you'd like to get a feel for how it
works in practice thus far.

Thanks for your time,

-- 
Ry4an Brase - http://ry4an.org/