[MERGE] Speed up diff spec handling.
aaron at aaronbentley.com
Fri Apr 25 20:34:54 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
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
> 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
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:
"""Return the revision_id derived from the specification"""
"""Return the specified tree"
"""Return the branch object associated with the specification"""
"""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
> 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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar