Patchwork D6113: py3: convert to/from bytes/unicode for json.(dump|load)s in debugcallconduit

login
register
mail settings
Submitter phabricator
Date March 9, 2019, 3:01 a.m.
Message ID <differential-rev-PHID-DREV-gnlgwsqacsypjs3jyv4x-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39167/
State Superseded
Headers show

Comments

phabricator - March 9, 2019, 3:01 a.m.
Kwan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6113

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS




To: Kwan, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - March 10, 2019, 1:28 a.m.
> -    params = json.loads(ui.fin.read())
> -    result = callconduit(repo, name, params)
> -    s = json.dumps(result, sort_keys=True, indent=2, separators=(b',', b': '))
> -    ui.write(b'%s\n' % s)
> +    # json.(load|dump)s only returns/accepts unicode strings
> +    params = pycompat.rapply(lambda x:
> +        encoding.unitolocal(x) if isinstance(x, pycompat.unicode) else x,
> +        json.loads(ui.fin.read())
> +    )

Perhaps, the input data has to be converted to unicode. IIRC old Python 3
versions don't accept bytes.

> +    result = pycompat.rapply(lambda x:
> +        encoding.unifromlocal(x) if isinstance(x, bytes) else x,
> +        callconduit(repo, name, params)
> +    )
> +    s = json.dumps(result, sort_keys=True, indent=2, separators=(u',', u': '))
> +    ui.write(b'%s\n' % encoding.unitolocal(s))

Maybe we can use `templatefilters.json()` to dump internal (i.e. no unicode)
dict.
phabricator - March 10, 2019, 1:30 a.m.
yuja added a comment.


  > - params = json.loads(ui.fin.read())
  > - result = callconduit(repo, name, params)
  > - s = json.dumps(result, sort_keys=True, indent=2, separators=(b',', b': '))
  > - ui.write(b'%s\n' % s) +    # json.(load|dump)s only returns/accepts unicode strings +    params = pycompat.rapply(lambda x: +        encoding.unitolocal(x) if isinstance(x, pycompat.unicode) else x, +        json.loads(ui.fin.read()) +    )
  
  Perhaps, the input data has to be converted to unicode. IIRC old Python 3
  versions don't accept bytes.
  
  > +    result = pycompat.rapply(lambda x:
  >  +        encoding.unifromlocal(x) if isinstance(x, bytes) else x,
  >  +        callconduit(repo, name, params)
  >  +    )
  >  +    s = json.dumps(result, sort_keys=True, indent=2, separators=(u',', u': '))
  >  +    ui.write(b'%s\n' % encoding.unitolocal(s))
  
  Maybe we can use `templatefilters.json()` to dump internal (i.e. no unicode)
  dict.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6113

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - March 14, 2019, 3:18 p.m.
Kwan added a comment.


  In https://phab.mercurial-scm.org/D6113#88992, @yuja wrote:
  
  > >   -    params = json.loads(ui.fin.read())
  > >   -    result = callconduit(repo, name, params)
  > >   -    s = json.dumps(result, sort_keys=True, indent=2, separators=(b',', b': '))
  > >   -    ui.write(b'%s\n' % s)
  > >   +    # json.(load|dump)s only returns/accepts unicode strings
  > >   +    params = pycompat.rapply(lambda x:
  > >   +        encoding.unitolocal(x) if isinstance(x, pycompat.unicode) else x,
  > >   +        json.loads(ui.fin.read())
  > >   +    )
  >
  > Perhaps, the input data has to be converted to unicode. IIRC old Python 3
  >  versions don't accept bytes.
  
  
  Ah, yeah you're right, it only started accepting bytes in 3.6 <https://docs.python.org/3/library/json.html#json.loads>.
  
  In https://phab.mercurial-scm.org/D6113#88992, @yuja wrote:
  
  > >   +    result = pycompat.rapply(lambda x:
  > >   +        encoding.unifromlocal(x) if isinstance(x, bytes) else x,
  > >   +        callconduit(repo, name, params)
  > >   +    )
  > >   +    s = json.dumps(result, sort_keys=True, indent=2, separators=(u',', u': '))
  > >   +    ui.write(b'%s\n' % encoding.unitolocal(s))
  >
  > Maybe we can use `templatefilters.json()` to dump internal (i.e. no unicode)
  >  dict.
  
  
  Ah,  we can indeed (thanks for the pointer), with one caveat:  the formatting.
  The existing output is nicely formatted for human readability, with each entry on a new line and indented appropriately.  `templatefilters.json()` isn't capable of that.
  Do we mind losing that?  It's also tested for in test-phabricator.t. Compare before:
  
    {
      "cursor": {
        "after": null,
        "before": null,
        "limit": 100,
        "order": null
      },
      "data": [],
      "maps": {},
      "query": {
        "queryKey": null
      }
    }
  
  After:
  
    {"cursor": {"after": null, "before": null, "limit": 100, "order": null}, "data": [], "maps": {}, "query": {"queryKey": null}}

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6113

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - March 16, 2019, 2:40 a.m.
>   Ah,  we can indeed (thanks for the pointer), with one caveat:  the formatting.
>   The existing output is nicely formatted for human readability, with each entry on a new line and indented appropriately.  `templatefilters.json()` isn't capable of that.
>   Do we mind losing that?  It's also tested for in test-phabricator.t. Compare before:

Good point. Let's not change the formatting as it is a debug command and
we'll probably want to read the output.
phabricator - March 16, 2019, 2:44 a.m.
yuja added a comment.


  >   Ah,  we can indeed (thanks for the pointer), with one caveat:  the formatting.
  >   The existing output is nicely formatted for human readability, with each entry on a new line and indented appropriately.  `templatefilters.json()` isn't capable of that.
  >   Do we mind losing that?  It's also tested for in test-phabricator.t. Compare before:
  
  Good point. Let's not change the formatting as it is a debug command and
  we'll probably want to read the output.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6113

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -239,10 +239,17 @@ 
     Call parameters are read from stdin as a JSON blob. Result will be written
     to stdout as a JSON blob.
     """
-    params = json.loads(ui.fin.read())
-    result = callconduit(repo, name, params)
-    s = json.dumps(result, sort_keys=True, indent=2, separators=(b',', b': '))
-    ui.write(b'%s\n' % s)
+    # json.(load|dump)s only returns/accepts unicode strings
+    params = pycompat.rapply(lambda x:
+        encoding.unitolocal(x) if isinstance(x, pycompat.unicode) else x,
+        json.loads(ui.fin.read())
+    )
+    result = pycompat.rapply(lambda x:
+        encoding.unifromlocal(x) if isinstance(x, bytes) else x,
+        callconduit(repo, name, params)
+    )
+    s = json.dumps(result, sort_keys=True, indent=2, separators=(u',', u': '))
+    ui.write(b'%s\n' % encoding.unitolocal(s))
 
 def getrepophid(repo):
     """given callsign, return repository PHID or None"""