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

feat: add RTCDataChannel id #3547

Merged
merged 4 commits into from Aug 5, 2023

Conversation

DougAnderson444
Copy link
Contributor

Adds RTCDataChannel readonly attribute id

Set as type unsigned short compiles to u16 which is the size of id (0 to 65,534)

fixes #3542

@DougAnderson444
Copy link
Contributor Author

I don't understand what I did wrong to upset the git diff step, all the generated code looks fine

@DougAnderson444
Copy link
Contributor Author

When I run the cargo run --release --package wasm-bindgen-webidl -- webidls src/features ./Cargo.toml command (Windows 10), I do get a rust fmt error:

Error: rustfmt failed

Caused by:
    The filename or extension is too long. (os error 206)

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

You should never modify the generated files yourself, only modify the WebIDL and then let the generator run.

I'm not sure what the issue is you are running into when running the WebIDL generator, it might be Windows specific, unfortunately I don't have access to a Windows machine right now. It would actually be nice if you can look into it and potentially fix it, but that's a matter for a separate PR.

Feel free to ping me if you don't get the generator to work and I can run it for you.

crates/web-sys/webidls/enabled/RTCDataChannel.webidl Outdated Show resolved Hide resolved
Co-authored-by: daxpedda <daxpedda@gmail.com>
@DougAnderson444
Copy link
Contributor Author

You should never modify the generated files yourself, only modify the WebIDL and then let the generator run.

I thought I did that :/

I'm not sure what the issue is you are running into when running the WebIDL generator, it might be Windows specific,

I'm not sure about this being a Windows issue anymore, because I just committed your suggestion, CI ran the generator, and git diff --exit-code still didn't pass. I see the generated code changed from u16 to Option<u16> so that looks right. So that shouldn't have anything to do with Windows, right?

Feel free to ping me if you don't get the generator to work and I can run it for you.

Yes please do, I'm getting frustrated as I don't really know what to try next. I didn't think adding one line would be so involved! 😐

@daxpedda
Copy link
Collaborator

daxpedda commented Aug 5, 2023

I'm not sure what the issue is you are running into when running the WebIDL generator, it might be Windows specific,

I'm not sure about this being a Windows issue anymore, because I just committed your suggestion, CI ran the generator, and git diff --exit-code still didn't pass. I see the generated code changed from u16 to Option<u16> so that looks right. So that shouldn't have anything to do with Windows, right?

Ah, apologies, I thought you did it by hand instead of running the generator.
I think the generator has a different output on Windows actually ... so looks like a bug to me.

Feel free to ping me if you don't get the generator to work and I can run it for you.

Yes please do, I'm getting frustrated as I don't really know what to try next. I didn't think adding one line would be so involved! neutral_face

No problem, it seems the generator runs correctly for me. Don't let this stop you from contributing though, I'm happy to fix this up in the future as well 😉.

@DougAnderson444
Copy link
Contributor Author

Oh for sure I'm happy to help with open source, I love it 😁

Thanks for all your help

@daxpedda daxpedda merged commit a210654 into rustwasm:main Aug 5, 2023
25 checks passed
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.

feat: add id to RtcDataChannel bindings
2 participants