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

feat: impl BorshSerialize/BorshDeserialize/BorshSchema for char #248

Closed
wants to merge 3 commits into from

Conversation

dj8yfo
Copy link
Collaborator

@dj8yfo dj8yfo commented Oct 16, 2023

resolves #247

@dj8yfo dj8yfo force-pushed the char_impl branch 2 times, most recently from a40db0f to e18f3b3 Compare October 16, 2023 18:02
@dj8yfo dj8yfo changed the title feat: impl BorshSerialize/BorshDeserialize for char feat: impl BorshSerialize/BorshDeserialize/BorshSchema for char Oct 16, 2023
@dj8yfo dj8yfo marked this pull request as ready for review October 16, 2023 18:45
@dj8yfo dj8yfo requested a review from frol as a code owner October 16, 2023 18:45
@mina86
Copy link
Contributor

mina86 commented Oct 23, 2023

FYI, Unicode character is 21-bit number. This could be optimised to encode as 3-byte little endian number. (Another alternative is to use UTF-8 which would be more in line with how strings are serialised).

@frol
Copy link
Collaborator

frol commented Oct 24, 2023

@mina86 Thanks for pointing that out. I have also raised a concern (#247 (comment)) that maybe we should not implement it in borsh unless we strictly define what char is, and given that I have never needed to serialize char nor have I heard a request to do that, I would suggest we don't spend time here.

@dj8yfo dj8yfo closed this Oct 24, 2023
@dj8yfo dj8yfo deleted the char_impl branch August 26, 2024 14:24
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.

add implementation for BorshSerialize, BorshDeserialize, BorshSchema for char
3 participants