[PATCH 1 of 2 V2] formatter: reserve _ prefixed keys as internal and don't render

[Gregory Szorc]

On Wed, Mar 22, 2017 at 9:21 AM, Ryan McElroy <rm at fb.com> wrote:

> On 3/22/17 2:55 PM, Yuya Nishihara wrote:
>
>> On Tue, 21 Mar 2017 23:49:25 -0700, Gregory Szorc wrote:
>>
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc at gmail.com>
>>> # Date 1490164306 25200
>>> #      Tue Mar 21 23:31:46 2017 -0700
>>> # Node ID ae796e23fd42b036352b298f570af8949c2db2d9
>>> # Parent  102f291807c92864a2231e5e925d6cd64783bb59
>>> formatter: reserve _ prefixed keys as internal and don't render
>>>
>>> As part of implementing `hg show`, Yuya noticed that JSON formatting
>>> was rendering an internal-only key, which was unfortunate.
>>>
>>> In this patch, I document new behavior on fm.data() that all
>>> keys beginning with underscores are reserved. I change the behavior
>>> of the JSON formatter to not render keys beginning with underscores.
>>>
>>> I ran the test suite with fm.data() raising if any key with a leading
>>> underscore was passed in and there were no test failures. So I think
>>> it is safe to adopt this convention.
>>>
>>> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
>>> --- a/mercurial/formatter.py
>>> +++ b/mercurial/formatter.py
>>> @@ -176,7 +176,11 @@ class baseformatter(object):
>>>           '''insert context objects to be used to render template
>>> keywords'''
>>>           pass
>>>       def data(self, **data):
>>> -        '''insert data into item that's not shown in default output'''
>>> +        '''insert data into item that's not shown in default output
>>> +
>>> +        Keys beginning with '_' are designated as internal and may not
>>> be
>>> +        rendered by all formatters.
>>> +        '''
>>>           self._item.update(data)
>>>       def write(self, fields, deftext, *fielddata, **opts):
>>>           '''do default text output while assigning data to item'''
>>> @@ -315,6 +319,9 @@ class jsonformatter(baseformatter):
>>>           self._out.write("\n {\n")
>>>           first = True
>>>           for k, v in sorted(self._item.items()):
>>> +            # Don't format hidden elements.
>>> +            if k.startswith('_'):
>>> +                continue
>>>
>> The idea of using _ prefix seems okay, but I don't think internal keys
>> should
>> be usable in template. If "longestlen" should be available for user
>> templates,
>> it shouldn't be _-prefixed.
>>
>>
> This seems like a good time to bring up by long-held desire for a JSON
> template that allows specifying what keys you're interested in. This would
> solve this problem more suitably than internal name-munging.
>
> To expand, -T json is insufficient for some things, like requesting file
> adds from hg log, which might be slow to ask for in large repos. I'd like a
> syntax like:
>
> -T json(desc,fileadds)
>
> Of course, there could be a default set of fields the a bare -T json would
> expose, but you could override it with the passed set.
>
> Then, we simply take the _longest out of the default set, but it's still
> available if explicitly asked for -- which is good! Because then other
> tools can use these values as needed.
>
> So, overall I'm -0 on this approach, but I understand that perfection is
> the enemy of better, so I'd be okay getting this in until "someone" has
> time to add the custom keys functionality to json.
>

Per feedback, I'll drop this patch.

We can clean things up in a follow-up if people have a good idea for
handling this.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170324/7ced519d/attachment.html>