[MERGE] Clean up pull -v

[Vincent Ladeuil]

>>>>> "aaron" == Aaron Bentley <aaron at aaronbentley.com> writes:

    aaron> Vincent Ladeuil wrote:
    >>>>>>> "aaron" == Aaron Bentley <aaron at aaronbentley.com> writes:
    >> Introducing a new public function in bzrlib.log is not.
    >> 
    >> bzrlib.log is already quite messy and includes many functions
    >> that we should deprecate in favor in a move unified API around
    >> show_log (there is a TODO in show_changed_revisions which you
    >> didn't copy when you create show_branch_change).
    >> 
    >> At a minimum, I'd like your new functions to be private, we
    >> *plan* to deprecate show_changed_revisions(), it would be good to
    >> *not* add new functions that will need to be deprecated at the
    >> same time.

    aaron> It seems unlikely to me that show_log will ever be a
    aaron> replacement for show_changed_revisions.

A replacement obviously not. show_changed_revisions already calls
show_log once, where I'd like it to call it twice.

But if they were less public functions in bzrlib.log, it would be
easier to refactor them so that show_log is used consistently
everywhere.

It's already an anomaly that 'pull' and 'push' use
show_changed_revisions where 'missing' use its own functions.

<snip/>
    >> 
    >> Why don't you respect the log_format parameter here ?

    aaron> There's no way for a user to specify a log format
    aaron> anyhow,

Yet. But I did an RFC about the showing revisions in a consistent
way across all bzr commands and that includes allowing user to
specify their preferred log format from the command line (in
addition to the one you care and love in branch.conf :).

And I voted approve on your previous patch regarding pull partly
because you were making the log_format visible just where I
wanted it :-)

    aaron> so I don't see a reason to allow the command to
    aaron> specify it.  Instead, show_branch_change determines it
    aaron> from the branch.

<snip/>

    >> Very nice one, but until we carefully rewrite and review log to
    >> know what should be public or not, please make it private.

    aaron> I don't see why this would be bad functionality to
    aaron> have as public.

I didn't say it was bad, quite the contrary: "Very nice one".

I said: please, stop adding public functions in bzrlib.log, there
are already too many there, even some evil ones that nobody
should use (find_touching_revisions for example). I plan a
deprecation dance there which will already be painful, have
mercy, please.

<internal check> Did I say please twice ? Good :) </internal check>

    aaron> +
    aaron> +def show_flat_log(repository, history, last_revno, lf):
    aaron> +    """Show a simple log of the specified history.
    aaron> +
    aaron> +    :param repository: The repository to retrieve revisions from.
    aaron> +    :param history: A list of revision_ids indicating the lefthand history.
    aaron> +    :param last_revno: The revno of the last revision_id in the history.
    aaron> +    :param lf: The log formatter to use.
    aaron> +    """
    aaron> +    start_revno = last_revno - len(history) + 1
    aaron> +    revisions = repository.get_revisions(history)
    aaron> +    for i, rev in enumerate(revisions):
    aaron> +        lr = LogRevision(rev, i + last_revno, 0, None)
    aaron> +        lf.log_revision(lr)
    aaron> +
    aaron> +
    >> 
    >> Clearly duplicate some code present in bzrlib.log.

    aaron> I don't think it does.

It's a combination of _linear_view_revisions + show_log.

Sorry I shouldn't have say 'clearly', I'm knee-deep (sp?) in
log.py these days...

In summary: I didn't vote reject, this is better than what we
have. I didn't vote tweak either because I thought I was asking
too much for a bare tweak. So I voted resubmit asking for the
functions to be made private so that the work I plan to do on log
is not made more complicated than it already is.

      Vincent