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

Move unicodedata to minipal #102139

Merged
merged 13 commits into from
May 16, 2024
Merged

Move unicodedata to minipal #102139

merged 13 commits into from
May 16, 2024

Conversation

am11
Copy link
Member

@am11 am11 commented May 12, 2024

Relocated next to utf8.c. Data generation mechanism remained the same:

$ curl -sSLo /tmp/UnicodeData-14.txt https://www.unicode.org/Public/14.0.0/ucd/UnicodeData.txt

$ cd runtime
$ ./dotnet.sh run --project src/native/minipal/UnicodeDataGenerator /tmp/UnicodeData-14.txt > src/native/minipal/unicodedata.c

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 12, 2024
@am11
Copy link
Member Author

am11 commented May 13, 2024

Moving to minipal is bringing related code together. We can also hook it in mono to address things like

/* FIXME: g_strcasecmp supports utf8 unicode stuff */

src/native/minipal/utf8.h Outdated Show resolved Hide resolved
src/native/minipal/utf8.h Outdated Show resolved Hide resolved
* @param code The UTF-16 character to be converted.
* @return The uppercase equivalent of the character or the character itself if no conversion is necessary.
*/
CHAR16_T minipal_toupper_invariant(CHAR16_T code);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two methods have nothing to do with UTF8. Should they be in a separate header?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header was dealing with UTF-8 and UTF-16, so I put them here. They can be moved to a separate one e.g. minipal/unicodedata.h?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would work. (Alternative: give the header a more generic name.)

@am11 am11 force-pushed the feature/minipal/unicodedata branch from 2e843b9 to e649c7b Compare May 15, 2024 07:54
src/native/minipal/strings.h Outdated Show resolved Hide resolved
src/native/minipal/strings.h Outdated Show resolved Hide resolved
am11 added 3 commits May 15, 2024 18:39
C mode (/TC) requires explicit windows.h as opposed to C++ mode (/TP).
src/native/minipal/types.h Outdated Show resolved Hide resolved
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jkotas jkotas merged commit 5025f5e into dotnet:main May 16, 2024
162 checks passed
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
* Move unicodedata to minipal

* Fix linux build

* Shave off a few bytes with 'static const'

* Add a separate header

* Regenerate unicodedata.c

* Cleanups
@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants