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
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().
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().
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.
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 ... ```
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.)
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?
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.
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).
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.
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.
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.
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)}"} ...
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")