Patchwork formatter: allow json to handle more levels of depth

login
register
mail settings
Submitter Kostia Balytskyi
Date March 4, 2016, 2:14 p.m.
Message ID <0d64a8a89cb33fb67704.1457100864@dev1902.lla1.facebook.com>
Download mbox | patch
Permalink /patch/13595/
State Changes Requested
Delegated to: Pierre-Yves David
Headers show

Comments

Kostia Balytskyi - March 4, 2016, 2:14 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1457099048 28800
#      Fri Mar 04 05:44:08 2016 -0800
# Node ID 0d64a8a89cb33fb677047e3b9357389b52015294
# Parent  4f7a5e4f2daff0a65aa470d9f70365ad55aaa100
formatter: allow json to handle more levels of depth

Only the topmost level gets pretty-printed, further ones
are printed on the same line. I feel this is good since
humans reading json output will have better separation
between items.

Sample output looks somewhat like this:
```
$ ./hg tf
[
 {
  "l1": {"l2": [1, 2]}
 }
]
```

achieved through the following formatter use
```
@command('tf', [] + formatteropts)
def tf(ui, repo, **opts):
    obj = {'l1': {'l2': [1,2]}}
    fm = ui.formatter('tf', {'template': 'json'})
    fm.startitem()
    fm.data(l1=obj['l1'])
    fm.end()
```
Yuya Nishihara - March 4, 2016, 4:17 p.m.
On Fri, 4 Mar 2016 06:14:24 -0800, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia@fb.com>
> # Date 1457099048 28800
> #      Fri Mar 04 05:44:08 2016 -0800
> # Node ID 0d64a8a89cb33fb677047e3b9357389b52015294
> # Parent  4f7a5e4f2daff0a65aa470d9f70365ad55aaa100
> formatter: allow json to handle more levels of depth

Can you share some examples of nested data structure? I guess you are working
on "evolve --list", but I don't understand the detail.

I'm curious how to handle these nested data in templateformatter. Because the
templater is complicated, I don't expect it will be simple.

> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
> --- a/mercurial/formatter.py
> +++ b/mercurial/formatter.py
> @@ -109,8 +109,22 @@ class pickleformatter(baseformatter):
>          baseformatter.end(self)
>          self._ui.write(cPickle.dumps(self._data))
>  
> +def _getjsonbody(obj, sep=',', indent=''):
> +    r = []
> +    first = True
> +    for key, val in sorted(obj.items()):
> +        if first:
> +            first = False
> +        else:
> +            r.append(sep)
> +        r.append(indent)
> +        r.append('"%s": %s' % (key, _jsonifyobj(val)))
> +    return ''.join(r)
> +
>  def _jsonifyobj(v):
> -    if isinstance(v, tuple):
> +    if isinstance(v, dict):
> +        return '{' + _getjsonbody(v) + '}'
> +    elif isinstance(v, tuple) or isinstance(v, list):
>          return '[' + ', '.join(_jsonifyobj(e) for e in v) + ']'
>      elif v is None:
>          return 'null'
> @@ -133,16 +147,8 @@ class jsonformatter(baseformatter):
>              self._ui._first = False
>          else:
>              self._ui.write(",")
> -
> -        self._ui.write("\n {\n")
> -        first = True
> -        for k, v in sorted(self._item.items()):
> -            if first:
> -                first = False
> -            else:
> -                self._ui.write(",\n")
> -            self._ui.write('  "%s": %s' % (k, _jsonifyobj(v)))
> -        self._ui.write("\n }")
> +        body = _getjsonbody(self._item, sep=",\n", indent='  ')
> +        self._ui.write("\n {\n" + body + "\n }")

Looks like it is somewhat similar to templatefilters.json().
Kostia Balytskyi - March 4, 2016, 4:29 p.m.
This is a sample output of `evolve --list` with below fix applied to mercurial:

```
ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py
[
...
  {
    "desc": "e (e+f split)",
    "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],
    "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
    "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
  },

...
]

```






On 3/4/16, 4:17 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:

