[PATCH] Trivial: bzr log fails when start/end_revision is the latest one

[Aaron Bentley]

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

John A Meinel wrote:
> Aaron Bentley wrote:
> 
>>The patch looks correct, but all of this code violates DRY.  There
>>should be one and only one way to determine whether a given revno exists
>>in the branch.
>>
>>Aaron
>>
> 
> 
> I can't say I recognize "DRY", care to define?

Don't Repeat Yourself, which is pretty much the same as SPOT: Single
Point of Truth.

The idea is that you should not have two pieces of code that do the same
thing, because it is harder to ensure that two pieces of code are
correct than to ensure that one is correct, and because it is easier to
maintain that way.

So an improvement would be
def show_log(...):
...
    max_revno = branch.revno()
    def check_revno(revno):
        if revno < 1 or revno > max_revno:
            raise InvalidRevisionNumber(revno)
    def get_revno(revno, default):
        if revno is None:
            return default
        else:
            check_revno(revno)
            return revno
...
    start_revision = get_revno(start_revision, 1)
    end_revision = get_revno(end_revision, max_revno)

Even better would be Branch.check_revno() or Branch.has_revno().

> I guess I would argue that "bzr log" could take a single revision, and
> print its log anywhere in the revision-store, but if given a range it
> should be in the revision-history.

That's okay by me.

> The reason the patch is needed is because of the revision numbers
> starting at 1 but lists starting at 0

Revno 0 refers to the null revision.  It's valid in some contexts, but
we don't want to show the 'commit message'.

, so revno==len(revs) is still a
> valid revision number. So it certainly needs to be added.

Agreed.

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

iD8DBQFDAh5+0F+nu1YWqI0RAlQOAJ9t9ntfhkE10fiZVFV4+UUuWGZumgCdHOge
XqX4JVONctMrN+EzGF/h1o0=
=3wN9
-----END PGP SIGNATURE-----