In D6082#89656, @pulkit wrote:

In D6082#88981, @Kwan wrote:

In D6082#88852, @mharbison72 wrote:

In D6082#88851, @pulkit wrote:

@mharbison72 thanks for tips on adding test. Will add tests in next iteration.

I found that Differentials do have a branch field, maybe we can use that? https://secure.phabricator.com/source/phabricator/browse/master/src/applications/differential/customfield/DifferentialBranchField.php

Seems worth a shot. I don’t know anything about it, but presumably this would be rendered specially in the web UI, like the test plan, etc. That sounds better than as a follow up comment. It also seems natural enough that maybe it can be done unconditionally, instead of needing the argument.

Yeah, it shows in the Diff Detail pane, like here. Unfortunately I think it can be only set when using the creatediff endpoint, which is what I had to change my fork to do (though I could be wrong, the conduit docs are too sparse to be sure).

Last night I tried different endpoints and looks like creatediff is the only one which can be used to set the branch. I have asked on their discourse https://discourse.phabricator-community.org/t/conduit-api-to-set-branch-of-a-differential/2521.

Talking about creatediff, can it be used instead of createrawdiff? It it's possible can you paste the changes you did your fork, maybe we need the exact same changes.

Make compatible with Py3.5, separate comments about json functions so they are more readable.

Switch to using pycompat.byteskwargs.

In D6103#88988, @yuja wrote:

Queued most of the patches in this series, thanks.

-        symbol = b''.join(itertools.takewhile(lambda ch: ch not in special,
-                                              view[pos:]))
+        symbol = b''.join(pycompat.iterbytestr(itertools.takewhile(
+            lambda ch: ch not in special,
+            view[pos:]

It's probably better to apply iterbytestr() to view[pos:] so ch is
always a byte string.

In D6107#88990, @yuja wrote:

pycompat.byteskwargs() can be used instead.

In 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.

In 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}}

In D6082#88852, @mharbison72 wrote:

In D6082#88851, @pulkit wrote:

@mharbison72 thanks for tips on adding test. Will add tests in next iteration.

I found that Differentials do have a branch field, maybe we can use that? https://secure.phabricator.com/source/phabricator/browse/master/src/applications/differential/customfield/DifferentialBranchField.php

Seems worth a shot. I don’t know anything about it, but presumably this would be rendered specially in the web UI, like the test plan, etc. That sounds better than as a follow up comment. It also seems natural enough that maybe it can be done unconditionally, instead of needing the argument.

Urgh, never mind, just realised I missed the existence of HGREVISION in the documentation.

In D6044#88262, @yuja wrote:

• parsed = json.loads(body) + parsed = pycompat.rapply(lambda x: encoding.unitolocal(x) + if isinstance(x, unicode) else x, json.loads(body))

s/unicode/pycompat.unicode/

Thanks, done.

Perhaps some of r'' would have to be changed to b'' since dict keys
are now byte strings. See the wiki page for our Py3 hacks.

https://www.mercurial-scm.org/wiki/Python3

Okay, making progress on the test now, just need to figure out how to update the vcr files cleanly.
Slightly complicated by the fact that there was a small error in the original creation of the phabsend-update-alpha-create-beta.json recording. It must have been made after the updated diff for alpha was already on here, because it hasn't recorded the alpha update, since the query reported the diff to already be the updated version.

Apologies for the lack of a test, but I can't seem to write one that works. Either the test harness isn't capable of tests with unicode in them, or I don't know how to represent it in the test files in such a way that works.