-
Notifications
You must be signed in to change notification settings - Fork 46
Import all constants from objc2 #138
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
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.
I spent a bit of time trying to find a case of inconsistency in the actual integer values. I found that using cargo-semver-checks
to actually help me see which of the enum fields actually changed their usize key - the enum usize keys that changed were the ones that never existed. Given that we've got a breaking change in the works with #137 (a now unsafe
function), it's a good time to clean up and break a few things.
Anyway, this is nice! Well done!
_ if os_status == kAudio_UnimplementedError => Err(Error::Unimplemented), | ||
_ if os_status == kAudio_FileNotFoundError => Err(Error::FileNotFound), | ||
_ if os_status == kAudio_FilePermissionError => Err(Error::FilePermission), | ||
_ if os_status == kAudio_TooManyFilesOpenError => Err(Error::TooManyFilesOpen), | ||
_ if os_status == kAudio_BadFilePathError => Err(Error::BadFilePath), | ||
_ if os_status == kAudio_ParamError => Err(Error::Param), | ||
_ if os_status == kAudio_MemFullError => Err(Error::MemFull), |
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.
I find this gross but don't have a better suggestion.
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.
Yes it's really ugly. I have the same here: https://github.com/HEnquist/wasapi-rs/blob/091e0da655a9a33eaeab035f3e373fdd34d5f6f3/src/api.rs#L464
I'll play around with making a macro for this, should be fairly straightforward to make something that isn't so painful to look at.
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.
With a macro, this could be written in this way instead:
gen_match! {
os_status => {
noerr => Ok(()),
kAudio_UnimplementedError => Err(Error::Unimplemented),
kAudio_FileNotFoundError => Err(Error::FileNotFound),
kAudio_FilePermissionError => Err(Error::FilePermission),
kAudio_TooManyFilesOpenError => Err(Error::TooManyFilesOpen),
kAudio_BadFilePathError => Err(Error::BadFilePath),
kAudio_ParamError => Err(Error::Param),
kAudio_MemFullError => Err(Error::MemFull),
_ => Err(Error::Unknown),
}
}
So now we have a nicer looking match. But we also have an ugly and hard to read macro:
macro_rules! gen_match {
($expr:expr => { $($const:ident => $out:expr),* , _ => $default:expr $(,)? }) => {
match $expr {
$(
_ if $expr == $const => $out,
)*
_ => $default,
}
};
Did we gain anything? I'm not convinced..
That semver checking tool is really useful. I didn't know about it but I will start using it. Thanks for the tip :) |
This library uses quite a few constants, for audio unit types, errors, etc etc. Currently, they are defined directly in the core audio-rs code. But their original definitions are in the sdk headers, and that means they are available in the objc2 crates.
This PR removes all the constant defined in this library, and instead imports them from objc2. This way there Is much less risk for typos, and if there is a typo is much easier to spot. The only exception is the kAudioUnitSubType_RemoteIO subtype which only exists in the iOS sdk. I have not managed to figure out how to import it, so I left this one as it was.
The api almost doesn't change. Only one missing BadData error is added, and some incomplete mapping between os status and error is fixed.