[PATCH] templates: add {lasttag} and {lasttagdistance} keywords
Mads Kiilerich
mads at kiilerich.com
Tue Jul 14 20:02:33 CDT 2009
Some further comments after having looked more at your nice patch.
Gilles Moris wrote, On 07/14/2009 06:45 PM:
> mercurial/cmdutil.py | 40 ++++++++++++++++++++++++++++++++++++++++
> mercurial/help.py | 3 +++
> tests/test-log | 44 ++++++++++++++++++++++++++++++++++++++++++++
> tests/test-log.out | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 137 insertions(+), 0 deletions(-)
>
>
> # HG changeset patch
> # User Gilles Moris<gilles.moris at free.fr>
> # Date 1247087563 -7200
> # Node ID bea99b6921ea2a76921b765cfa5fe4a9f5ffe6e1
> # Parent e1dacc2c0aaee8bfa4a1749ed504b3afed119adb
> templates: add {lasttag} and {lasttagdistance} keywords.
>
> Useful to build nightly version names and see on which tag a rev is based on.
>
> Among tag ancestors, the one with the latest date, or with the longuest path if
>
longest?
> there are multiple pathes to the same tag, is chosen.
> This way, lasttag does not depend on the push/pull order and matches the
> user's expectation, in particular in the case of a backout merge.
> Only global tags are considered. The local tags (including 'tip' and automatic
> MQ tags) are discarded.
>
> Both lasttag and lasttagdistance are defined as String, so that they default to
> an empty string if no tag is found.
>
They are strings, but I wouldn't say that they are "defined". But being
string doesn't cause it to be empty - it just allows it. So not "so
that" but just an "an". But as discussed below I would rather never get
an empty string ...
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -741,6 +741,35 @@
> 'manifest': '{rev}:{node|formatnode}',
> 'filecopy': '{name} ({source})'})
>
> + @util.propertycache
> + def lasttagmap(self):
> + '''get an array[rev] of latest tag tuple, each tuple being
> + (date of the latest tag, the [first] tag, distance to that tag).
> + Exclude "tip" and MQ tags from the search.
>
Wouldn't it be better to say what it is instead of what it isn't? It is
the latest global tag, and here we don't even know mq enough to mention it.
> + '''
> + defvalue = 0, '', -1
>
For revisions without any tagged ancestors I would rather have the
distance to the first revision than nothing. Using the "null" revision
would be an obvious choice:
defvalue = 0, 'null', 0
> + ltmap = [defvalue] * len(self.repo)
> +
> + for r in range(len(self.repo)):
> + ctx = self.repo[r]
> +
> + tags = [t for t in ctx.tags() if self.repo.tagtype(t) == 'global']
> + if tags:
> + ltmap[r] = ctx.date(), tags[0], 0
>
Is the ordering of multiple tags for a revision well-defined? Is it a
good choice to use the first?
Perhaps
'-'.join(sorted(tags))
> + else:
> + # trick: use tuple comparison to sort by latest date first
> + # and if the dates (and by construction also the tag) are
> + # equal, by longuest path.
>
I think it is fine to be clever. If it so clever that we have to explain
it then it is less fine. (Not that I am against comments!) But if it is
so dirty that we call it a trick then we shouldn't use it.
It took me some time to understand the comment. Especially the use of
"sort" and "by construction" and that it referred to the _tag_ date
tricked me.
Perhaps
# the last tag of this rev comes from the parent with
the latest
# tag (secondarily the alphabetically latest tag name
and the
# longest distance to the tag), achieved by comparing
the tuples.
> + lasttag = reduce(max,
> + [ltmap[p.rev()] for p in ctx.parents()],
> + defvalue)
>
This looks unnecessary clever and hard to read. Especially because there
always are 1 or 2 parents.
And be aware that p.rev() will be -1 for "null" and it will thus look at
the wrong place in ltmap. It works anyway, but if it is intentional then
it deserves a comment.
So the same can be achieved with
ldate, ltag, ldist = max(ltmap[p.rev()] for p in
ctx.parents())
Or slightly more verbose, but without wrong indexing and without relying
on initialization of ltmap:
ldate, ltag, ldist = max(
p.rev() >= 0 and ltmap[p.rev()] or (0, 'null', 0)
for p in ctx.parents())
> + ldate, ltag, ldist = lasttag
> + # if no previous tag, nothing to do (leave default value)
>
> + if ltag:
>
Why shouldn't we give useful distances for revisions which just happens
to not have other tags to count from than "null"? However, with "null"
as default value as suggested above this is redundant.
> + ltmap[r] = ldate, ltag, ldist + 1
> +
> + return ltmap
> +
> def use_template(self, t):
> '''set template string to use'''
> self.t.cache['changeset'] = t
> @@ -874,6 +903,15 @@
> removes += i[2]
> return '%s: +%s/-%s' % (files, adds, removes)
>
> + def showlasttag(**args):
> + return self.lasttagmap[ctx.rev()][1]
> + def showlasttagdistance(**args):
> + dist = self.lasttagmap[ctx.rev()][2]
> + if dist>= 0:
> + return str(dist)
> + else:
> + return ""
>
As discussed above I would rather always have the distance.
Or actually ... for some uses it would be nice to have a simple template
which gives "1.3" instead of "1.3-0" or "1.3-" - but it should still
give for example "1.3-117".
Perhaps a {lasttaganddistance} would be useful too?
> +
> defprops = {
> 'author': ctx.user(),
> 'branches': showbranches,
> @@ -891,6 +929,8 @@
> 'tags': showtags,
> 'extras': showextras,
> 'diffstat': showdiffstat,
> + 'lasttag': showlasttag,
> + 'lasttagdistance': showlasttagdistance,
> }
> props = props.copy()
> props.update(defprops)
> diff --git a/mercurial/help.py b/mercurial/help.py
> --- a/mercurial/help.py
> +++ b/mercurial/help.py
> @@ -369,6 +369,9 @@
> - file_adds: List of strings. Files added by this changeset.
> - file_mods: List of strings. Files modified by this changeset.
> - file_dels: List of strings. Files removed by this changeset.
> + - lasttag: String. Most recent global tag in the ancestors of this
> + changeset.
> + - lasttagdistance: String. Longuest path to the last tag.
> - node: String. The changeset identification hash, as a 40-character
> hexadecimal string.
> - parents: List of strings. The parents of the changeset.
> diff --git a/tests/test-log b/tests/test-log
> --- a/tests/test-log
> +++ b/tests/test-log
>
Do this test really belong in the test of "log"? Wouldn't it be more
natural in test-command-template ?
> @@ -118,4 +118,48 @@
> hg log -u "user1" -u "user2"
> hg log -u "user3"
>
> +cd ..
> +
> +echo % lasttag
> +hg init lasttag
> +cd lasttag
> +
> +echo a> file
> +hg ci -Am a -d '0 0'
> +
> +echo b>> file
> +hg ci -m b -d '1 0'
> +
> +echo c>> head1
> +hg ci -Am h1c -d '2 0'
> +
> +hg update -q 1
> +echo d>> head2
> +hg ci -Am h2d -d '3 0'
> +
> +echo e>> head2
> +hg ci -m h2e -d '4 0'
> +
> +hg merge -q
> +hg ci -m merge -d '5 0'
> +
> +echo % No tag set
> +hg log --template '{rev}: {lasttag} {lasttagdistance}\n'
> +
> +echo % one common tag: longuest path wins
> +hg tag -r 1 -m t1 -d '6 0' t1
> +hg log --template '{rev}: {lasttag} {lasttagdistance}\n'
> +
> +echo % one ancestor tag: more recent wins
> +hg tag -r 2 -m t2 -d '7 0' t2
> +hg log --template '{rev}: {lasttag} {lasttagdistance}\n'
> +
> +echo % two branch tags: more recent wins
> +hg tag -r 3 -m t3 -d '8 0' t3
> +hg log --template '{rev}: {lasttag} {lasttagdistance}\n'
> +
> +echo % merged tag overrides
> +hg tag -r 5 -m t5 -d '9 0' t5
> +hg log --template '{rev}: {lasttag} {lasttagdistance}\n'
> +
> exit 0
> diff --git a/tests/test-log.out b/tests/test-log.out
> --- a/tests/test-log.out
> +++ b/tests/test-log.out
> @@ -257,3 +257,53 @@
> date: Thu Jan 01 00:00:00 1970 +0000
> summary: a
>
> +% lasttag
> +adding file
> +adding head1
> +adding head2
> +created new head
> +% No tag set
> +5:
> +4:
> +3:
> +2:
> +1:
> +0:
> +% one common tag: longuest path wins
> +6: t1 4
> +5: t1 3
> +4: t1 2
> +3: t1 1
> +2: t1 1
> +1: t1 0
> +0:
> +% one ancestor tag: more recent wins
> +7: t2 3
> +6: t2 2
> +5: t2 1
> +4: t1 2
> +3: t1 1
> +2: t2 0
> +1: t1 0
> +0:
> +% two branch tags: more recent wins
> +8: t3 5
> +7: t3 4
> +6: t3 3
> +5: t3 2
> +4: t3 1
> +3: t3 0
> +2: t2 0
> +1: t1 0
> +0:
> +% merged tag overrides
> +9: t5 4
> +8: t5 3
> +7: t5 2
> +6: t5 1
> +5: t5 0
> +4: t3 1
> +3: t3 0
> +2: t2 0
> +1: t1 0
> +0:
>
More information about the Mercurial-devel
mailing list