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

Use JS/Python prototype instead of conditions to choose the correct JS <=> Brython conversion method #2300

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

denis-migdal
Copy link
Contributor

Enable users to declare new JS <=> Brython conversions without any additional cost to the conversion functions.
It enables to delegate this responsibility making conversions functions more clear/readable.

It doesn't seem to have a big impact on performances when using the test suite ?
I'll fix the server.py CGI to see the speed test report.

$B.addPy2JSWrapper(pyclass, function(pyobj) {
	
        return .... // return your jsobj
});

$B.addJS2PyWrapper(JSClass, function(jsobj) {
        return ... // return your pyobj
});

@PierreQuentel
Copy link
Contributor

Sorry Denis, I won't accept this PR. Conversion to and from Javascript should not be included in py_dict.js, py_string.js and the like, these scripts are implementation of the Python objects.

The interface between Brython and Javascript is only a part of the Brython project, and one I don't give a huge importance to: I am more interested in providing tools to write client-side programs without having to write in Javascript.

All or most of the JS/Brython interface should remain in js_objects.js.

@denis-migdal
Copy link
Contributor Author

So should I make a commit moving them to js_objects.js ?

@PierreQuentel
Copy link
Contributor

So should I make a commit moving them to js_objects.js ?

Yes, please.

@denis-migdal
Copy link
Contributor Author

Yes, please.

Done. I can't move the one for DOMNode as it is defined after execution of js_object.js.

I added a "description", so that we can have an overview on the convertions being made.

@PierreQuentel what should be the next step ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants