Skip to content

feat(core): unescape u 🙀#10356

Merged
srl295 merged 13 commits intomasterfrom
feat/core/10319-escaping-you-epic-ldml
Jan 18, 2024
Merged

feat(core): unescape u 🙀#10356
srl295 merged 13 commits intomasterfrom
feat/core/10319-escaping-you-epic-ldml

Conversation

@srl295
Copy link
Copy Markdown
Member

@srl295 srl295 commented Jan 11, 2024 •
edited
Loading

#10319

@keymanapp-test-bot skip

@srl295 srl295 self-assigned this Jan 11, 2024
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jan 11, 2024
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Jan 11, 2024 •
edited
Loading

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

@srl295 srl295 changed the title feat(core): unscape u 🙀 feat(core): unescape u 🙀 Jan 11, 2024
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S30 milestone Jan 11, 2024
@mcdurdin mcdurdin linked an issue Jan 11, 2024 that may be closed by this pull request
feat(developer): escaping should be \u{…} not \uXXXX 🙀 #10319
Closed
6 tasks
@srl295 srl295 force-pushed the feat/core/10319-escaping-you-epic-ldml branch from 60bd536 to 39cea9a Compare January 11, 2024 17:04
@github-actions github-actions bot added core/ Keyman Core and removed core/ Keyman Core labels Jan 11, 2024
@srl295
feat(core): unscape u 🙀
c49dab2 
- update unescaping from \u{…} to \uXXXX

#10319
@srl295 srl295 force-pushed the feat/core/10319-escaping-you-epic-ldml branch from 39cea9a to c49dab2 Compare January 11, 2024 17:43
@github-actions github-actions bot added core/ Keyman Core and removed core/ Keyman Core labels Jan 11, 2024
@srl295
feat(developer,common): all unescaping as \uXXXX 🙀
d662f67 
- update unescaping from \u{…} to \uXXXX in regex
- re-unescape in the string analyzr (empty-compiler.ts) so that we catch PUA etc.
- fix some XML files that had bad DTDs
- common; updates to string util api

#10319
@github-actions github-actions bot added core/ Keyman Core and removed core/ Keyman Core labels Jan 11, 2024
@srl295 srl295 marked this pull request as ready for review January 11, 2024 19:07
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jan 11, 2024
Base automatically changed from chore/core/10320-refactor-epic-ldml to master January 11, 2024 21:38
@github-actions github-actions bot removed the core/ Keyman Core label Jan 11, 2024
@github-actions github-actions bot added the core/ Keyman Core label Jan 11, 2024
@srl295 srl295 marked this pull request as draft January 11, 2024 21:55
@srl295
Copy link
Copy Markdown
Member Author

srl295 commented Jan 11, 2024

issues repro'ed on linux locally and also wasm (butonly under linux, from mac wasm worked)

@srl295
feat(developer): 32 bit escapades 🙀
04bce67 
- match sentinels as \\u…
- support \Uxxxxxxxx for >0xffff
- more escaping stuff

#10319
@github-actions github-actions bot added core/ Keyman Core and removed core/ Keyman Core labels Jan 13, 2024
srl295 added 4 commits January 13, 2024 12:55
@srl295
feat(developer): 32 bit escapades 🙀
c5d0e05 
- escaping working, matching not yet

#10319
@srl295
feat(developer): 32 bit escapades 🙀
e0a2ce2 
- basic.txt fixup

#10319
@srl295
feat(core): 32 bit escapades 🙀
ab67061 
- improved test code and marker parsing

#10319
@srl295
feat(developer): don't re-escape non-syntax chars 🙀
a5ae4e3 
- \u{0300} needs to turn back into an actual char, so normalization gets an opportunity to run.

#10319
@srl295 srl295 marked this pull request as ready for review January 15, 2024 22:45
@srl295 srl295 requested a review from rc-swag as a code owner January 15, 2024 22:45
@srl295 srl295 removed a link to an issue Jan 15, 2024
feat(developer): escaping should be \u{…} not \uXXXX 🙀 #10319
Closed
6 tasks
@srl295 srl295 linked an issue Jan 15, 2024 that may be closed by this pull request
feat(developer): escaping should be \u{…} not \uXXXX 🙀 #10319
Closed
6 tasks
mcdurdin
mcdurdin approved these changes Jan 17, 2024
View reviewed changes
common/web/types/src/util/util.ts Outdated
}

/** chars that must be escaped: syntax, C0 + C1 controls */
const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?.^$*-]$/;
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin Jan 17, 2024

Choose a reason for hiding this comment

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

(, ), and / also need escaping, right?

srl295 added 4 commits January 17, 2024 18:05
@srl295
feat(core): ldml updates per code review 🙀
763bc96 
- remove stray change to kmx_processevent.cpp
- fix escaping of syntax chars
- fix comment per review

For: #10319
@srl295
feat(core): ldml updates per code review 🙀
d08e705 
- don't try to remove irregular sentinel sequences
- update test

For: #10319
@srl295
feat(core): ldml fix test case and parser 🙀
b6a6b7e 
For: #10319
@srl295
Merge branch 'master' into feat/core/10319-escaping-you-epic-ldml
9e828b1
@srl295 srl295 merged commit 215bbfc into master Jan 18, 2024
@srl295 srl295 deleted the feat/core/10319-escaping-you-epic-ldml branch January 18, 2024 16:37
@keyman-server
Copy link
Copy Markdown
Collaborator

keyman-server commented Jan 18, 2024

Changes in this pull request will be available for download in Keyman version 17.0.247-alpha

srl295 added a commit that referenced this pull request Mar 25, 2024
@srl295
fix(core): fix 2 marker cases in ldml_event_state::emit_difference 🙀
18c6ecf 
- handle the case where old and new context strings are the same (i.e. no work to do).
- fix pointer arithmetic error in #10356 - the special case where one marker is being replaced by another. The named regression test didn't actually hit this case.

Fixes: bug(core): 'string too long' #11057
@srl295 srl295 mentioned this pull request Mar 25, 2024
Merged
srl295 added a commit that referenced this pull request Apr 4, 2024
@srl295
fix(core): fix 2 marker cases in ldml_event_state::emit_difference 🙀
fcf1ea5 
- handle the case where old and new context strings are the same (i.e. no work to do).
- fix pointer arithmetic error in #10356 - the special case where one marker is being replaced by another. The named regression test didn't actually hit this case.

Fixes: bug(core): 'string too long' #11057
mcdurdin pushed a commit that referenced this pull request Jun 25, 2024
@srl295 @mcdurdin
fix(core): fix 2 marker cases in ldml_event_state::emit_difference 🙀
686683f 
- handle the case where old and new context strings are the same (i.e. no work to do).
- fix pointer arithmetic error in #10356 - the special case where one marker is being replaced by another. The named regression test didn't actually hit this case.

Fixes: bug(core): 'string too long' #11057
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

@darcywong00 darcywong00 Awaiting requested review from darcywong00

@jahorton jahorton Awaiting requested review from jahorton

@rc-swag rc-swag Awaiting requested review from rc-swag

Assignees

@srl295 srl295

Labels

common/web/ common/ core/ Keyman Core developer/compilers/ developer/ feat

Projects

None yet

Milestone

A17S30

Development

Successfully merging this pull request may close these issues.

feat(developer): escaping should be \u{…} not \uXXXX 🙀

3 participants

@srl295 @keyman-server @mcdurdin