Created attachment 137338 [details] [review]
girepository-Add-short-ushort-support.patch

Comment on attachment 137338 [details] [review]
girepository-Add-short-ushort-support.patch

Patch looks good!

Simon is moving the type check code over to PythonC, you should probably sort out conflicts with him before landing this.

>diff --git a/girepository/btypes.py b/girepository/btypes.py
>index 46319c4..6eb6d24 100644
>--- a/girepository/btypes.py
>+++ b/girepository/btypes.py
>@@ -45,8 +45,10 @@ class Callable(object):
>                         name, type(value).__name__))
>         elif tag in (repo.TYPE_TAG_INT,
>                      repo.TYPE_TAG_INT8,
>+                     repo.TYPE_TAG_SHORT,
>                      repo.TYPE_TAG_UINT,
>                      repo.TYPE_TAG_UINT8,
>+                     repo.TYPE_TAG_USHORT,
>                      repo.TYPE_TAG_INT16,
>                      repo.TYPE_TAG_UINT16,
>                      repo.TYPE_TAG_INT32):
>@@ -56,6 +58,7 @@ class Callable(object):
>                 raise TypeError("%s must be int, not %s" % (name, type(value).__name__))
>             if tag in (repo.TYPE_TAG_UINT,
>                        repo.TYPE_TAG_UINT8,
>+                       repo.TYPE_TAG_USHORT,
>                        repo.TYPE_TAG_UINT16) and value < 0:
>                 raise TypeError("%s must be an unsigned value, not %s", name, value)
>         elif tag in (repo.TYPE_TAG_UINT32,

Indeed, I rewrote those checks in C (but my branch is not public yet).

>diff --git a/tests/test_girepository.py b/tests/test_girepository.py
>index 3c626e5..dc77aed 100644
>--- a/tests/test_girepository.py
>+++ b/tests/test_girepository.py
>@@ -43,6 +43,11 @@ class TestGIEverything(unittest.TestCase):
>         self.assertFalse(Everything.test_boolean(0))
>         self.assertTrue(Everything.test_boolean(2))
> 
>+    def testShort(self):
>+        self.assertEqual(3, Everything.test_short(3))
>+        self.assertEqual(-3, Everything.test_short(-3))
>+        self.assertRaises(TypeError, Everything.test_short, 'a')
>+
>     def testInt(self):
>         self.assertEqual(3, Everything.test_int(3))
>         self.assertEqual(-3, Everything.test_int(-3))


Wouldn't it be helpful to test that it works (and make it working that way) with Python longs as well, and that G_MINSHORT and G_MAXSHORT are accepted whereas G_MINSHORT-1 and G_MAXSHORT+1 raise an error?

This is not what is currently in upstream, but I've got patches for the other cases.

Created attachment 137484 [details] [review]
patch based off of the invoke-checks-rewriting branch

(In reply to comment #3)
> Indeed, I rewrote those checks in C (but my branch is not public yet).

This new patch is based off of this branch. I'm not obsoleting the other patch, at Simon's request.

> Wouldn't it be helpful to test that it works (and make it working that way)
> with Python longs as well, and that G_MINSHORT and G_MAXSHORT are accepted
> whereas G_MINSHORT-1 and G_MAXSHORT+1 raise an error?

These tests are now incorporated into the short/ushort unit tests.

This is a year old.  Is it still relevant?

Mark: Is this still relevant?

Mark, Ping.

Reopening as I can't see any non-developer question. But a maintainer might want to clear to question reg. the relevance of this bug.

3.2.0 (and earlier versions as well) support short, and have test cases. Closing.