-
Notifications
You must be signed in to change notification settings - Fork 427
Add bindings for git_attr_value()
#671
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
src/lib.rs
Outdated
|
||
/// Converts [`AttrValueBytes`] to [`AttrValue`]. If the attribute is [set to a string](`AttrValueBytes::String`), | ||
/// this implementation will use [`str::from_utf8`] to perform the conversion. | ||
impl<'string> TryFrom<AttrValueBytes<'string>> for AttrValue<'string> { |
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.
Instead of having two enums, could there perhaps be one enum with a string/bytes variant where the bytes are guaranteed to always be invalid utf-8?
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.
where the bytes are guaranteed to always be invalid utf-8?
Does that mean AttrValue::new()
will always perform UTF-8 validation? Could I add another new
that skips UTF-8 validation and always return bytes if the value is string?
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.
Seems reasonable to me!
Thanks! |
New public APIs
Summary
Repository::get_attr
andRepository::get_attr_bytes
will probably be changed in the next major version in their documentations.Wrapping the return value of
Repository::get_attr(_bytes)
in a new type is probably better, but that's a breaking change, so it's probably not desirable unless it's ready to bump to 0.14.Resolves #670.