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

WIP: Support WebIDL modules #31

Closed
wants to merge 6 commits into from
Closed

WIP: Support WebIDL modules #31

wants to merge 6 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 4, 2019

Companion PR to rustwasm/wasm-bindgen#1746; adds support for WebIDL modules.

@alexcrichton
Copy link
Contributor

Looks great to me! I'm mostly just assuming this matches the syntax of the spec itself but I don't see any issues in merging this

@ghost
Copy link
Author

ghost commented Sep 7, 2019

@alexcrichton do you have any thoughts on the best way to represent scoped identifiers here?

Right now I am just parsing the qualifying prefix to Vec<&'src str> but it seems like we would need to change that to Vec<String> or at least Vec<Cow<'src, str>> since there is string processing (case manipulation, etc.) that happens during code generation in wasm-bindgen-webidl.

It also seems like we are going to want to intern the identifiers at some point for performance.

@alexcrichton
Copy link
Contributor

I don't have too much of a preference myself, I don't really know a huge amount about WebIDL or modules or such, I just learned enough to get web-sys implemented :)

@ghost
Copy link
Author

ghost commented Sep 19, 2019

Just want to give a quick update on this. I've decided to spend some time working on a rewrite targeting nom 5 first since I've found it far less painful to work with than the current macro heavy implementation. I also wanted to take the opportunity to try and simplify the AST.

I wasn't sure how a rewrite would be received so I've been working on that in a separate crate here.

Once it's further along (can parse webidls) I'd be happy to donate the parser crate to this project (and take ownership of maintenance) if y'all think that's a good idea.

@alexcrichton
Copy link
Contributor

Nice!

We're not necessarily wed to any particular WebIDL parser or the way things are, so long as it serves the purposes of web-sys and it builds quickly I think we're all good to go :)

@fitzgen
Copy link
Member

fitzgen commented Sep 24, 2019

I do love the name weedle tho, so whatever we end up using should probably live here ;)

@saschanaz
Copy link
Contributor

whatwg/webidl#675 is dropped, so this should probably be closed.

@alexcrichton
Copy link
Contributor

Ah ok!

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.

4 participants