[MERGE] hack logging to not always do whole history operations

[Aaron Bentley]

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

Michael Hudson wrote:
> In the continued spirit of
> 
> http://blog.orebokech.com/2008/03/emacs-in-bzr-initial-impressions.html
> 
> I got to thinking that log --line and log --short do not need to do
> whole-history operations.  The result was the attached patch, which is
> rather hackish.

bb:resubmit

I think it would be nice to avoid the whole-history operations where
possible, but I think this is a bit too hackish to merge as-is.

There 4 lines which exceed 79 columns.

The _show_log method is getting really long, and that makes it much
harder to read.  Splitting the two iter_revision implementations out
into standalone functions would help a lot.  Or making a
ShowLogController class and making them methods, etc.

You've also disabled a significant optimization by fetching single
revisions at a time, and it doesn't seem necessary to me.

I don't actually think there's a need for two separate implementations
of iter_revision.  It doesn't do any of the expensive whole-history
operations.

I think what's needed here is to change the way we generate
view_revisions, so that it's cheap when no merge revisions are being shown.

The stock implementation *does* assume the view_revisions are a list,
but changing it to use a generator wouldn't be hard.

> -        
> +    generate_delta = verbose and getattr(lf, 'supports_delta', False)
>      rev_tag_dict = {}
>      generate_tags = getattr(lf, 'supports_tags', False)
>      if generate_tags:
>          if branch.supports_tags():
>              rev_tag_dict = branch.tags.get_reverse_tag_dict()
>  
> -    generate_delta = verbose and getattr(lf, 'supports_delta', False)

^^^ I don't understand the purpose of moving this line

> +    if not generate_merge_revisions and start_revision is end_revision is None and direction == 'reverse':
> +        def iter_revisions():
> +            num = 9
> +            revno, revid = branch.last_revision_info()
> +            if revno == 0:

^^^ revnos are considered display data, and control logic should rarely
depend on them.  We would generally test revid == revision.NULL_REVISION

But also, new code should eschew "revid" for "revision_id".

> +                for i in range(num):
> +                    revision = repository.get_revision(revid)
> +                    revisions.append(revision)
> +                    if not revision.parent_ids:
> +                        break
> +                    revid = revision.parent_ids[0]

This is essentially a reimplementation of
Repository.iter_reverse_revision_history.  It would better to use that
instead.

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

iD8DBQFH5rV00F+nu1YWqI0RArfuAJ4pI7vtXIBNZVaD4GVWz62egJ64pACfYcAR
Am8yt5zZVPrE/f0CKqbdTgc=
=zqz9
-----END PGP SIGNATURE-----