[MERGE] tags in log output

[Erik Bågfors]

On 4/11/07, Alexander Belchenko <bialix at ukr.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Erik Bågfors пишет:
> > Here is an updated version, based on input from you guys.
> >
> > What do you think?
>
> I think some moments could be slightly reworked:
>
> === modified file 'bzrlib/log.py'
> - --- bzrlib/log.py     2007-04-11 01:04:57 +0000
> +++ bzrlib/log.py       2007-04-11 09:14:28 +0000
> @@ -273,7 +273,7 @@
>                  delta = None
>
>              if use_tags:
> - -                lf.show(revno, rev, delta, rev_tag_dict.get(rev_id, []))
> +                lf.show(revno, rev, delta, rev_tag_dict.get(rev_id))

Ahh.. true, for a while that didn't work, but I changed that code.
Guess I should rather change it to rev_tag_dict[rev_id] ?

>              else:
>                  lf.show(revno, rev, delta)
>          else:
> @@ -281,7 +281,8 @@
>                  lf.show_merge(rev, merge_depth)
>              else:
>                  if use_tags:
> - -                    lf.show_merge_revno(rev, merge_depth, revno, rev_tag_dict.get(rev_id, []))
> +                    lf.show_merge_revno(rev, merge_depth, revno,
> +                                        rev_tag_dict.get(rev_id))
>                  else:
>                      lf.show_merge_revno(rev, merge_depth, revno)
>
> @@ -355,24 +356,26 @@
>
>
>  class LongLogFormatter(LogFormatter):
> - -    def show(self, revno, rev, delta, tags=[]):
> +    supports_tags = True    # must exist and be True
> +                            # if this log formatter support tags.
> +                            # .show() and .show_merge_revno() must then accept
> +                            # the 'tags'-argument with list of tags
> +
> +    def show(self, revno, rev, delta, tags=None):
>          return self._show_helper(revno=revno, rev=rev, delta=delta, tags=tags)
> - -
> - -    def supports_tags(self):
> - -        """must exist if this log formatter support tags
> - -           show and show_merge_revno must then accept the 'tags'-argument """
> - -        pass
>
>      @deprecated_method(zero_eleven)
>      def show_merge(self, rev, merge_depth):
> - -        return self._show_helper(rev=rev, indent='    '*merge_depth, merged=True, delta=None)
> +        return self._show_helper(rev=rev, indent='    '*merge_depth,
> +                                 merged=True, delta=None)
>
> - -    def show_merge_revno(self, rev, merge_depth, revno, tags=[]):
> +    def show_merge_revno(self, rev, merge_depth, revno, tags=None):
>          """Show a merged revision rev, with merge_depth and a revno."""
>          return self._show_helper(rev=rev, revno=revno,
>              indent='    '*merge_depth, merged=True, delta=None, tags=tags)
>
> - -    def _show_helper(self, rev=None, revno=None, indent='', merged=False, delta=None, tags=[]):
> +    def _show_helper(self, rev=None, revno=None, indent='', merged=False,
> +                     delta=None, tags=None):
>          """Show a revision, either merged or not."""
>          from bzrlib.osutils import format_date
>          to_file = self.to_file
>
>
> I'm wrap some long lines (>80 chars) as well.

Ok, I was thinking of that but some code was already >80chars, so I
ignored it.  But you are of course right.

> I'm also think that this code is somewhat superfluous:
>
> +            if use_tags:
> +                lf.show(revno, rev, delta, rev_tag_dict.get(rev_id, []))
> +            else:
> +                lf.show(revno, rev, delta)
>
> I'd like to have it unconditional, but I understand pros and cons.
> So I can live with that approach.

The only reason to have that was to not break the API for old log plugins.

Alexander,
I'm very busy right now, and am going to Vegas in a few days, so I
will not have time to care for this in a few weeks.  Feel free to
rework this (applying this patch and anything else you need to do) and
submitt it for review again if you want.  If not, it will wait a few
weeks for me.

/Erik