get_record_stream(include_delta_closure=???) wrong meaning

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

While trying to figure out how the logic was working I came across this
flag.

The docstring says:
~        :param include_delta_closure: If True then the closure across any
~            compression parents will be included (in the opaque data).


However, when looking at the implementations and callers, they seem to
actually mean "as_fulltexts=True".

Specifically, 'fetch()' passes False, because it knows it only wants the
deltas.

The Knit data converters use True, as does iter_files_bytes. And when
you look at the implementation:

~        if include_delta_closure:
~            # XXX: get_content_maps performs its own index queries;
allow state
~            # to be passed in.
~            text_map, _ = self._get_content_maps(present_keys)
~            for key in present_keys:
~                yield FulltextContentFactory(key, parent_map[key], None,
~                    ''.join(text_map[key]))


^- This does return the "delta closure" deltas in the data stream, it
unpacks everything into fulltexts and returns that.

Which is okay, considering it is actually what we want, and it unpacks
everything in a proper order (such that it performs well, doesn't have
to double extract things, etc.).

I'm mostly just arguing that the parameter is misleading, and it should
be something like "return_fulltexts = True" or something else like it.

This may be something that Robert was thinking in terms of a Remote api,
where it would return the deltas, and then the local abstraction might
turn them back into fulltexts. But it seems like it would be better to
request what you actually are asking for.

At least, that is my thought.
John
=:->

PS> I was certainly happy to see that iter_files_bytes is implemented
such that requesting multiple versions of the same file will extract
both texts concurrently. So it does what I wanted, but it was quite hard
to track down that it was actually doing so.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkhpIsoACgkQJdeBCYSNAAMZmQCffpv6oLy20TXW3JIHOFa6q8EH
LaAAn1b/muopM4FmnjWnUlo8AcFXrLd3
=j425
-----END PGP SIGNATURE-----