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

refactor(common): Consider moving from xml2js to fast-xml-parser #12208

Closed
mcdurdin opened this issue Aug 16, 2024 · 2 comments · Fixed by #12502
Closed

refactor(common): Consider moving from xml2js to fast-xml-parser #12208

mcdurdin opened this issue Aug 16, 2024 · 2 comments · Fixed by #12502
Assignees
@srl295 @mcdurdin
Labels
common/ refactor
Milestone
A18S12

Comments

@mcdurdin
Copy link
Member

mcdurdin commented Aug 16, 2024 •
edited
Loading

Consider moving from xml2js to https://www.npmjs.com/package/fast-xml-parser?

Originally posted by @mcdurdin in #8616 (comment)

This arose again in discussions on kmc-convert

@srl295 we should discuss
@mcdurdin mcdurdin added this to the A18S9 milestone Aug 16, 2024
@srl295
Copy link
Member

srl295 commented Aug 19, 2024

FWIW I use fast-xml-parser in https://github.com/unicode-org/cldr/tree/main/docs/charts/keyboards for those charts. But this is a simplistic use that is not attempting to validate.

Our json validation is separate from the xml validation, though, so that should not be an impact.
I'm sure there will be some bumps due to differences in handling.

fast-xml-parser is at least currently maintained!

srl295 added a commit that referenced this issue Aug 30, 2024
@srl295
chore(developer): improve kpj schema
030e1b6 
Fixes: #12208
@srl295 srl295 mentioned this issue Aug 30, 2024
Closed
@darcywong00 darcywong00 modified the milestones: A18S9, A18S10 Aug 31, 2024
srl295 added a commit that referenced this issue Aug 31, 2024
@srl295
chore(developer): Drop stray package-lock.json
8779ca9 
Fixes: #12208
srl295 added a commit that referenced this issue Aug 31, 2024
@srl295
chore(developer): update kps/kmp parsing
a1f1bf7 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 1, 2024
@srl295
chore(developer): kmc-package working
877522c 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 6, 2024
@srl295
chore(developer): add invalid XML error
1cb33da 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 6, 2024
@srl295
chore(developer): unboxing fix for displays
06a5683 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 9, 2024
@srl295
chore(developer): fix for overly-trimmed attributes
171e37c 
- need to use trimValues:false and a custom tagValueProcessor to preserve spaces

Fixes: #12208
srl295 added a commit that referenced this issue Sep 10, 2024
@srl295
chore(common): handle an empty <transformGroup/> properly
c24ba0f 
- difference in fast-xml-parser and xml2js handling here
- part of the move to fast-xml-parser

Fixes: #12208
srl295 added a commit that referenced this issue Sep 12, 2024
@srl295
chore(developer): fix for kvks writing
02330e1 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 12, 2024
@srl295
chore(developer): fix for NCR parsing
6f7f268 
turns out we didn't have a test for XML NCR in keyboards. Now we do.

Fixes: #12208
@darcywong00 darcywong00 modified the milestones: A18S10, A18S11 Sep 14, 2024
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
WIP unit test for xml files
b5989da 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(common): unified xml parser scaffolding
00d7d93 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(common): unified xml parser reader test
609faf8 
Fixes: #12208
@srl295 srl295 mentioned this issue Sep 27, 2024
Merged
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(common): xml: map \r\n to \n
7da7375 
- help make the parsed form identical across platforms

Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(developer,common): use unified xml parser
3832ca1 
Subsystems changed:
- ldml keyboard reader (main and test)
- kpj
- kvks
- kmp compiler

test: made the test-xml-utils less verbose about the pathnames

Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(common): unified xml writer test
d540e4c 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
feat(developer,common): use unified xml writer
1780b1e 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
chore(developer,common): remove xml2js
266dd17 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 27, 2024
@srl295
chore(developer,common): add fast-xml-parser
37d746b 
Fixes: #12208
@darcywong00 darcywong00 modified the milestones: A18S11, A18S12 Sep 28, 2024
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
chore(developer): update per style guide
eba7079 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
feat(developer): update the unified xml parser per review
158c4a7 
- declarative syntax for options
- workaround an issue where xml2js is mutating our options objects
- improve tests

Fixes: #12208
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
chore(developer,common): remove xml2js
e309c9c 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
chore(developer,common): add fast-xml-parser
f476578 
Fixes: #12208
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
feat(developer): fast-xml-parser writer:
230de76 
- writer passes kvks tests
- two small changes: allow &quot; for XML generation, and also the same XML prologue as the actual .kvks files.

Fixes: #12208
srl295 added a commit that referenced this issue Sep 30, 2024
@srl295
feat(developer): fast-xml-parser writer:
4ed4a58 
- WIP

Fixes: #12208
srl295 added a commit that referenced this issue Oct 2, 2024
@srl295
feat(developer): fast-xml-parser reader:
55473bf 
- update for ldml

Fixes: #12208
srl295 added a commit that referenced this issue Oct 2, 2024
@srl295
feat(developer): fast-xml-parser reader:
b69f636 
- WIP preserveorder

Fixes: #12208
srl295 added a commit that referenced this issue Oct 2, 2024
@srl295
feat(developer): fast-xml-parser reader:
f85391f 
- WIP preserveorder

Fixes: #12208
@srl295 srl295 closed this as completed in 7e631c1 Oct 3, 2024
@github-project-automation github-project-automation bot moved this to Done in Keyman Oct 3, 2024
@srl295 srl295 reopened this Oct 3, 2024
@srl295
Copy link
Member

srl295 commented Oct 3, 2024

I had split the PRs so that this isn't actually fixed, yet.

@srl295 srl295 mentioned this issue Oct 3, 2024
Merged
srl295 added a commit that referenced this issue Oct 3, 2024
@srl295
feat(developer): fast-xml-parser: fix for ldml empty attributes
03a24a1 
- because of the form to="" in ldml, we need to distinguish attributes and sub-elements in the ldml xml parsing
- use an attributePrefix, and fixup the object tree afterwards

Fixes: #12208
srl295 added a commit that referenced this issue Oct 7, 2024
@srl295
feat(developer): fast-xml-parser: typo fix in xml-utils.ts
9a4c7e5 
Fixes: #12208
@srl295 srl295 closed this as completed in 3e168aa Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srl295 srl295

@mcdurdin mcdurdin

Labels
common/ refactor
Projects
Keyman
Archived in project
Milestone
A18S12
3 participants
@srl295 @mcdurdin @darcywong00