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

Switch to a different murmurhash2 implementation to handle unicode characters #9158

Merged
merged 2 commits into from
Oct 15, 2020

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Oct 13, 2020

References

Port of jupyterlab/debugger#549

Code changes

This change vendors the implementation of the murmurhash implementation we were using: https://github.com/garycourt/murmurhash-js

This implementation was for ascii strings only: https://github.com/garycourt/murmurhash-js/blob/0197ce38bedac0e05f40b9d7152095d06db8292c/murmurhash2_gc.js#L9

See jupyterlab/debugger#549 for more context.

The vendored implementation is heavily based on the js implementation, but uses a TextEncoder to encode the string into a Uint8Array before running the algorithm, so it behaves the same as the original C++ implementation (used on xeus-python).

User-facing changes

Before

unicode-before

After

unicode-after

Backwards-incompatible changes

None

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member Author

jtpio commented Oct 14, 2020

cc @afshin

@jtpio jtpio added this to the 3.0 milestone Oct 14, 2020
@@ -0,0 +1,69 @@
// Copyright (c) Jupyter Development Team.
Copy link
Member Author

@jtpio jtpio Oct 14, 2020

Choose a reason for hiding this comment

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

This file is vendored as part of the implementation of the debugger package to avoid increasing the API surface for now.

Copy link
Member

@afshin afshin left a comment

Choose a reason for hiding this comment

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

Thank you!

👍

@afshin afshin merged commit 3bc0ab1 into jupyterlab:master Oct 15, 2020
@jtpio jtpio deleted the debugger-murmur branch October 15, 2020 14:16
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Apr 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:debugger status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants