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

Update WebIDL to use undefined instead of void #2427

Merged
merged 2 commits into from
Jan 22, 2021

Conversation

grovesNL
Copy link
Contributor

We updated weedle to use undefined instead of void in rustwasm/weedle#43, so now the plan is to update all WebIDL too.

The changes here are split into two commits for reviewing. The first commit updates all of the WebIDL and the second commit updates how the tokens are used from weedle.

Overall the changes seem to be fine because the generated bindings don't show any differences.

@@ -21,7 +21,7 @@ proc-macro2 = "1.0"
quote = '1.0'
syn = { version = '1.0', features = ['full'] }
wasm-bindgen-backend = { version = "=0.2.69", path = "../backend" }
weedle = "0.11"
weedle = { git = "https://github.com/rustwasm/weedle" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll need to use a published version here, so this PR can wait for that

@@ -503,6 +503,10 @@ impl<'src> FirstPass<'src, (&'src str, ApiStability)> for weedle::interface::Int
log::warn!("Unsupported WebIDL Setlike interface member: {:?}", self);
Ok(())
}
InterfaceMember::AsyncIterable(_iterable) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to undefined/void changes, it's just a new variant added between weedle 0.11 and weedle master also noticed in #2373 (comment)

crates/webidl/src/idl_type.rs Show resolved Hide resolved
@alexcrichton
Copy link
Contributor

Looks great to me, thanks! I think the only thing this needs now is a weedle publish, right?

@grovesNL
Copy link
Contributor Author

Yeah I think we just need to publish a new version of weedle and use it here, which will also unblock progress in #2373. Created a PR at rustwasm/weedle#44

@alexcrichton
Copy link
Contributor

Ok published!

@grovesNL grovesNL marked this pull request as ready for review January 21, 2021 23:20
@grovesNL
Copy link
Contributor Author

Thanks! This should be ready now

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