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

UTF-16 surrogate pairs #8

Merged
merged 3 commits into from
Oct 12, 2018
Merged

UTF-16 surrogate pairs #8

merged 3 commits into from
Oct 12, 2018

Conversation

patchyderm
Copy link

Enclosed is my code to support JSON UTF-16 surrogate pairs.

Please review and suggest any appropriate improvements.

Credit goes to Jason Miller & Hans Hübner of yason, where I got the elegant and simple translation logic to convert UTF-16 to code points.

While I have also added writer as well (the original code would still work but may output non-Ascii characters - this is still possible with the output-literal-unicode option).

I am sure many JSON libraries (notably cl-json hankhero/cl-json#11) continue to suffer from this bug/missing feature, though this feature seems to be used mainly for representing emojis today!

PS: My tests are current embedded as comments within the source file, one day, I assume they can move into properly structured unit tests. It might be a good idea to share certain test samples across all JSON libraries too?

patchyderm added 3 commits October 8, 2018 21:57
Characters outside the Basic Multilingual Plane have until now
been (mistakenly) parsed into characters with invalid code points.
Not sure why this hasn't been discovered before, it wouldn't have
worked before. Also refactored the code slightly given large chunks of
repeated code.
Also added an option to turn that off if in cases where it'll be
simpler.
@marijnh
Copy link
Owner

marijnh commented Oct 8, 2018

Hi. I'm not really maintaining this library anymore. Would you be interested in taking over maintenance?

@patchyderm
Copy link
Author

You know what, I looked at the known forks on github, and @jl2 seems to be furthest ahead (with more merges compared to the other forks too)

https://github.com/jl2/ST-JSON

Could I suggest handing maintainership over to jl2 instead? I'm more than happy to rebase my commits to that branch instead.

@marijnh
Copy link
Owner

marijnh commented Oct 8, 2018

If @jl2 is interested in maintaining this, I'm absolutely okay with that

@jl2
Copy link
Collaborator

jl2 commented Oct 10, 2018

Hi, sorry I missed this earlier, but I'd be happy to maintain this.

I think I have at least one backwards incompatible change in my fork (getjso* as a function instead of macro), but I can move that to a separate function to maintain compatibility.

I'll take a look after work today to make sure I've got the latest code in my fork.

@marijnh
Copy link
Owner

marijnh commented Oct 11, 2018

Great! I've added you as a collaborator here, but if you want we can also make the readme point at your repository instead.

@jl2
Copy link
Collaborator

jl2 commented Oct 12, 2018

Thank you!

I think it's easiest to keep everything pointed here for now. And I've never been a collaborator on a GitHub project, so I'm curious to see how it works.

@jl2 jl2 merged commit a756e17 into marijnh:master Oct 12, 2018
@patchyderm
Copy link
Author

Yay!

Thanks to Marijn for creating this project, and jl2 for taking over its maintainance!

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.

3 participants