[PATCH 0 of 1 stable 3] accept long as revs

Sune Foldager cryo at cyanite.org
Mon May 21 09:36:33 CDT 2012


On 2012-05-21 09:28, Matt Mackall wrote:
>On Mon, 2012-05-21 at 15:23 +0200, Sune Foldager wrote:
>> Since the index is no longer an array (is there a document about the design of the
>> new ADT somewhere?), I had to change the C code to make this work. Note that if we
>> don't take this patch, since some places do accept long, I bet it will fail in the
>> C code (with the weird message "expected string or Unicode. Got long").
>
>Whoa whoa whoa. I thought we were talking about a one-liner in
>context.py.
>
>Again, using a long here is utterly bogus and misguided. Catching it in
>one central place is acceptable, letting that nonsense ripple down into
>the C code is not.

I'm sorry, but I don't agree. Just because Mercurial doesn't support more than
some-number-less-than-intmax revisions, doesn't mean long shouldn't work. No need
for all the superlatives, really.

It's already handled in several places in our code, and Python in general accepts ints
and longs throughout. It's only because we have special isinstance code which really
isn't optimal anyway, and because the C code has been introduced that doesn't use
PyLong_AsLong (in order to work with both types).

If we don't take this or a similar patch, we have a situation where longs work in
some places, and not in others, and where it might fail in C code if you use them,
with a somewhat obscure error message.

-Sune


More information about the Mercurial-devel mailing list