[PATCH 4 of 7] tags: make argument 'tagtype' optional in '_updatetags'

[Pierre-Yves David]

On 03/28/2017 11:58 AM, Ryan McElroy wrote:
> On 3/28/17 7:16 AM, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1490679550 -7200
>> #      Tue Mar 28 07:39:10 2017 +0200
>> # Node ID e49ee337ec51b64e440585d44e6c7df736164e98
>> # Parent  f0c93dd8d018c9f6828c97be8ccb80dbfca694b8
>> # EXP-Topic tags
>> tags: make argument 'tagtype' optional in '_updatetags'
>>
>> This is the next step from the previous changesets, we are now ready
>> to use this
>> function in a simpler way.
>>
>> diff --git a/mercurial/tags.py b/mercurial/tags.py
>> --- a/mercurial/tags.py
>> +++ b/mercurial/tags.py
>> @@ -223,15 +223,20 @@ def _readtags(ui, repo, lines, fn, recod
>>           newtags[tag] = (taghist[-1], taghist[:-1])
>>       return newtags
>>   -def _updatetags(filetags, alltags, tagtype, tagtypes):
>> +def _updatetags(filetags, alltags, tagtype=None, tagtypes=None):
>>       '''Incorporate the tag info read from one file into the two
>>       dictionaries, alltags and tagtypes, that contain all tag
>> -    info (global across all heads plus local).'''
>> +    info (global across all heads plus local).
>> +
>> +    The second dictionnary is optional.'''
>
> The last two parameters are optional now, and it's an error to set
> tagtypes and not tagtype. I think this message can be updated to be more
> clear about how to use this in various circumstances.

I've updated it to:

     """Incorporate the tag info read from one file into dictionnaries

     The first one, 'alltags', is a "tagmaps" (see 'findglobaltags' for 
details).

     The second one, 'tagtypes', is optional and will be updated to 
track the "tagtype" of entries
     in the tagmaps. When set, the 'tagtype' argument also needs to be 
set."""


>
>> +    if tagtype is None:
>> +        assert tagtypes is None
>
> Is it more appropriate these days to raise an error.ProgrammingError()?

Not really. I see ProgrammingError as something useful widely used API 
that have a change to get missused. This assert is mostly to catch early 
a silly error from someone hacking this file.

>
>>         for name, nodehist in filetags.iteritems():
>>           if name not in alltags:
>>               alltags[name] = nodehist
>> -            tagtypes[name] = tagtype
>> +            if tagtype is not None:
>> +                tagtypes[name] = tagtype
>>               continue
>>             # we prefer alltags[name] if:
>> @@ -243,7 +248,7 @@ def _updatetags(filetags, alltags, tagty
>>           if (bnode != anode and anode in bhist and
>>               (bnode not in ahist or len(bhist) > len(ahist))):
>>               anode = bnode
>> -        else:
>> +        elif tagtype is not None:
>>               tagtypes[name] = tagtype
>>           ahist.extend([n for n in bhist if n not in ahist])
>>           alltags[name] = anode, ahist
>>
>

-- 
Pierre-Yves David