[PATCH 3 of 4 V3] tags: add mergemode flag to _readtags

[Angel Ezquerra]

On Fri, Jun 27, 2014 at 1:38 AM, Matt Mackall <mpm at selenic.com> wrote:
> On Thu, 2014-06-26 at 01:22 +0200, Angel Ezquerra wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1403677632 -7200
>> #      Wed Jun 25 08:27:12 2014 +0200
>> # Node ID 488f01d33770c338ebe987b4efbd986b0ef91ed0
>> # Parent  10ffd0db8fea2e7324233136b262822e775c45a5
>> tags: add mergemode flag to _readtags
> ...
>> +    The output changes depending on the mergemode flag:
>> +    - When mergemode is False (the default), return a mapping from tag name to
>> +      (node, hist): node is the node id from the last line read for that name,
>> +      and hist is the list of node ids previously associated with it (in file
>> +      order).  All node ids are binary, not hex.
>> +    - When mergemode is True, return a mapping from tag name to a list of
>> +      [hexnode, line number] pairs, ordered from the oldest to the newest node.
>> +      hex node is hex not binary.
>> +      '''
>
> I generally frown on functions that change return type based on
> argument. Perhaps we can have:
>
> _readtags() -> _readtagshist()
>
> ..assuming the overhead of tracking the extra history isn't too high.

I don't like doing that much either so I'll be happy to try something
else. I don't want to duplicate _readtags() on the new tagmerge
module, so the tags._readtags() function needs to change a bit
though...

I think that if _readtagshist() had two separte outputs (e.g. one with
the original _readtags() format) and another one with just the line
numbers) the performance hit should be fine (managing the extra dict
plus the extra function call). We could improve things further by
passing an extra flag to _readtagshist() which would disable the
calculation of the second output dict (e.g. make it return None). Then
the only overhead would be the extra function call and an additional
comparison for each tag.

I'll give it a try and send a new patch series.

Thanks,

Angel