Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Object.get and Object.set #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions module.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ static PyObject *StackOverflow = NULL;
//
// Takes ownership of the JSValue and will deallocate it (refcount reduced by 1).
static PyObject *quickjs_to_python(ContextData *context_obj, JSValue value);
// Whether converting item to QuickJS would be possible.
static int python_to_quickjs_possible(ContextData *context, PyObject *item);
// Converts item to QuickJS.
//
// If the Python object is not possible to convert to JS, undefined will be returned. This fallback
// will not be used if python_to_quickjs_possible returns 1.
static JSValueConst python_to_quickjs(ContextData *context, PyObject *item);

// Returns nonzero if we should stop due to a time limit.
static int js_interrupt_handler(JSRuntime *rt, void *opaque) {
Expand Down Expand Up @@ -129,6 +136,42 @@ static void object_dealloc(ObjectData *self) {
PyObject_GC_Del(self);
}

// _quickjs.Object.get
//
// Gets a Javascript property of the object.
static PyObject *object_get(ObjectData *self, PyObject *args) {
const char *name;
if (!PyArg_ParseTuple(args, "s", &name)) {
return NULL;
}
JSValue value = JS_GetPropertyStr(self->context->context, self->object, name);
return quickjs_to_python(self->context, value);
}

// _quickjs.Object.set
//
// Sets a Javascript property to the object.
static PyObject *object_set(ObjectData *self, PyObject *args) {
const char *name;
PyObject *item;
if (!PyArg_ParseTuple(args, "sO", &name, &item)) {
return NULL;
}
int ret = 0;
if (python_to_quickjs_possible(self->context, item)) {
ret = JS_SetPropertyStr(self->context->context, self->object, name,
python_to_quickjs(self->context, item));
if (ret != 1) {
PyErr_SetString(PyExc_TypeError, "Failed setting the variable.");
}
}
if (ret == 1) {
Py_RETURN_NONE;
} else {
return NULL;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need an error message if python_to_quickjs_possible fails.

The unit tests should also cover the failure cases.

Copy link
Collaborator Author

@qwenger qwenger Apr 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed (though this code is copied over / modified from context_set, so that should be the case there too).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, python_to_quickjs_possible does set an error message when failing, so that's fine. Still agreeing on the need for unit tests.

}
}

// _quickjs.Object.__call__
static PyObject *object_call(ObjectData *self, PyObject *args, PyObject *kwds);

Expand All @@ -143,6 +186,8 @@ static PyObject *object_json(ObjectData *self) {

// All methods of the _quickjs.Object class.
static PyMethodDef object_methods[] = {
{"get", (PyCFunction)object_get, METH_VARARGS, "Gets a Javascript property of the object."},
{"set", (PyCFunction)object_set, METH_VARARGS, "Sets a Javascript property to the object."},
{"json", (PyCFunction)object_json, METH_NOARGS, "Converts to a JSON string."},
{NULL} /* Sentinel */
};
Expand Down
15 changes: 15 additions & 0 deletions test_quickjs.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,21 @@ def test_wrong_context(self):
with self.assertRaisesRegex(ValueError, "Can not mix JS objects from different contexts."):
f(d)

def test_get(self):
self.context.eval("a = {x: 42, y: 'foo'};")
a = self.context.get("a")
self.assertEqual(a.get("x"), 42)
self.assertEqual(a.get("y"), "foo")
self.assertEqual(a.get("z"), None)

def test_set(self):
self.context.eval("a = {x: 'overridden'}")
a = self.context.get("a")
a.set("x", 42)
a.set("y", "foo")
self.assertTrue(self.context.eval("a.x == 42"))
self.assertTrue(self.context.eval("a.y == 'foo'"))


class FunctionTest(unittest.TestCase):
def test_adder(self):
Expand Down