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
Add GB18030 encode, decode, and decodestream support #1160
Conversation
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.
Just a few small changes but otherwise looks good.
src/strings/gb18030.c
Outdated
} | ||
|
||
MVMint32 gb18030_valid_check_len4_first2(MVMint32 c_1, MVMint32 c_2) { | ||
return (((0x81 <= c_1 && c_1 <= 0x83) || (c_1 == 0x84 && c_2 == 0x30)) && (0x30 <= c_2 && c_2 <= 0x39)) || (c_1 == 0x84 && c_2 == 0x31) ? 1 : 0; |
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.
Same here and all the other places
src/strings/gb18030.c
Outdated
|
||
MVMint32 gb18030_valid_check_len4(MVMint32 c_1, MVMint32 c_2, MVMint32 c_3, MVMint32 c_4) { | ||
if ((0x81 <= c_1 && c_1 <= 0x83) || (c_1 == 0x84 && c_2 == 0x30)) { | ||
return (0x30 <= c_2 && c_2 <= 0x39) && (0x81 <= c_3 && c_3 <= 0xfe) && (0x30 <= c_4 && c_4 <= 0x39) ? 1 : 0; |
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 ? :
ternary operator is not needed as (0x30 <= c_2 && c_2 <= 0x39) && (0x81 <= c_3 && c_3 <= 0xfe) && (0x30 <= c_4 && c_4 <= 0x39)
will evaluate to if true to 1 or false as 0 without it.
src/strings/gb18030.c
Outdated
} | ||
else { | ||
if (i + 1 < bytes) { | ||
// GB18030 codepoint of length 2 |
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.
We prefer using /* comment */
style comments in MoarVM so if you can change all the //
comments to use the /* */
style that would be great.
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.
All required changes are done.
Merging |
This pull request adds GB18030 encoding, the more recent extension to gb2312, to MoarVM.
It contains encode, decode, and decodestream support of GB18030.