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.

Sign up for GitHub

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

Jump to bottom

[C++] Add lone high and low code-point test case for UTF8StringToUTF16 #36173

Closed
sgilmore10 opened this issue Jun 20, 2023 · 3 comments · Fixed by #36383
Closed

[C++] Add lone high and low code-point test case for UTF8StringToUTF16 #36173

sgilmore10 opened this issue Jun 20, 2023 · 3 comments · Fixed by #36383
Assignees
@sgilmore10
Labels
Component: C++ good-first-issue Type: enhancement
Milestone
13.0.0

Comments

@sgilmore10
Copy link
Member

sgilmore10 commented Jun 20, 2023 •
edited by kou
Loading

Describe the enhancement requested

Add lone surrogate pair test case for UTF8StringToUTF16.

Component(s)

C++
@westonpace
Copy link
Member

Is this something you are interested in contributing? Or is there a particular reason why you believe this is needed?

@sgilmore10
Copy link
Member Author

Hi @westonpace,

Apologies, I made this as a followup task for #36167. One reviewer commented that it would be nice to include a test point for this case, but the PR was already merged.

I meant to submit a PR for this last week, but it slipped my mind. I just pushed my branch with the changes here. I'll create a pull request for this issue today.

@sgilmore10
Copy link
Member Author

take

@sgilmore10 sgilmore10 changed the title [C++] Add lone surrogate pair test case for UTF8StringToUTF16 [C++] Add lone high and low code-point test case for UTF8StringToUTF16 Jun 29, 2023
@github-actions github-actions bot mentioned this issue Jun 29, 2023
Merged
pitrou added a commit that referenced this issue Jun 29, 2023
@sgilmore10 @pitrou
GH-36173: [C++] Add lone high and low code-point test case for UTF8St…
52d830e 
…ringToUTF16 (#36383)

### Rationale for this change

This is a followup PR to #36167 that addresses feedback left after the PR was merged.

### What changes are included in this PR?

1. Added a test point verifying `UTF8StringToUTF16` returns an `Invalid` status if given a UTF-8 encoded string that contains a lone high or low code point.
2. Removed `ARROW_EXPORT` from definitions of `UTF8StringToUTF16` and `UTF16StringToUTF18`.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.

* Closes: #36173

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: sgilmore10 <74676073+sgilmore10@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 13.0.0 milestone Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sgilmore10 sgilmore10

Labels
Component: C++ good-first-issue Type: enhancement
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-36173: [C++] Add lone high and low code-point test case for UTF8StringToUTF16 mathworks/arrow
3 participants
@westonpace @pitrou @sgilmore10