| Submitter | timeless@mozdev.org |
|---|---|
| Date | April 13, 2016, 5:14 p.m. |
| Message ID | <4161598737bc141a062d.1460567675@waste.org> |
| Download | mbox | patch |
| Permalink | /patch/14588/ |
| State | Changes Requested |
| Delegated to: | Yuya Nishihara |
| Headers | show |
Comments
On Wed, 13 Apr 2016 12:14:35 -0500, timeless wrote: > # HG changeset patch > # User timeless <timeless@mozdev.org> > # Date 1460548060 0 > # Wed Apr 13 11:47:40 2016 +0000 > # Node ID 4161598737bc141a062d8fa0d2c90da823e5b6c9 > # Parent 57f09cb9c2e69b0e8ef89cd21758722ba7e07ff6 > changeset: adjust json output to use new/old (BC) > > The previous json structure was not friendly to strongly structured parsing. Maybe I'm dumb, but I really can't understand what the "strongly structured parsing" is. Could you elaborate a little more? > --- a/tests/test-log.t > +++ b/tests/test-log.t > @@ -524,7 +524,7 @@ > "tags": ["tip"], > "parents": ["2ca5ba7019804f1f597249caddf22a64d34df0ba"], > "files": ["dir/b", "e"], > - "copies": {"e": "dir/b"} > + "copies": [{"new": {"path": "e"}, "old": {"path": "dir/b"}}] Why are only "copies" wrapped by {"path": ...} ?
On Fri, Apr 15, 2016 at 12:17 PM, Yuya Nishihara <yuya@tcha.org> wrote: > On Wed, 13 Apr 2016 12:14:35 -0500, timeless wrote: >> # HG changeset patch >> # User timeless <timeless@mozdev.org> >> # Date 1460548060 0 >> # Wed Apr 13 11:47:40 2016 +0000 >> # Node ID 4161598737bc141a062d8fa0d2c90da823e5b6c9 >> # Parent 57f09cb9c2e69b0e8ef89cd21758722ba7e07ff6 >> changeset: adjust json output to use new/old (BC) >> >> The previous json structure was not friendly to strongly structured parsing. > > Maybe I'm dumb, but I really can't understand what the "strongly structured > parsing" is. Could you elaborate a little more? > >> --- a/tests/test-log.t >> +++ b/tests/test-log.t >> @@ -524,7 +524,7 @@ >> "tags": ["tip"], >> "parents": ["2ca5ba7019804f1f597249caddf22a64d34df0ba"], >> "files": ["dir/b", "e"], >> - "copies": {"e": "dir/b"} >> + "copies": [{"new": {"path": "e"}, "old": {"path": "dir/b"}}] > > Why are only "copies" wrapped by {"path": ...} ? > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Fri, Apr 15, 2016 at 12:17 PM, Yuya Nishihara <yuya@tcha.org> wrote: > On Wed, 13 Apr 2016 12:14:35 -0500, timeless wrote: >> # HG changeset patch >> # User timeless <timeless@mozdev.org> >> # Date 1460548060 0 >> # Wed Apr 13 11:47:40 2016 +0000 >> # Node ID 4161598737bc141a062d8fa0d2c90da823e5b6c9 >> # Parent 57f09cb9c2e69b0e8ef89cd21758722ba7e07ff6 >> changeset: adjust json output to use new/old (BC) >> >> The previous json structure was not friendly to strongly structured parsing. > > Maybe I'm dumb, but I really can't understand what the "strongly structured > parsing" is. Could you elaborate a little more? Basically being able to do : {object}.{well-known-property-name} for a in copies: print(a.new.path, a.old.path) Upon review "new" is a bad choice since some languages would object. I'm actually not sure if this is a path or an abspath, I should re-review. > >> --- a/tests/test-log.t >> +++ b/tests/test-log.t >> @@ -524,7 +524,7 @@ >> "tags": ["tip"], >> "parents": ["2ca5ba7019804f1f597249caddf22a64d34df0ba"], >> "files": ["dir/b", "e"], >> - "copies": {"e": "dir/b"} >> + "copies": [{"new": {"path": "e"}, "old": {"path": "dir/b"}}] > > Why are only "copies" wrapped by {"path": ...} ? If we have renames, it should affect that too... Basically, the problem is that the old system used filepaths as keys, which makes key based parsing hard...
On Fri, 15 Apr 2016 13:00:11 -0400, timeless wrote: > On Fri, Apr 15, 2016 at 12:17 PM, Yuya Nishihara <yuya@tcha.org> wrote: > > On Wed, 13 Apr 2016 12:14:35 -0500, timeless wrote: > >> # HG changeset patch > >> # User timeless <timeless@mozdev.org> > >> # Date 1460548060 0 > >> # Wed Apr 13 11:47:40 2016 +0000 > >> # Node ID 4161598737bc141a062d8fa0d2c90da823e5b6c9 > >> # Parent 57f09cb9c2e69b0e8ef89cd21758722ba7e07ff6 > >> changeset: adjust json output to use new/old (BC) > >> > >> The previous json structure was not friendly to strongly structured parsing. > > > > Maybe I'm dumb, but I really can't understand what the "strongly structured > > parsing" is. Could you elaborate a little more? > > Basically being able to do : {object}.{well-known-property-name} > > for a in copies: > print(a.new.path, a.old.path) Hmm, it sounds like "I want this structure, so I made hg do that." It varies what structure is the best for users. The original structure is convenient to map new path to old path, for example. for p in files: copies.get(p) > Upon review "new" is a bad choice since some languages would object. > I'm actually not sure if this is a path or an abspath, I should re-review. IIRC, it's a canonical path (slash-separated relative path to repository root), which is an "abspath" in your sense. > >> --- a/tests/test-log.t > >> +++ b/tests/test-log.t > >> @@ -524,7 +524,7 @@ > >> "tags": ["tip"], > >> "parents": ["2ca5ba7019804f1f597249caddf22a64d34df0ba"], > >> "files": ["dir/b", "e"], > >> - "copies": {"e": "dir/b"} > >> + "copies": [{"new": {"path": "e"}, "old": {"path": "dir/b"}}] > > > > Why are only "copies" wrapped by {"path": ...} ? > > If we have renames, it should affect that too... I mean why "files" aren't [{"path": "dir/b"}, {"path": "e"}], and "tags" aren't [{"name": "tip"}], etc. I don't like these, but they would be more consistent with the new "copies".
On Sat, 2016-04-16 at 14:40 +0900, Yuya Nishihara wrote: > On Fri, 15 Apr 2016 13:00:11 -0400, timeless wrote: > > > > On Fri, Apr 15, 2016 at 12:17 PM, Yuya Nishihara <yuya@tcha.org> wrote: > > > > > > On Wed, 13 Apr 2016 12:14:35 -0500, timeless wrote: > > > > > > > > # HG changeset patch > > > > # User timeless <timeless@mozdev.org> > > > > # Date 1460548060 0 > > > > # Wed Apr 13 11:47:40 2016 +0000 > > > > # Node ID 4161598737bc141a062d8fa0d2c90da823e5b6c9 > > > > # Parent 57f09cb9c2e69b0e8ef89cd21758722ba7e07ff6 > > > > changeset: adjust json output to use new/old (BC) > > > > > > > > The previous json structure was not friendly to strongly structured > > > > parsing. > > > Maybe I'm dumb, but I really can't understand what the "strongly > > > structured > > > parsing" is. Could you elaborate a little more? > > Basically being able to do : {object}.{well-known-property-name} > > > > for a in copies: > > print(a.new.path, a.old.path) > Hmm, it sounds like "I want this structure, so I made hg do that." It varies > what structure is the best for users. The original structure is convenient > to map new path to old path, for example. > > for p in files: > copies.get(p) Right. It matches what hg does internally, which has proved convenient. -- Mathematics is the supreme nostalgia of our time.
Patch
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -1449,8 +1449,9 @@ ", ".join('"%s"' % j(f) for f in ctx.files())) if copies: - self.ui.write(',\n "copies": {%s}' % - ", ".join('"%s": "%s"' % (j(k), j(v)) + self.ui.write(',\n "copies": [%s]' % + ", ".join('{"new": {"path": "%s"}, ' + '"old": {"path": "%s"}}' % (j(k), j(v)) for k, v in copies)) matchfn = self.matchfn diff --git a/tests/test-log.t b/tests/test-log.t --- a/tests/test-log.t +++ b/tests/test-log.t @@ -524,7 +524,7 @@ "tags": ["tip"], "parents": ["2ca5ba7019804f1f597249caddf22a64d34df0ba"], "files": ["dir/b", "e"], - "copies": {"e": "dir/b"} + "copies": [{"new": {"path": "e"}, "old": {"path": "dir/b"}}] } ]