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 lua-objc mapping to handle high/low surrogate pairs in hs.styledtext #3356

Merged
merged 2 commits into from
Feb 9, 2023

Conversation

asmagill
Copy link
Member

Addresses issues in #3219

Completely rewrites lua-objc index mapping to properly handle UTF16 High/Low surrogate pairs

UTF8 (which Lua uses) can have Unicode characters of 1-4 bytes
UTF16 (which Objective-C/Swift use) are always 2 bytes, but for Unicode characters above U+10000 inclusive, 2 UTF16 characters (i.e. 4 bytes) are used.

This code assumes that no 1-byte UTF8 character can be a surrogate member... as far as I can tell this is a safe assumption, but I haven't found an explicit statement to that effect, so if anyone knows otherwise, please chime in.

@asmagill asmagill added the pr-fix Pull Request implementing a bug fix label Jan 13, 2023
@github-actions
Copy link

View Test Results

348 tests   296 ✔️  11m 44s ⏱️
    2 suites    52 💤
    1 files        0

Results for commit a6c0744.

@cmsj
Copy link
Member

cmsj commented Feb 7, 2023

@asmagill I'm not going to pretend to understand all the unicode stuff - are you happy for this to merge?

@asmagill
Copy link
Member Author

asmagill commented Feb 7, 2023

I left in a debugging function that I originally had commented out... it's undocumented, but might be helpful if someone does find a specific case where this fails, so I'm inclined to leave it as is. If you don't mind the undocumented function being left in, then yes, I think this is ready to be merged -- it hasn't broken anything on my setup and passed every test I've thought to come up with at it...

Though to be fair, I don't think a set of comprehensive unicode tests that covers absolutely all use cases exists... sometimes I feel like they make it up as they go! 😁

@cmsj cmsj merged commit b9a6a0d into Hammerspoon:master Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-fix Pull Request implementing a bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants