[MERGE] Clean up pull -v

[Aaron Bentley]

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.

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

>     aaron>  bzr 1.10 2008-12-05
> 
>     aaron> === modified file 'bzrlib/builtins.py'
>     aaron> --- bzrlib/builtins.py	2008-12-11 06:03:57 +0000
>     aaron> +++ bzrlib/builtins.py	2008-12-12 04:49:02 +0000
>     aaron> @@ -801,15 +801,8 @@
>  
>     aaron>              result.report(self.outf)
>     aaron>              if verbose and result.old_revid != result.new_revid:
>     aaron> -                old_rh = list(
>     aaron> -                    branch_to.repository.iter_reverse_revision_history(
>     aaron> -                    result.old_revid))
>     aaron> -                old_rh.reverse()
>     aaron> -                new_rh = branch_to.revision_history()
>     aaron> -                log_format = branch_to.get_config().log_format()
>     aaron> -                log.show_changed_revisions(branch_to, old_rh, new_rh,
>     aaron> -                                           to_file=self.outf,
>     aaron> -                                           log_format=log_format)
>     aaron> +                log.show_branch_change(branch_to, self.outf, result.old_revno,
>     aaron> +                                       result.old_revid)
> 
> Why don't you respect the log_format parameter here ?

There's no way for a user to specify a log format anyhow, so I don't see
a reason to allow the command to specify it.  Instead,
show_branch_change determines it from the branch.

Providing a log_formatter parameter would increase the number of
parameters to show_branch_change, without any useful effect.

>     aaron> +                
>     aaron>          finally:
>     aaron>              branch_to.unlock()
>  
> 
>     aaron> === modified file 'bzrlib/log.py'
>     aaron> --- bzrlib/log.py	2008-11-27 03:18:40 +0000
>     aaron> +++ bzrlib/log.py	2008-12-12 02:43:42 +0000
>     aaron> @@ -1004,6 +1004,101 @@
>     aaron>                   search=None)
>  
>  
>     aaron> +def get_history_change(old_revision_id, new_revision_id, repository):
>     aaron> +    """Calculate the uncommon lefthand history between two revisions.
>     aaron> +
>     aaron> +    :param old_revision_id: The original revision id.
>     aaron> +    :param new_revision_id: The new revision id.
>     aaron> +    :param repository: The repository to use for the calcualtion.
>     aaron> +
>     aaron> +    return old_history, new_history
>     aaron> +    """
> 
> Very nice one, but until we carefully rewrite and review log to
> know what should be public or not, please make it private.

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

>     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.

I don't think it does.

> Finally, show_changed_revisions() is also used by push, why not
> upgrading it too ?

I didn't know it was used there too.

Aaron