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

Bug with utf8casecmp? #60

Closed
kainjow opened this issue Jul 29, 2019 · 3 comments · Fixed by #62
Closed

Bug with utf8casecmp? #60

kainjow opened this issue Jul 29, 2019 · 3 comments · Fixed by #62

Comments

@kainjow
Copy link

@kainjow kainjow commented Jul 29, 2019

It seems utf8casecmp is not working correctly. I was trying to use it with std::set as a custom comparator. I compared it to strcasecmp and found it is not giving the same results for basic ASCII strings. Note I wouldn't expect the same values, but I would expect it to match negative or positive.

    printf("%d\n", strcasecmp(".gdoc", ".GSHeeT")); // -15
    printf("%d\n", utf8casecmp(".gdoc", ".GSHeeT")); // 1
    printf("%d\n", strcasecmp(".gsheet", ".gSLiDe")); // -4
    printf("%d\n", utf8casecmp(".gsheet", ".gSLiDe")); // 1
@kainjow

This comment has been minimized.

Copy link
Author

@kainjow kainjow commented Jul 29, 2019

So I got it working by changing the last part of utf8casecmp from:

// If they don't match, then we return which of the original's are less
if (src1_orig_cp < src2_orig_cp) {
  return -1;
} else if (src1_orig_cp > src2_orig_cp) {
  return 1;
}

To:

// If they don't match, then we return the difference between the characters
return src1_cp - src2_cp;

This matches strcasecmp's output.

@sheredom

This comment has been minimized.

Copy link
Owner

@sheredom sheredom commented Nov 30, 2019

I'm so sorry I missed this - all my notifications were disable in GitHub at some point and I don't know why, but I missed every single issue on every lib I have :(

Are you happy to file a PR for the above, or do you want me to file it?

@kainjow

This comment has been minimized.

Copy link
Author

@kainjow kainjow commented Dec 23, 2019

You can go ahead.

sheredom added a commit that referenced this issue Dec 24, 2019
Fixes #60.
sheredom added a commit that referenced this issue Dec 24, 2019
Fixes #60.
@sheredom sheredom closed this in #62 Dec 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.