Skip to content

feat(core): add sample file, update union #5015 🙀#7071

Merged
srl295 merged 5 commits intofeature-ldmlfrom
feat/core/5015-process0
Aug 16, 2022
Merged

feat(core): add sample file, update union #5015 🙀#7071
srl295 merged 5 commits intofeature-ldmlfrom
feat/core/5015-process0

Conversation

@srl295
Copy link
Member

@srl295 srl295 commented Aug 13, 2022 •
edited
Loading

  • add tiny sample kmx
  • update union
  • add DTD

#5015

@keymanapp-test-bot skip

@srl295 srl295 self-assigned this Aug 13, 2022
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Aug 13, 2022
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Aug 13, 2022 •
edited
Loading

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

mcdurdin
mcdurdin approved these changes Aug 15, 2022
View reviewed changes
core/tests/unit/ldml/001_tiny.xml Outdated

<keys>
<key id="hmaqtua" to="ħ" />
<key id="say" to="ថា" />
Copy link
Member

@mcdurdin mcdurdin Aug 15, 2022

Choose a reason for hiding this comment

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

Technically, that rather than say but very good 😆

core/tests/unit/ldml/ldmlKeyboard.dtd
@@ -0,0 +1,192 @@
<!--
Copy link
Member

@mcdurdin mcdurdin Aug 15, 2022

Choose a reason for hiding this comment

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

I wonder if we can put this in resources/standards-data/ldml-keyboard along with a README.md detailing where it is sourced from and update strategy?

See for example https://github.com/keymanapp/keyman/blob/d4b8094b137ef2a1aac1eb60a6d0a01658bb525f/resources/standards-data/langtags/readme.md and its corresponding data file.

I realise that this may be problematic/annoying for DOCTYPE decls so feel free to push back on this.

Copy link
Member Author

@srl295 srl295 Aug 15, 2022

Choose a reason for hiding this comment

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

I think that's reasonable. Will need a long term strategy for this anyway. Should be versioned.

Copy link
Member Author

@srl295 srl295 Aug 15, 2022

Choose a reason for hiding this comment

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

https://github.com/keymanapp/keyman/blob/master/resources/standards-data/unicode-character-database/readme.md this one is not so descriptive :)

Copy link
Member

@mcdurdin mcdurdin Aug 16, 2022

Choose a reason for hiding this comment

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

Personally, I love the simplicity of that readme.

@srl295 srl295 requested a review from darcywong00 as a code owner August 15, 2022 16:53
@srl295 srl295 changed the title feat(core): add sample file, update union 🙀 feat(core): add sample file, update union #5015 🙀 Aug 15, 2022
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Aug 15, 2022
@github-actions github-actions bot added the common/resources/ Build infrastructure label Aug 15, 2022
@srl295
Copy link
Member Author

srl295 commented Aug 15, 2022

00041d6 belongs in PR #7066 instead

@srl295 srl295 mentioned this pull request Aug 15, 2022
Merged
@srl295 srl295 force-pushed the feat/core/5015-process0 branch from 0a2167d to 4f141eb Compare August 15, 2022 18:39
@srl295
Copy link
Member Author

srl295 commented Aug 15, 2022

👉 Note: rebased to pickup upstream PR changes

mcdurdin
mcdurdin reviewed Aug 16, 2022
View reviewed changes
Base automatically changed from feat/core/7046-processflag to feature-ldml August 16, 2022 13:10
srl295 and others added 5 commits August 16, 2022 10:24
@srl295
test(core): add sample tiny file
29d4cc0 
- also snapshot dtd
@srl295
feat(core): update COMP_KEYBOARD_EX #5015
189124b 
- uberstruct
@srl295 @mcdurdin
Apply suggestions from code review
1ccc43d 
Co-authored-by: Marc Durdin <marc@durdin.net>
@srl295
feat(core): Sample LDML data updates, kmx_file #5015
9e44a7f 
Review comments:
- moved KMXPLUSINFO into separate struct
- created resource/standards-data/ldml-keyboards for CLDR data
- corrected Khmer err
@srl295 @mcdurdin
Update core/src/kmx/kmx_file.h
5e203f8 
Co-authored-by: Marc Durdin <marc@durdin.net>
@srl295 srl295 force-pushed the feat/core/5015-process0 branch from 7fa78a1 to 5e203f8 Compare August 16, 2022 15:25
@srl295
Copy link
Member Author

srl295 commented Aug 16, 2022

just a rebase onto the feature branch

@srl295
Copy link
Member Author

srl295 commented Aug 16, 2022

What is the modelling layer error?

LMLayer using the trie model Regression tests.Chrome 100.0.4896.127 (Windows 10):

@mcdurdin
Copy link
Member

mcdurdin commented Aug 16, 2022

What is the modelling layer error?

Lexical Models. You can ignore it. Being addressed by #7037.

@srl295 srl295 merged commit 3c7279d into feature-ldml Aug 16, 2022
@srl295 srl295 deleted the feat/core/5015-process0 branch August 16, 2022 16:13
@mcdurdin mcdurdin added this to the Support milestone Aug 18, 2023
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

@jahorton jahorton Awaiting requested review from jahorton

@darcywong00 darcywong00 Awaiting requested review from darcywong00

Assignees

@srl295 srl295

Labels

common/resources/ Build infrastructure common/ developer/ epic-ldml feat

Projects

None yet

Milestone

Support

Development

Successfully merging this pull request may close these issues.

2 participants

@srl295 @mcdurdin