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

add ruby String to rust Byte Collection conversion #57

Merged
merged 2 commits into from
Feb 10, 2023

Conversation

cramt
Copy link
Contributor

@cramt cramt commented Feb 8, 2023

i Added support for converting a ruby String into a rust byte collection

In ruby the main way to represent some bytes is with a string, (see File.read or Base64.strict_decode64). and the current way to accept such
a string with magnus is by receiving a RString and calling .as\_slice().as\_vec() which is unsafe, this creates safe interface to receive bytes from ruby.

I've decided to use the Bytes crate since it seems to be rust communty's prefered way of dealing with collections of bytes and made the inclusion of it a non defaulted feature

Copy link
Owner

@matsadler matsadler left a comment

Choose a reason for hiding this comment

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

Thanks, this is great!

Could you add an IntoValue implementation for Bytes too? That way it can also be returned to Ruby.
I'm in the process of transitioning from Into<Value> to IntoValue, but Into<Value> won't go away til after the next release, so there should probably be a impl From<bytes::Bytes> for Value too.

I'll add an entry to the CHANGELOG.md for this after I merge, but feel free to add it there yourself in your own words if you want to.

src/try_convert.rs Show resolved Hide resolved
src/r_string.rs Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@matsadler
Copy link
Owner

Thank you!

@matsadler matsadler merged commit 30297d5 into matsadler:main Feb 10, 2023
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.

None yet

2 participants