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

Python 3 compatibility for WebIDL.py #5799

Merged
merged 4 commits into from
Nov 20, 2017

Conversation

saschanaz
Copy link
Collaborator

@saschanaz saschanaz commented Nov 16, 2017

Syntax changes for WebIDL.py. Closes #5788.

... which includes except-as, print function, and removal of xrange.
and create a class manually.
...which cannot introduce any new behavior.
@kripken
Copy link
Member

kripken commented Nov 16, 2017

The python meta stuff goes over my head, someone else should review that part... otherwise looks good to me.

Please also run the webidl browser tests (I think there's one or two) that aren't done on travis.

@dschuff
Copy link
Member

dschuff commented Nov 16, 2017

Actually if I'm understanding it correctly, it seems less meta and magical now, so 👍

@saschanaz
Copy link
Collaborator Author

BTW, recently A implements B IDL syntax is removed and replaced with A includes B. whatwg/webidl#433

Do we want to support the new syntax?

@saschanaz
Copy link
Collaborator Author

A note to ease reviewing: The original Foo receives the target class name, its base class list and an (empty) attribute dictionary to create an actual class type object. The new Foo does not automatically get them so it creates them by itself. Both Foo adds enum items as attributes so that they are available on instances and subclasses.

@kripken kripken merged commit b634968 into emscripten-core:incoming Nov 20, 2017
@kripken
Copy link
Member

kripken commented Nov 20, 2017

It might be nice to support the newer webidl syntax, but if it means not supporting the old one, or if it would take effort, probably not worth it.

@saschanaz saschanaz deleted the webidlpy branch November 21, 2017 01:47
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.

3 participants