Patchwork [5,of,7] tags: only return 'alltags' in 'findglobaltags'

login
register
mail settings
Submitter Pierre-Yves David
Date March 28, 2017, 6:17 a.m.
Message ID <d5c70d5f7de740d3fa94.1490681820@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/19768/
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 1490679683 -7200
#      Tue Mar 28 07:41:23 2017 +0200
# Node ID d5c70d5f7de740d3fa946318998f0f0c1204e4eb
# Parent  e49ee337ec51b64e440585d44e6c7df736164e98
# 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 d5c70d5f7de7
tags: only return 'alltags' in 'findglobaltags'

This is minor update along the way. We simplify the 'findglobaltags' function to
only return the tags. Since no existing data are reused, we know that all tags
are returned are global and we can let the caller get that information if it
cares about it.
Ryan McElroy - March 28, 2017, 10:05 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 1490679683 -7200
> #      Tue Mar 28 07:41:23 2017 +0200
> # Node ID d5c70d5f7de740d3fa946318998f0f0c1204e4eb
> # Parent  e49ee337ec51b64e440585d44e6c7df736164e98
> # EXP-Topic tags
> tags: only return 'alltags' in 'findglobaltags'
>
> This is minor update along the way. We simplify the 'findglobaltags' function to
> only return the tags. Since no existing data are reused, we know that all tags

Nit: English is hard. Even though data is technically the plural of 
"datum", in this context, "data" is being used in more of the 
"uncountable" sense (eg, I have "some data", not "I have five data", so 
it's more "appropriate sounding" to say "Since no existing data is 
reused..."

At least, that's my understanding of English. Anybody who knows better 
than me, please correct me.

> are returned are global and we can let the caller get that information if it

Spurious extra "are" at beginning of line.

> cares about it.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -708,9 +708,10 @@ class localrepository(object):
>           # quo fine?
>   
>   
> -        globaldata = tagsmod.findglobaltags(self.ui, self)
> -        alltags = globaldata[0]   # map tag name to (node, hist)
> -        tagtypes = globaldata[1]  # map tag name to tag type
> +        # map tag name to (node, hist)
> +        alltags = tagsmod.findglobaltags(self.ui, self)
> +        # map tag name to tag type
> +        tagtypes = dict((tag, 'global') for tag in alltags)

Ah, it makes sense why you split these out across two lines now. I 
retract my previous statement on patch number 2 now that I see this.

>   
>           tagsmod.readlocaltags(self.ui, self, alltags, tagtypes)
>   
> diff --git a/mercurial/tags.py b/mercurial/tags.py
> --- a/mercurial/tags.py
> +++ b/mercurial/tags.py
> @@ -79,17 +79,13 @@ from . import (
>   # setting it) for each tag is last.
>   
>   def findglobaltags(ui, repo):
> -    '''Find global tags in a repo: return (alltags, tagtypes)
> +    '''Find global tags in a repo: return a tagsmap
>   
> -    "alltags" maps tag name to (node, hist) 2-tuples.
> -
> -    "tagtypes" maps tag name to tag type. Global tags always have the
> -    "global" tag type.
> +    tagsmap: tag name to (node, hist) 2-tuples.
>   
>       The tags cache is read and updated as a side-effect of calling.
>       '''
>       alltags = {}
> -    tagtypes = {}
>   
>       (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
>       if cachetags is not None:
> @@ -97,8 +93,8 @@ def findglobaltags(ui, repo):
>           # 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?
> -        _updatetags(cachetags, alltags, 'global', tagtypes)
> -        return alltags, tagtypes
> +        _updatetags(cachetags, alltags)
> +        return alltags
>   
>       seen = set()  # set of fnode
>       fctx = None
> @@ -115,12 +111,12 @@ def findglobaltags(ui, repo):
>                   fctx = fctx.filectx(fnode)
>   
>               filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
> -            _updatetags(filetags, alltags, 'global', tagtypes)
> +            _updatetags(filetags, alltags)
>   
>       # and update the cache (if necessary)
>       if shouldwrite:
>           _writetagcache(ui, repo, valid, alltags)
> -    return alltags, tagtypes
> +    return alltags
>   
>   def readlocaltags(ui, repo, alltags, tagtypes):
>       '''Read local tags in repo. Update alltags and tagtypes.'''

This is much cleaner, nice!

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -708,9 +708,10 @@  class localrepository(object):
         # quo fine?
 
 
-        globaldata = tagsmod.findglobaltags(self.ui, self)
-        alltags = globaldata[0]   # map tag name to (node, hist)
-        tagtypes = globaldata[1]  # map tag name to tag type
+        # map tag name to (node, hist)
+        alltags = tagsmod.findglobaltags(self.ui, self)
+        # map tag name to tag type
+        tagtypes = dict((tag, 'global') for tag in alltags)
 
         tagsmod.readlocaltags(self.ui, self, alltags, tagtypes)
 
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -79,17 +79,13 @@  from . import (
 # setting it) for each tag is last.
 
 def findglobaltags(ui, repo):
-    '''Find global tags in a repo: return (alltags, tagtypes)
+    '''Find global tags in a repo: return a tagsmap
 
-    "alltags" maps tag name to (node, hist) 2-tuples.
-
-    "tagtypes" maps tag name to tag type. Global tags always have the
-    "global" tag type.
+    tagsmap: tag name to (node, hist) 2-tuples.
 
     The tags cache is read and updated as a side-effect of calling.
     '''
     alltags = {}
-    tagtypes = {}
 
     (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
     if cachetags is not None:
@@ -97,8 +93,8 @@  def findglobaltags(ui, repo):
         # 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?
-        _updatetags(cachetags, alltags, 'global', tagtypes)
-        return alltags, tagtypes
+        _updatetags(cachetags, alltags)
+        return alltags
 
     seen = set()  # set of fnode
     fctx = None
@@ -115,12 +111,12 @@  def findglobaltags(ui, repo):
                 fctx = fctx.filectx(fnode)
 
             filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
-            _updatetags(filetags, alltags, 'global', tagtypes)
+            _updatetags(filetags, alltags)
 
     # and update the cache (if necessary)
     if shouldwrite:
         _writetagcache(ui, repo, valid, alltags)
-    return alltags, tagtypes
+    return alltags
 
 def readlocaltags(ui, repo, alltags, tagtypes):
     '''Read local tags in repo. Update alltags and tagtypes.'''