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

Protect against hypothetical future where javascript stops using surrogate pairs #2560

Merged
merged 4 commits into from
Jun 8, 2017

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 8, 2017

to avoid introducing the inverse of jupyter/jupyter_client#259

and check surrogate pairs more strictly, in case of wonky unicode that shouldn't happen but is technically possible.

bumps protocol version to 5.2 for explicit unicode fix (jupyter/jupyter_client#262)

follow-up to #2509

minrk added 2 commits June 8, 2017 12:44
check both surrogates with strict bounds, not just the first, in case of weirdness.
…e sensibly

We don't need to count surrogate pairs if js stops using them
@takluyver
Copy link
Member

This looks sensible to me.

Do you think there's any possibility that CodeMirror could switch to using the index in code points rather than code units, even if it's not fixed at the language level? I just realised that's still not something that this code would catch, but I suspect it's unlikely.

@stevengj
Copy link

stevengj commented Jun 8, 2017

@takluyver, almost certainly not, unless some hypothetical future version of JavaScript switches its indexing behavior (ala Python 2 vs Python 3, but this seems unlikely): codemirror/codemirror5#4750

@takluyver
Copy link
Member

Ok, this is fine by me then.

@takluyver takluyver merged commit 9f578dc into jupyter:master Jun 8, 2017
@minrk minrk deleted the gate-surrogate-pairs branch June 8, 2017 16:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants