-
Notifications
You must be signed in to change notification settings - Fork 181
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 C with Cedilla GSM code point #318
Conversation
EncodingDetector incorrectly uses the lowercase version of `Latin Small Letter C with Cedilla (U+00E7)` instead of `Latin Capital Letter C with Cedilla (U+00C7)`. See https://en.wikipedia.org/wiki/GSM_03.38#GSM_7-bit_default_alphabet_and_extension_table_of_3GPP_TS_23.038_/_GSM_03.38
Hello, anything blocking this PR? |
I'm working on a fairly major release, so I'm aiming to incorporate this into it. As usual with libraries like this, time is always short |
I've enabled the test runner - can you please fix the tests accordingly? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test suite is still failing - can you update the tests to pass and run locally before pushing? Thanks
Fixed, tests are passing locally. |
Codecov Report
@@ Coverage Diff @@
## main #318 +/- ##
=========================================
Coverage 82.37% 82.37%
Complexity 1928 1928
=========================================
Files 157 157
Lines 4918 4918
=========================================
Hits 4051 4051
Misses 867 867
Continue to review full report at Codecov.
|
Awesome, thanks for this |
Description
EncodingDetector incorrectly uses the lowercase version of
Latin Small Letter C with Cedilla (U+00E7)
instead ofLatin Capital Letter C with Cedilla (U+00C7)
.See https://en.wikipedia.org/wiki/GSM_03.38#GSM_7-bit_default_alphabet_and_extension_table_of_3GPP_TS_23.038_/_GSM_03.38
Motivation and Context
Prevents sending characters that require unicode encoding to be sent as text, causing invalid characters to be displayed in an SMS.
How Has This Been Tested?
Manual call to
EncodingDetector::requiresUnicodeEncoding()
after correcting the list.Example Output or Screenshots (if appropriate):
Types of changes
Checklist: