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

[Ryan McElroy]

On 3/28/17 7:17 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at 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!