>On Fri, 4 Mar 2016 06:14:24 -0800, Kostia Balytskyi wrote:

>> # HG changeset patch

>> # User Kostia Balytskyi <ikostia@fb.com>

>> # Date 1457099048 28800

>> #      Fri Mar 04 05:44:08 2016 -0800

>> # Node ID 0d64a8a89cb33fb677047e3b9357389b52015294

>> # Parent  4f7a5e4f2daff0a65aa470d9f70365ad55aaa100

>> formatter: allow json to handle more levels of depth

>

>Can you share some examples of nested data structure? I guess you are working

>on "evolve --list", but I don't understand the detail.

>

>I'm curious how to handle these nested data in templateformatter. Because the

>templater is complicated, I don't expect it will be simple.

>

>> diff --git a/mercurial/formatter.py b/mercurial/formatter.py

>> --- a/mercurial/formatter.py

>> +++ b/mercurial/formatter.py

>> @@ -109,8 +109,22 @@ class pickleformatter(baseformatter):

>>          baseformatter.end(self)

>>          self._ui.write(cPickle.dumps(self._data))

>>  

>> +def _getjsonbody(obj, sep=',', indent=''):

>> +    r = []

>> +    first = True

>> +    for key, val in sorted(obj.items()):

>> +        if first:

>> +            first = False

>> +        else:

>> +            r.append(sep)

>> +        r.append(indent)

>> +        r.append('"%s": %s' % (key, _jsonifyobj(val)))

>> +    return ''.join(r)

>> +

>>  def _jsonifyobj(v):

>> -    if isinstance(v, tuple):

>> +    if isinstance(v, dict):

>> +        return '{' + _getjsonbody(v) + '}'

>> +    elif isinstance(v, tuple) or isinstance(v, list):

>>          return '[' + ', '.join(_jsonifyobj(e) for e in v) + ']'

>>      elif v is None:

>>          return 'null'

>> @@ -133,16 +147,8 @@ class jsonformatter(baseformatter):

>>              self._ui._first = False

>>          else:

>>              self._ui.write(",")

>> -

>> -        self._ui.write("\n {\n")

>> -        first = True

>> -        for k, v in sorted(self._item.items()):

>> -            if first:

>> -                first = False

>> -            else:

>> -                self._ui.write(",\n")

>> -            self._ui.write('  "%s": %s' % (k, _jsonifyobj(v)))

>> -        self._ui.write("\n }")

>> +        body = _getjsonbody(self._item, sep=",\n", indent='  ')

>> +        self._ui.write("\n {\n" + body + "\n }")

>

>Looks like it is somewhat similar to templatefilters.json().
Yuya Nishihara - March 6, 2016, 4:16 p.m.
On Fri, 4 Mar 2016 16:29:26 +0000, Kostia Balytskyi wrote:
> This is a sample output of `evolve --list` with below fix applied to mercurial:
> 
> ```
> ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py
> [
> ...
>   {
>     "desc": "e (e+f split)",
>     "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],
>     "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
>     "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
>   },

Looks like you've designed the data structure to be processed by the templater.
Isn't that verbose as a JSON output?

For example, "unstableparents" could be a list of node ids in JSON:

  "unstableparents": ["1995fc658ad6db877b932a45deb411980ccc064e"]

but templater requires a key:

  "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]

moreover it would need a ctx object for full templating:

  {unstableparents % "{rev}:{node|short} {desc|firstline}"}
   ~~~~~~~~~~~~~~~
     yields {'ctx': ctx}

I don't have any idea right now, but nested data structure wouldn't be handled
well by the current formatter API.
Kostia Balytskyi - March 6, 2016, 11:06 p.m.
On 3/6/16, 4:16 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:


>On Fri, 4 Mar 2016 16:29:26 +0000, Kostia Balytskyi wrote:

>> This is a sample output of `evolve --list` with below fix applied to mercurial:

>> 

>> ```

>> ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py

