Patchwork [6,of,7] tags: extract tags computation from fnodes in its own function

login
register
mail settings
Submitter Pierre-Yves David
Date March 28, 2017, 6:17 a.m.
Message ID <3d8a09214760799868b4.1490681821@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/19769/
State Changes Requested
Headers show

Comments

Pierre-Yves David - March 28, 2017, 6:17 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1490674092 -7200
#      Tue Mar 28 06:08:12 2017 +0200
# Node ID 3d8a09214760799868b472b97e964e88f7ec8fd5
# Parent  d5c70d5f7de740d3fa946318998f0f0c1204e4eb
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3d8a09214760
tags: extract tags computation from fnodes in its own function

I'm about to introduce code that needs to perform such computation on
"arbitrary" node. The logic is extract in its own function for reuse.
Ryan McElroy - March 28, 2017, 10:10 a.m.
On 3/28/17 7:17 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1490674092 -7200
> #      Tue Mar 28 06:08:12 2017 +0200
> # Node ID 3d8a09214760799868b472b97e964e88f7ec8fd5
> # Parent  d5c70d5f7de740d3fa946318998f0f0c1204e4eb
> # EXP-Topic tags
> tags: extract tags computation from fnodes in its own function

s/in/into (or just "to")

>
> I'm about to introduce code that needs to perform such computation on
> "arbitrary" node. The logic is extract in its own function for reuse.

s/node/nodes
s/extract/extracted
s/in/into (or just "to")
>
> diff --git a/mercurial/tags.py b/mercurial/tags.py
> --- a/mercurial/tags.py
> +++ b/mercurial/tags.py
> @@ -85,19 +85,18 @@ def findglobaltags(ui, repo):
>   
>       The tags cache is read and updated as a side-effect of calling.
>       '''
> -    alltags = {}
> -
>       (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
>       if cachetags is not None:
>           assert not shouldwrite
>           # XXX is this really 100% correct?  are there oddball special
>           # cases where a global tag should outrank a local tag but won't,
>           # because cachetags does not contain rank info?
> +        alltags = {}
>           _updatetags(cachetags, alltags)
>           return alltags
>   
>       seen = set()  # set of fnode
> -    fctx = None
> +    fnodes = []
>       for head in reversed(heads):  # oldest to newest
>           assert head in repo.changelog.nodemap, \
>                  "tag cache returned bogus head %s" % short(head)
> @@ -105,19 +104,32 @@ def findglobaltags(ui, repo):
>           fnode = tagfnode.get(head)
>           if fnode and fnode not in seen:
>               seen.add(fnode)
> -            if not fctx:
> -                fctx = repo.filectx('.hgtags', fileid=fnode)
> -            else:
> -                fctx = fctx.filectx(fnode)
> +            fnodes.append(fnode)
>   
> -            filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
> -            _updatetags(filetags, alltags)
> +    alltags = _tagsfromfnodes(ui, repo, fnodes)
>   
>       # and update the cache (if necessary)
>       if shouldwrite:
>           _writetagcache(ui, repo, valid, alltags)
>       return alltags
>   
> +def _tagsfromfnodes(ui, repo, fnodes):
> +    """return a tagsmap from a list of file-node
> +
> +    tagsmap: tag name to (node, hist) 2-tuples.
> +
> +    The order of the list matters."""
> +    alltags = {}
> +    fctx = None
> +    for fnode in fnodes:
> +        if fctx is None:
> +            fctx = repo.filectx('.hgtags', fileid=fnode)
> +        else:
> +            fctx = fctx.filectx(fnode)
> +        filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
> +        _updatetags(filetags, alltags)
> +    return alltags
> +
>   def readlocaltags(ui, repo, alltags, tagtypes):
>       '''Read local tags in repo. Update alltags and tagtypes.'''
>       try:
>

Code changes lgtm.

Patch

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -85,19 +85,18 @@  def findglobaltags(ui, repo):
 
     The tags cache is read and updated as a side-effect of calling.
     '''
-    alltags = {}
-
     (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
     if cachetags is not None:
         assert not shouldwrite
         # XXX is this really 100% correct?  are there oddball special
         # cases where a global tag should outrank a local tag but won't,
         # because cachetags does not contain rank info?
+        alltags = {}
         _updatetags(cachetags, alltags)
         return alltags
 
     seen = set()  # set of fnode
-    fctx = None
+    fnodes = []
     for head in reversed(heads):  # oldest to newest
         assert head in repo.changelog.nodemap, \
                "tag cache returned bogus head %s" % short(head)
@@ -105,19 +104,32 @@  def findglobaltags(ui, repo):
         fnode = tagfnode.get(head)
         if fnode and fnode not in seen:
             seen.add(fnode)
-            if not fctx:
-                fctx = repo.filectx('.hgtags', fileid=fnode)
-            else:
-                fctx = fctx.filectx(fnode)
+            fnodes.append(fnode)
 
-            filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
-            _updatetags(filetags, alltags)
+    alltags = _tagsfromfnodes(ui, repo, fnodes)
 
     # and update the cache (if necessary)
     if shouldwrite:
         _writetagcache(ui, repo, valid, alltags)
     return alltags
 
+def _tagsfromfnodes(ui, repo, fnodes):
+    """return a tagsmap from a list of file-node
+
+    tagsmap: tag name to (node, hist) 2-tuples.
+
+    The order of the list matters."""
+    alltags = {}
+    fctx = None
+    for fnode in fnodes:
+        if fctx is None:
+            fctx = repo.filectx('.hgtags', fileid=fnode)
+        else:
+            fctx = fctx.filectx(fnode)
+        filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
+        _updatetags(filetags, alltags)
+    return alltags
+
 def readlocaltags(ui, repo, alltags, tagtypes):
     '''Read local tags in repo. Update alltags and tagtypes.'''
     try: