Patchwork [2,of,3] changeset: adjust json output to use new/old (BC)

login
register
mail settings
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

timeless@mozdev.org - April 13, 2016, 5:14 p.m.
# 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.
Yuya Nishihara - April 15, 2016, 4:17 p.m.
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": ...} ?
timeless - April 15, 2016, 4:56 p.m.
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
timeless - April 15, 2016, 5 p.m.
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...
Yuya Nishihara - April 16, 2016, 5:40 a.m.
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".
Matt Mackall - April 16, 2016, 9:19 p.m.
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"}}]
    }
   ]