[MERGE] Speed up diff spec handling.

[Aaron Bentley]

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

John Arbash Meinel wrote:
> Aaron Bentley wrote:
> | But this is significant scope creep, because branch locations on
> | RevisionSpecs are a newer interface that is frequently ignored.  I think
> | there are many callers who still assume that any revision-id returned is
> | in the context of the default branch.
> 
> 
> Good enough, I guess. If there are tons of tests that fail with this
> activated,
> then not a big deal. 

That's the unfortunate thing-- lots of commands break, but hardly anyone
is testing the branch spec, so only two tests break.  And I assume *no*
tests broke when the as_revision_id change was made, even though 11
commands regressed.

The thing is, we shouldn't *have* to test that the branch spec works for
every single command.  I think more of the command processing should be
factored out, so that commands can't make this mistake.

What would that look like?

Perhaps cmd_log would use something like this:

start_holder, end_holder = get_start_end(revisions, location)

These holder objects would provide this interface:

class Holder(object):

    def get_revision_id(self):
        """Return the revision_id derived from the specification"""

    def get_tree(self):
        """Return the specified tree"

    def get_branch(self):
        """Return the branch object associated with the specification"""

    def get_filename(self):
        """Return the filename associated with the branch"""

The holder object would take care of evaluating the revision spec, so
that we would only need to test the *holder* object, not the commands
that used it.

cmd_annotate would presumably do

holder = get_one(location, revisions)
tree = holder.get_tree()
lines = tree.annotate(tree.path2id(holder.get_filename()))

> We still have revisions specs which are broken in
> those
> cases, but ....

But not a regression, yes.  Because those revision specs never worked in
the first place.  (Actually, I think there's only one revision spec that
supports branch locations: the revno spec.)

> So I'm pretty sure that makes this approved. Not voting because BB
> doesn't like
> it when I vote far down in a thread.

The original reason was because I didn't want to turn BB into a threaded
mail reader just so that voting worked.  I can try using the References:
header, but that will be ambiguous when more than one merge request was
submitted in a thread.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIEjJe0F+nu1YWqI0RAnGaAJ0VSrsXaM3IVP0EpG8Ixo5P8B4NHgCdFOuU
ZBvCk3Rr7BzXcJvcFyzokNo=
=FltY
-----END PGP SIGNATURE-----