-
Notifications
You must be signed in to change notification settings - Fork 19
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks useful
if (ret == 1) { | ||
Py_RETURN_NONE; | ||
} else { | ||
return NULL; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
When I originally created this library, I did the bare minimum for it to be useful in production. The API was deliberately kept small to reduce the likelihood of bugs. However, I don't use it in production any more, so if you wish to make it easier to use, we can do so. Just make sure everything including failures/exceptions is covered by tests.
May be a good idea to merge them, yes.
I like this. Can be implemented in Python code as a convenience. |
Could you please have a look at #82? It is an alternative proposal that goes further by merging |
Implement
Object.get
andObject.set
using the same ideas asContext.get
andContext.set
.Currently it is very tedious to get or set an object property. When the object is only known via its Python wrapper, one has to first rebind it to the global context using
Context.set
and then access it withContext.get
. With this, one could directly access or set the property via the Python wrapper.The
json()
method is not sufficient for this, as it does not expose properties from the prototype.Discussion:
get
,set
, etc. methods of Context, add a genericget_global
or similar to get the global object from the context)Further discussion (for the future?):
add_callable
? Should Object also support it? Should we mergeset
andadd_callable
?