>> [

>> ...

>>   {

>>     "desc": "e (e+f split)",

>>     "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],

>>     "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",

>>     "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]

>>   },

>

>Looks like you've designed the data structure to be processed by the templater.

>Isn't that verbose as a JSON output?

>

>For example, "unstableparents" could be a list of node ids in JSON:

>

>  "unstableparents": ["1995fc658ad6db877b932a45deb411980ccc064e"]

>

>but templater requires a key:

>

>  "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]

>

>moreover it would need a ctx object for full templating:

>

>  {unstableparents % "{rev}:{node|short} {desc|firstline}"}

>   ~~~~~~~~~~~~~~~

>     yields {'ctx': ctx}

>

>I don't have any idea right now, but nested data structure wouldn't be handled

>well by the current formatter API.


I guess it depends on whether you want to be able to have all the 'rev', 'node', 'desc' and stuff to be there. If you're fine with having a limited output where you only have access not to all the ``ctx`` fields, but just some predefined set of them, my approach should be good enough. This is the current formatter template used for default ``evolve --list`` output (not terribly readable, but describes the point):

```
troublelisttemplate = "{short(node)}: {desc}\n"\
    "{if(unstableparents, '  unstable because of unstable parent(s) "\
    "{unstableparents % \'{short(unstableparent)} \'}\n')}"\
    "{if(obsoleteparents, '  unstable because of obsolete parent(s) "\
    "{obsoleteparents % \'{short(obsoleteparent)} \'}\n')}"\
    "{if(immutableprecursors, '  bumped because of immmutable precursor(s) "\
    "{immutableprecursors % \'{short(immutableprecursor)} \'}\n')}"\
    "{if(divergentsets, '  divergent with:\n{divergentsets % \'    "\
    "({divergentwith % \\'{short(node)} \\' })" \
    "; common evolution ancestor: {short(gcea)}\n\'}')}"


```


And here's a sample output produced with this template, fwiw:
```
ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ hg evolve --list
  01a3e66ba030: e (amended)
    unstable because of unstable parent(s) 1995fc658ad6
    divergent with:
       (84e1c6ae319d d3b90e9c84ab ); common evolution ancestor: 3efa43a7427b
       (add9a356b8cf ); common evolution ancestor: 3efa43a7427b
  add9a356b8cf: e (rebased)
    divergent with:
      (84e1c6ae319d d3b90e9c84ab ); common evolution ancestor: 3efa43a7427b
      (01a3e66ba030 ); common evolution ancestor: 3efa43a7427b
  84e1c6ae319d: e (e+f split)
    unstable because of unstable parent(s) 1995fc658ad6
    divergent with:
    (01a3e66ba030 ); common evolution ancestor: 3efa43a7427b
    (add9a356b8cf ); common evolution ancestor: 3efa43a7427b
...


```
Yuya Nishihara - March 7, 2016, 3:49 p.m.
On Sun, 6 Mar 2016 23:06:49 +0000, Kostia Balytskyi wrote:
> On 3/6/16, 4:16 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:
> >On Fri, 4 Mar 2016 16:29:26 +0000, Kostia Balytskyi wrote:
> >> This is a sample output of `evolve --list` with below fix applied to mercurial:
> >> 
> >> ```
> >> ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py
> >> [
> >> ...
> >>   {
> >>     "desc": "e (e+f split)",
> >>     "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],
> >>     "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
> >>     "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
> >>   },
> >
> >Looks like you've designed the data structure to be processed by the templater.
> >Isn't that verbose as a JSON output?
> >
> >For example, "unstableparents" could be a list of node ids in JSON:
> >
> >  "unstableparents": ["1995fc658ad6db877b932a45deb411980ccc064e"]
> >
> >but templater requires a key:
> >
> >  "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
> >
> >moreover it would need a ctx object for full templating:
> >
> >  {unstableparents % "{rev}:{node|short} {desc|firstline}"}
> >   ~~~~~~~~~~~~~~~
> >     yields {'ctx': ctx}
> >
> >I don't have any idea right now, but nested data structure wouldn't be handled
> >well by the current formatter API.
> 
> I guess it depends on whether you want to be able to have all the 'rev', 'node', 'desc' and stuff to be there. If you're fine with having a limited output where you only have access not to all the ``ctx`` fields, but just some predefined set of them, my approach should be good enough.

Hmm, but we provide -T option because people are likely to have their own needs.
They'll expect {rev}, {branch}, etc. can be used if "evolve -l" looks like a
log command.

That said, I think it's okay to start from a limited version. My main concern
is that a nested data structure looks ugly in JSON if it has to be compatible
with the templater. And JSON output should be stable. (timeless is working on
it.)
Kostia Balytskyi - March 7, 2016, 3:55 p.m.
On 3/7/16, 3:49 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:


>On Sun, 6 Mar 2016 23:06:49 +0000, Kostia Balytskyi wrote:

>> On 3/6/16, 4:16 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:

>> >On Fri, 4 Mar 2016 16:29:26 +0000, Kostia Balytskyi wrote:

>> >> This is a sample output of `evolve --list` with below fix applied to mercurial:

>> >> 

>> >> ```

>> >> ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py

>> >> [

>> >> ...

>> >>   {

>> >>     "desc": "e (e+f split)",

>> >>     "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],

>> >>     "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",

>> >>     "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]

>> >>   },

>> >

>> >Looks like you've designed the data structure to be processed by the templater.

>> >Isn't that verbose as a JSON output?

>> >

>> >For example, "unstableparents" could be a list of node ids in JSON:

>> >

>> >  "unstableparents": ["1995fc658ad6db877b932a45deb411980ccc064e"]

>> >

>> >but templater requires a key:

>> >

>> >  "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]

>> >

>> >moreover it would need a ctx object for full templating:

>> >

>> >  {unstableparents % "{rev}:{node|short} {desc|firstline}"}

>> >   ~~~~~~~~~~~~~~~

>> >     yields {'ctx': ctx}

>> >

>> >I don't have any idea right now, but nested data structure wouldn't be handled

>> >well by the current formatter API.

>> 

>> I guess it depends on whether you want to be able to have all the 'rev', 'node', 'desc' and stuff to be there. If you're fine with having a limited output where you only have access not to all the ``ctx`` fields, but just some predefined set of them, my approach should be good enough.

>

>Hmm, but we provide -T option because people are likely to have their own needs.

>They'll expect {rev}, {branch}, etc. can be used if "evolve -l" looks like a

>log command.

>

>That said, I think it's okay to start from a limited version. My main concern

>is that a nested data structure looks ugly in JSON if it has to be compatible

>with the templater. And JSON output should be stable. (timeless is working on

>it.)


Well, if people expect ``rev``, ``node``, ``desc``, I can just provide those to a reasonable amount. And "nothing in evolution is stable" seems to be an argument for getting working versions out there without much fear for backwards compatibility-related problems.

Another thing: we're currently discussing ``evolve --list`` rather than multi-level JSON output. What harm does the latter do?

Also, I don't get what you mean by "nested data structure looks ugly in JSON if it has to be compatible with templater" and "JSON output should be stable". Could you please explain a bit more?
Yuya Nishihara - March 8, 2016, 3:17 p.m.
On Mon, 7 Mar 2016 15:55:08 +0000, Kostia Balytskyi wrote:
> On 3/7/16, 3:49 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:
> >Hmm, but we provide -T option because people are likely to have their own needs.
> >They'll expect {rev}, {branch}, etc. can be used if "evolve -l" looks like a
> >log command.
> >
> >That said, I think it's okay to start from a limited version. My main concern
> >is that a nested data structure looks ugly in JSON if it has to be compatible
> >with the templater. And JSON output should be stable. (timeless is working on
> >it.)
> 
> Well, if people expect ``rev``, ``node``, ``desc``, I can just provide those to a reasonable amount. And "nothing in evolution is stable" seems to be an argument for getting working versions out there without much fear for backwards compatibility-related problems.
> 
> Another thing: we're currently discussing ``evolve --list`` rather than
> multi-level JSON output. What harm does the latter do?

Oh, do we? I'm totally fine with the multi-level template output of
"evolve --list".

If you don't care about the JSON output, I think using the changeset_templater
will be much easier. We can put new keywords to templatekw.keywrods. And we'll
need a way to select template other than ui.logtemplate.

> Also, I don't get what you mean by "nested data structure looks ugly in JSON
> if it has to be compatible with templater"

The templater requires a list of dicts, which isn't always necessary in JSON.
For example, "log -Tjson" renders "tags" as follows:

  "tags": ["tip"],

We can't do that with the current formatter API. To make it compatible with
the template functions, we would have to render it as follows:

  "tags": [{"tag": "tip"}]

This is IMO ugly.

> "JSON output should be stable".

It would be a big BC to change the structure of JSON output. I know it's okay
for "evolve --list" since it is experimental, but this patch isn't specific to
evolve.
Kostia Balytskyi - March 8, 2016, 3:26 p.m.
On 3/8/16, 3:17 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:


>On Mon, 7 Mar 2016 15:55:08 +0000, Kostia Balytskyi wrote:

>> On 3/7/16, 3:49 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:

>> >Hmm, but we provide -T option because people are likely to have their own needs.

>> >They'll expect {rev}, {branch}, etc. can be used if "evolve -l" looks like a

>> >log command.

>> >

>> >That said, I think it's okay to start from a limited version. My main concern

>> >is that a nested data structure looks ugly in JSON if it has to be compatible

>> >with the templater. And JSON output should be stable. (timeless is working on

>> >it.)

>> 

>> Well, if people expect ``rev``, ``node``, ``desc``, I can just provide those to a reasonable amount. And "nothing in evolution is stable" seems to be an argument for getting working versions out there without much fear for backwards compatibility-related problems.

>> 

>> Another thing: we're currently discussing ``evolve --list`` rather than

>> multi-level JSON output. What harm does the latter do?

>

>Oh, do we? I'm totally fine with the multi-level template output of

>"evolve --list".

>

>If you don't care about the JSON output, I think using the changeset_templater

>will be much easier. We can put new keywords to templatekw.keywrods. And we'll

>need a way to select template other than ui.logtemplate.

>

>> Also, I don't get what you mean by "nested data structure looks ugly in JSON

>> if it has to be compatible with templater"

>

>The templater requires a list of dicts, which isn't always necessary in JSON.

>For example, "log -Tjson" renders "tags" as follows:

>

>  "tags": ["tip"],

>

>We can't do that with the current formatter API. To make it compatible with

>the template functions, we would have to render it as follows:

>

>  "tags": [{"tag": "tip"}]

>

>This is IMO ugly.

I agree, this is ugly. Technically, the change that I'm proposing can output ``"tags": ["tip"]`` as well if you do ``fm.data(tags=["tip"])``. Bigger problem would be to actually use in a templated output produced by formatter.

>

>> "JSON output should be stable".

>

>It would be a big BC to change the structure of JSON output. I know it's okay

>for "evolve --list" since it is experimental, but this patch isn't specific to

>evolve.


I don't get how this change is breaking. All the existing behavior stays the same, one-level json gets printed by formatter in the same way (at least this was my goal).
Yuya Nishihara - March 8, 2016, 3:56 p.m.
On Tue, 8 Mar 2016 15:26:38 +0000, Kostia Balytskyi wrote:
> On 3/8/16, 3:17 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote:
> >The templater requires a list of dicts, which isn't always necessary in JSON.
> >For example, "log -Tjson" renders "tags" as follows:
> >
> >  "tags": ["tip"],
> >
> >We can't do that with the current formatter API. To make it compatible with
> >the template functions, we would have to render it as follows:
> >
> >  "tags": [{"tag": "tip"}]
> >
> >This is IMO ugly.
> I agree, this is ugly. Technically, the change that I'm proposing can output
> ``"tags": ["tip"]`` as well if you do ``fm.data(tags=["tip"])``. Bigger
> problem would be to actually use in a templated output produced by formatter.

Yep. Can you give me some time to consider this? I don't have enough time
to hack on hg on weekdays.

> >> "JSON output should be stable".
> >
> >It would be a big BC to change the structure of JSON output. I know it's okay
> >for "evolve --list" since it is experimental, but this patch isn't specific to
> >evolve.
> 
> I don't get how this change is breaking. All the existing behavior stays
> the same, one-level json gets printed by formatter in the same way
> (at least this was my goal).

I meant multi-level data could be used by another command, and it could render
unfortunate JSON and force us to tackle on the compatibility hell.
Pierre-Yves David - March 8, 2016, 8:53 p.m.
On 03/04/2016 04:29 PM, Kostia Balytskyi wrote:
> This is a sample output of `evolve --list` with below fix applied to mercurial:
>
> ```
> ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py
> [
> ...
>    {
>      "desc": "e (e+f split)",
>      "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],
>      "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
>      "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
>    },
>
> ...
> ]
>
> ```

I had a bit more unified structure in mind (vague proposal):

[
  {"desc": "e (e+f split)",
   "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
   [… author node level stuff…]
   "troubles": [
     {"troubletype": "unstable",
      "sourcetype": "unstableparent",
      "sourcenode": "1995fc658ad6db877b932a45deb411980ccc064e"},
     {"troubletype": "divergent",
      "divergentnode": "01a3e66ba0301a4cbb7d519f8b9980af271241bd",
      "gceanode": ""3efa43a7427b9b19dc715853251d4891d3e9334c"},
     {"troubletype": "divergent",
      "divergentnode": "add9a356b8cfca95c588f51f05291ad97ffbaea8",
      "gceanode": ""3efa43a7427b9b19dc715853251d4891d3e9334c"},
     ]
]

This make the structure a bit more "complexobject" containing few (one) 
list of complex object with somewhat standard strucuture. Instead of 
multiple small list with very specific data in each of them.

I'm not sure this is superior, but I'm dumping my brain to expand the 
discussion a bit.


Something interesting that emerged from the current discussion is "How 
is the templater expected to handle the nesting. I did not though much 
about it beforehand so I don't have a strong opinion yet.

I seems like the current kostia approach would help to standardize 
handling for list of elements. On the otherhand the more unified version 
above would give us a "troublectx" like "object" to feed the templater 
quite simply.

I'll let the night rest over it.
Matt Mackall - March 8, 2016, 9:43 p.m.
On Wed, 2016-03-09 at 00:56 +0900, Yuya Nishihara wrote:
> On Tue, 8 Mar 2016 15:26:38 +0000, Kostia Balytskyi wrote:
> > On 3/8/16, 3:17 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tch
> > a.org> wrote:
> > > The templater requires a list of dicts, which isn't always necessary in
> > > JSON.
> > > For example, "log -Tjson" renders "tags" as follows:
> > > 
> > >  "tags": ["tip"],
> > > 
> > > We can't do that with the current formatter API. To make it compatible
> > > with
> > > the template functions, we would have to render it as follows:
> > > 
> > >  "tags": [{"tag": "tip"}]
> > > 
> > > This is IMO ugly.
> > I agree, this is ugly. Technically, the change that I'm proposing can output
> > ``"tags": ["tip"]`` as well if you do ``fm.data(tags=["tip"])``. Bigger
> > problem would be to actually use in a templated output produced by
> > formatter.
> 
> Yep. Can you give me some time to consider this? I don't have enough time
> to hack on hg on weekdays.
> 
> > > > "JSON output should be stable".
> > > 
> > > It would be a big BC to change the structure of JSON output. I know it's
> > > okay
> > > for "evolve --list" since it is experimental, but this patch isn't
> > > specific to
> > > evolve.
> > 
> > I don't get how this change is breaking. All the existing behavior stays
> > the same, one-level json gets printed by formatter in the same way
> > (at least this was my goal).
> 
> I meant multi-level data could be used by another command, and it could render
> unfortunate JSON and force us to tackle on the compatibility hell.

A good example of such a command is "hg summary". It fits pretty poorly into the
the formatter's list of dicts of simple values model.

One idea I've considered is to have non-uniform list elements. So, something
like:

$ hg sum
parent: 34812:b9296b330a54 tip
 convert: darcs use absolute_import
branch: default
commit: 28 unknown (clean)
update: 61 new changesets, 13 branch heads (merge)
phases: 54 draft, 38 secret
unstable: 20 changesets
divergent: 2 changesets
unstable: 20 changesets
divergent: 2 changesets

..becomes something like:

[
 {
  "field": "parent",
  "rev": 34812,
  "node": ...,
  "tags": ["tip"],
  "desc": "convert:..."
 },
 {
  "field": "commit",
  "unknown": 28,
  "hint": "clean",
 }
]

This has the downside that if you want to show, for instance, just the commit
hint, you have to do something like:

$ hg sum -T "{ifeq(field, "commit", hint)}"

Note that there will also be an update hint in this example, so the hint field
is overloaded.

The alternative is to flatten all the lines into a single entry, so you could do
something like:

$ hg sum -T "unknown: {commitunknown} {commithint}\n"

-- 
Mathematics is the supreme nostalgia of our time.
Yuya Nishihara - March 13, 2016, 11:49 a.m.
On Tue, 8 Mar 2016 20:53:29 +0000, Pierre-Yves David wrote:
> 
> 
> On 03/04/2016 04:29 PM, Kostia Balytskyi wrote:
> > This is a sample output of `evolve --list` with below fix applied to mercurial:
> >
> > ```
> > ikostia@dev1902.lla1:~/temprepos/supertroubledrepo$ HGRCPATH= ~/clowncopter/hg evolve --list  --config extensions.lz4revlog= --config extensions.color= --config extensions.evolve=~/evolve/hgext/evolve.py
> > [
> > ...
> >    {
> >      "desc": "e (e+f split)",
> >      "divergentsets": [{"divergentwith": [{"node": "01a3e66ba0301a4cbb7d519f8b9980af271241bd"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}, {"divergentwith": [{"node": "add9a356b8cfca95c588f51f05291ad97ffbaea8"}],"gcea": "3efa43a7427b9b19dc715853251d4891d3e9334c"}],
> >      "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
> >      "unstableparents": [{"unstableparent": "1995fc658ad6db877b932a45deb411980ccc064e"}]
> >    },
> >
> > ...
> > ]
> >
> > ```
> 
> I had a bit more unified structure in mind (vague proposal):
> 
> [
>   {"desc": "e (e+f split)",
>    "node": "84e1c6ae319d139241a02ef5e939d7494b9b11d7",
>    [… author node level stuff…]
>    "troubles": [
>      {"troubletype": "unstable",
>       "sourcetype": "unstableparent",
>       "sourcenode": "1995fc658ad6db877b932a45deb411980ccc064e"},
>      {"troubletype": "divergent",
>       "divergentnode": "01a3e66ba0301a4cbb7d519f8b9980af271241bd",
>       "gceanode": ""3efa43a7427b9b19dc715853251d4891d3e9334c"},
>      {"troubletype": "divergent",
>       "divergentnode": "add9a356b8cfca95c588f51f05291ad97ffbaea8",
>       "gceanode": ""3efa43a7427b9b19dc715853251d4891d3e9334c"},
>      ]
> ]
> 
> This make the structure a bit more "complexobject" containing few (one) 
> list of complex object with somewhat standard strucuture. Instead of 
> multiple small list with very specific data in each of them.
> 
> I'm not sure this is superior, but I'm dumping my brain to expand the 
> discussion a bit.

Yeah, I think this will be easier to handle by templater API. On the other
hand, it will be slightly harder to write a template that sorts out "troubles"
by "troubletype".

  unstable: {troubles % "{ifeq(troubletype, 'unstable', sourcenode|short)}"}
  divergent: {troubles % "{ifeq(troubletype, 'divergent', sourcenode|short)}"}
  ...
Yuya Nishihara - March 13, 2016, 11:50 a.m.
On Tue, 08 Mar 2016 15:43:23 -0600, Matt Mackall wrote:
> On Wed, 2016-03-09 at 00:56 +0900, Yuya Nishihara wrote:
> > I meant multi-level data could be used by another command, and it could render
> > unfortunate JSON and force us to tackle on the compatibility hell.
> 
> A good example of such a command is "hg summary". It fits pretty poorly into the
> the formatter's list of dicts of simple values model.
> 
> One idea I've considered is to have non-uniform list elements. So, something
> like:
> 
> $ hg sum
> parent: 34812:b9296b330a54 tip
>  convert: darcs use absolute_import
> branch: default
> commit: 28 unknown (clean)
> update: 61 new changesets, 13 branch heads (merge)
> phases: 54 draft, 38 secret
> unstable: 20 changesets
> divergent: 2 changesets
> unstable: 20 changesets
> divergent: 2 changesets

I think a sub formatter or something can help this case. For example,

  fm = ui.formatter('summary', opts)
  f = fm.sub()
  f.write('rev node desc', '%d:%s\n %s', rev, node, desc)
  fm.startitem()
  fm.write('parent', 'parent: %s\n', f.end())
  ...
  fm.end()

For plain output, a sub formatter 'f' uses ui.push/popbuffer().

  f.write('rev node desc', '%d:%s\n %s', rev, node, desc)
  # write to ui._buffer
  f.end()
  # return '{rev}:{node}\n {desc}'

For json output, a sub formatter 'f' just keeps raw data.

  f.write('rev node desc', '%d:%s\n %s', rev, node, desc)
  # _data = {'rev': rev, 'node': node, 'desc': desc}
  f.end()
  # return _data

So, the output will be like:

  [
   {
    "parent": {"node": "deadbeef1234", "rev": 10, "desc": "foo"},
    ...
   }
  ]


> > > > For example, "log -Tjson" renders "tags" as follows:
> > > > 
> > > >  "tags": ["tip"],
> > > > 
> > > > We can't do that with the current formatter API. To make it compatible
> > > > with
> > > > the template functions, we would have to render it as follows:
> > > > 
> > > >  "tags": [{"tag": "tip"}]

I have no idea how to handle this cleanly. Maybe fm.writelist() or something
will be simple and can handle common cases.

  fm.writelist('tags', '%s', ['tip'], sep=' ', templatename='tag')
  # plain output: ' '.join('%s' % v for v in ['tip'])
  # json: "tags": ["tip"]
  # templater: 'tags': [{'tag': v} for v in ['tip']]

Patch

diff --git a/mercurial/formatter.py b/mercurial/formatter.py
--- a/mercurial/formatter.py
+++ b/mercurial/formatter.py
@@ -109,8 +109,22 @@  class pickleformatter(baseformatter):
         baseformatter.end(self)
         self._ui.write(cPickle.dumps(self._data))
 
+def _getjsonbody(obj, sep=',', indent=''):
+    r = []
+    first = True
+    for key, val in sorted(obj.items()):
+        if first:
+            first = False
+        else:
+            r.append(sep)
+        r.append(indent)
+        r.append('"%s": %s' % (key, _jsonifyobj(val)))
+    return ''.join(r)
+
 def _jsonifyobj(v):
-    if isinstance(v, tuple):
+    if isinstance(v, dict):
+        return '{' + _getjsonbody(v) + '}'
+    elif isinstance(v, tuple) or isinstance(v, list):
         return '[' + ', '.join(_jsonifyobj(e) for e in v) + ']'
     elif v is None:
         return 'null'
@@ -133,16 +147,8 @@  class jsonformatter(baseformatter):
             self._ui._first = False
         else:
             self._ui.write(",")
-
-        self._ui.write("\n {\n")
-        first = True
-        for k, v in sorted(self._item.items()):
-            if first:
-                first = False
-            else:
-                self._ui.write(",\n")
-            self._ui.write('  "%s": %s' % (k, _jsonifyobj(v)))
-        self._ui.write("\n }")
+        body = _getjsonbody(self._item, sep=",\n", indent='  ')
+        self._ui.write("\n {\n" + body + "\n }")
     def end(self):
         baseformatter.end(self)
         self._ui.write("\n]\n")