-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add feature flag for deriving serde macros. The default is off. #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice lookin good! few change requests.
webrtc-audio-processing-sys/build.rs
Outdated
.replace_all(&contents, "#[derive($d, PartialEq)] pub struct") | ||
.to_string(); | ||
|
||
#[cfg(feature = "derive_serde")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if cfg!(feature = "derive_serde") {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
webrtc-audio-processing-sys/build.rs
Outdated
File::create(&binding_file)?.write_all(new_binding_contents.as_bytes())?; | ||
// Add PartialEq to structs. | ||
// Used for checking partial equality of `Config` struct. | ||
contents = Regex::new(r"#\s*\[\s*derive\s*\((?P<d>[^)]+)\)\s*\]\s*pub\s*struct")? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe best to add a comment to the bindgen issue that is requesting the ability to inject these types of things directly without regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was more relevant issue already existing. I just upvoted a comment there.
rust-lang/rust-bindgen#1089
This is a workaround. Ideally bindgen supports adding custom derive macros, esp common ones like serde.
Some discussion here:
rust-lang/rust-bindgen#1089
rust-lang/rust-bindgen#1301