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

Fix Unicode support to cope with Javascript Unicode size limitations #23

Merged
merged 1 commit into from Apr 19, 2013

Conversation

Fire-
Copy link
Contributor

@Fire- Fire- commented Mar 23, 2013

Looks like jison was parsing incorrectly due to a size limitation with javascript unicode support

Test data:

id: 1
data: hello world

event: test
data: {"body":"豆腐"}


Python:

0x8C46 in range(0x003B,0x10FFFF)
True

Jison:

[\u003B-\u10FFFF] return 'char';

Parse error on line 5:
...testdata: {"body":"豆腐"}
----------------------^
Expecting 'eol', 'colon', 'char', 'space', got 'INVALID'

Python:

0x8C46 in range(0x003B,0x9FFFF)
True

Jison

[\u003B-\u9FFFF] return 'char';

[{"comments":[],"data":"hello world\n","id":"1"},{"comments":[],"data":"{\"body\":\"豆腐\"}\n","event":"test"}]

it also happens in both chrome and firefox's js consoles:

"豆".match(/[0-\u10FFFF]/)
null

"豆".match(/[0-\uFFFFF]/)
["豆"]

"豆".match(/[0-\u9FFFF]/)
["豆"]

Even a single decimal character less ( 10 -> F , 16 to 15 ) makes it work with no discernible impact on actual usability ( It's a single character at the very tip top of Supplemental Private Use Area-B )

@aslakhellesoy
Copy link
Contributor

Looks great! I'm away for a few weeks. Please remind ne to merge and release if I haven't in 2 weeks.

@Fire-
Copy link
Contributor Author

Fire- commented Apr 17, 2013

@aslakhellesoy Poking for reminder, bit past two weeks though, sorry!

aslakhellesoy added a commit that referenced this pull request Apr 19, 2013
@aslakhellesoy aslakhellesoy merged commit 6ffaad1 into EventSource:master Apr 19, 2013
@aslakhellesoy
Copy link
Contributor

Thanks for the patch and the reminder! Merged. Will cut a release shortly.

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.

None yet

2 participants