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

All types of Bytes should be available in hex #300

Closed
TheAlgorythm opened this issue Apr 17, 2021 · 2 comments · Fixed by #301
Closed

All types of Bytes should be available in hex #300

TheAlgorythm opened this issue Apr 17, 2021 · 2 comments · Fixed by #301

Comments

@TheAlgorythm
Copy link
Contributor

I have a [u8, N] and want to deserialize it in the hex format. Base64 would also be cool but also way more complex.

@jonasbb
Copy link
Owner

jonasbb commented Apr 17, 2021

Hi, thanks for the interest.
It seems that serialization already works with the existing Hex type, only deserialization is impossible since there is no [u8; N]: From<Vec<u8>> implementation.

Even disconnected from this issue, it might be worthwhile to see if the DeserializeAs implementation can be changed to something like this, since that would allow TryFrom in addition to all currently existing From implementations.
This also does not work for arrays though, since the TryFrom error does not implement Display.

impl<'de, T, E, FORMAT> DeserializeAs<'de, T> for Hex<FORMAT>
where
    T: TryFrom<Vec<u8>, Error = E>,
    E: Display,
    FORMAT: Format,

Therefore, I think the only way forward is to add another Hex-like type especially for array, a HexArray. Do you want to contribute this? The implementation in hex.rs is quite simple.
If you want to contribute base64 too, I would be happy to accept that too. Implementation wise it shouldn't differ too much from the hex type, except that there are different base64 character sets. But they can always be added later.

bors bot added a commit that referenced this issue Apr 19, 2021
301: Add array deserialization support to `Hex` r=jonasbb a=TheAlgorythm

I did what I thought you described on #300.
Please let me know if there are any issues.

Co-authored-by: ZSchoen <dev@zschoen.dev>
Co-authored-by: Jonas Bushart <jonas@bushart.org>
@bors bors bot closed this as completed in #301 Apr 19, 2021
@jonasbb
Copy link
Owner

jonasbb commented Apr 19, 2021

I just released a new version of the crate with this fix included. Thanks for implementing the fix.

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 a pull request may close this issue.

2 participants