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

feat(core): for KMW/wasm, devolve regex/normalization back to JS 🙀 #9467

Closed
srl295 opened this issue Aug 15, 2023 · 4 comments · Fixed by #11778
Closed

feat(core): for KMW/wasm, devolve regex/normalization back to JS 🙀 #9467

srl295 opened this issue Aug 15, 2023 · 4 comments · Fixed by #11778
Assignees
@srl295
Labels
core/ Keyman Core epic-ldml feat m:normalization
Milestone
A18S4

Comments

@srl295
Copy link
Member

srl295 commented Aug 15, 2023 •
edited
Loading

Note: make sure ICU isn't included in the wasm payload.
@srl295 srl295 added this to the 18.0 milestone Aug 15, 2023
@srl295 srl295 self-assigned this Aug 15, 2023
@srl295 srl295 mentioned this issue Aug 15, 2023
Merged
@mcdurdin mcdurdin added the core/ Keyman Core label Aug 16, 2023
@srl295
Copy link
Member Author

srl295 commented Mar 8, 2024

@srl295
Copy link
Member Author

srl295 commented Mar 8, 2024

  • could use icu4x's UnicodeSet support

@mcdurdin mcdurdin modified the milestones: 18.0, A18S2 Apr 29, 2024
@srl295
Copy link
Member Author

srl295 commented May 23, 2024

  • Probably still link icu4c for unit tests.

srl295 added a commit that referenced this issue May 23, 2024
@srl295
chore(core): refactor to move most normalization calls into util_norm…
d8b5c25 
…alize.cpp

- fold ldml_utils.hpp into core_icu.h
- refactor to use km::core::util::normalize_nfd() functions where simple to do so

For #9467
@srl295 srl295 mentioned this issue May 23, 2024
Merged
srl295 added a commit that referenced this issue May 23, 2024
@srl295
feat(core): change normalize_nfd() to use JS native call instead of ICU
43cd828 
- does not hit all ICU usage yet

#9467
@srl295 srl295 mentioned this issue May 23, 2024
Merged
srl295 added a commit that referenced this issue May 24, 2024
@srl295
feat(core): add a KMN_NO_ICU internal switch to start being able to t…
465c4bf 
…urn off ICU

- always set to 0 for now (keep ICU around)
- set KMN_IN_LDML_TESTS in tests to keep ICU there for test and comparison
- add core_icu.cpp and put some utils there.

#9467
srl295 added a commit that referenced this issue May 24, 2024
@srl295
feat(core): move more normalization logic into JS
8f0dbf6 
- add a normalize_nfd() which takes a single codepoint
- temporarily keep ICU in actions_normalize.cpp and ldml_transforms.cpp
- expand wasm opts in unit tests

#9467
srl295 added a commit that referenced this issue May 24, 2024
@srl295
feat(core): move more normalization logic into JS
5d42024 
- major redo of actions_normalize - into UTF-32 and not using ICU directly
- add some utilities: u32len, u32dup, context_items_from_utf32

#9467
@srl295 srl295 mentioned this issue May 24, 2024
Merged
1 task
@mcdurdin mcdurdin modified the milestones: A18S2, A18S3 May 24, 2024
srl295 added a commit that referenced this issue Jun 4, 2024
@srl295
feat(core): build improvements for --update-unicode option
fdb2e95 
Fixes: #9467
srl295 added a commit that referenced this issue Jun 5, 2024
@srl295
feat(core): generator in core/src for util_normalize_table.h
f4eca49 
- temporary header file generated by wasm during build
- built using icu
- test_unicode verifies the contents and synchronization with running ICU.
- this is used by util_normalize to provide normalization properties under wasm without needing to include ICU.

Fixes: #9467
srl295 added a commit that referenced this issue Jun 5, 2024
@srl295
feat(core): generator in core/src for util_normalize_table.h
f156a72 
- temporary header file generated by wasm during build
- built using icu
- test_unicode verifies the contents and synchronization with running ICU.
- this is used by util_normalize to provide normalization properties under wasm without needing to include ICU.

Fixes: #9467
srl295 added a commit that referenced this issue Jun 7, 2024
@srl295
test(core): detailed unit test for util_regex
c4b01f7 
Fixes: #9467
srl295 added a commit that referenced this issue Jun 7, 2024
@srl295
feat(core): devolve regex to JS
acd3cc5 
- use JS regex code
- almost there - some perhaps escaping differences

Fixes: #9467
@mcdurdin mcdurdin modified the milestones: A18S3, A18S4 Jun 7, 2024
srl295 added a commit that referenced this issue Jun 13, 2024
@srl295 @rc-swag
chore(core): update comments and remove a raw numeric literal
263d0e3 
- per review comments

Fixes: #9467

Co-authored-by: rc-swag <58423624+rc-swag@users.noreply.github.com>
srl295 added a commit that referenced this issue Jun 13, 2024
@srl295
feat(core): devolve regex to js for wasm
1a61a58 
- assert was wrong!

Fixes: #9467
@srl295 srl295 mentioned this issue Jun 13, 2024
Merged
srl295 added a commit that referenced this issue Jun 13, 2024
@srl295
feat(core): remove ICU from core under wasm
a744afc 
Fixes: #9467
@srl295 srl295 mentioned this issue Jun 13, 2024
Merged
@srl295 srl295 closed this as completed in e967386 Jun 13, 2024
@srl295 srl295 reopened this Jun 13, 2024
@srl295
Copy link
Member Author

srl295 commented Jun 13, 2024

will be completed by #11778

srl295 added a commit that referenced this issue Jun 14, 2024
@srl295
feat(core): update util_regex per review comments
ca34550 
- remove some TODOs that were obsolete
- copy/clarify/expand comments between the ICU and non-ICU sides

Fixes: #9467
@srl295 srl295 linked a pull request Jun 14, 2024 that will close this issue
feat(core): remove ICU from core under wasm 🙀 #11778
Merged
@srl295 srl295 closed this as completed in 55a025c Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srl295 srl295

Labels
core/ Keyman Core epic-ldml feat m:normalization
Projects
None yet
Milestone
A18S4
Development

Successfully merging a pull request may close this issue.

feat(core): remove ICU from core under wasm 🙀 keymanapp/keyman
2 participants
@srl295 @mcdurdin