feat(developer): use ABNF to validate LDML transform #13236
Conversation
srl295
requested review from
mcdurdin,
jahorton,
darcywong00,
SabineSIL,
markcsinclair and
rc-swag
as code owners
keymanapp-test-bot
bot
added
the
user-test-missing
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
github-actions
bot
added
developer/
common/
developer/compilers/
common/resources/
keymanapp-test-bot
bot
removed
the
user-test-missing
- update fetch-latest-cldr.sh - so next time, abnf will get updated. - note, this is abnf from 47 but we’re not doing the full 47 import yet because it's not released. Fixes: feat(developer): utilize CLDR ABNF rules to validate transforms #13175
- convert each .abnf to a .pegjs file Fixes: #13175
- just convert over all abnf (as we do with copying imports) - hard coded CLDR version number for now Fixes: #13175
srl295
force-pushed
the
feat/developer/13175-abnf-on-compileepic-ldml
branch
from
ebc7bb8 to
2e842bc
Compare
github-actions
bot
added
common/web/
core/
- fix build output Fixes: #13175
|
Okay, I think I've fixed the build issues, should be ready for review. |
- late breaking update from CLDR, update with in-progress fixes to escaping chars slated for v47 Fixes: #13175
|
pulled in latest ABNF which supports |
github-actions
bot
added
common/web/
core/
|
|
||
| <transforms type="simple"> | ||
| <transformGroup> | ||
| <transform from="(abc){1,}" /> |
There was a problem hiding this comment.
Can you add a comment on what you are testing here?
| @@ -27,24 +27,44 @@ builder_describe "Keyman kmc Keyboard Compiler module" \ | |||
| "--dry-run,-n don't actually publish, just dry run" | |||
|
|
|||
| builder_describe_outputs \ | |||
| configure /node_modules \ | |||
| configure /developer/src/kmc-ldml/src/util/abnf/46/transform-from-required.js \ | |||
There was a problem hiding this comment.
Did we originally have /node_modules so configure step will run if it's not present?
There was a problem hiding this comment.
can there be multiple lines with the same target?
anyway- without this line, configure didn't re-run because node_modules was already present.
There was a problem hiding this comment.
The /node_modules is the default for node projects if there are no local targets -- ensures npm ci is called if it is missing. If there is a local target (in this case transform-from-required.js) then you need to use that instead.
At this point, we only support a single output per target (we could support more, but we start to drift into being a full build system with out-of-date checks, and that's not really our goal with build.sh -- that is the responsibility of other build systems: gradle, meson, xcodebuild, delphi, etc).
The intent of this is to avoid unnecessary configure and build steps for dependencies when the build is already in a good state. If the build is not in a good state, you can:
git clean -fdx "$KEYMAN_ROOT"; ./build.sh(the big hammer but also deletes local config yeuch), or$dep/build.sh cleanclean action./build.sh --force-deps(ignore outputs and reconfigure and rebuild each dep)
See also discussion at builder_describe_outputs and build script parameters - dependencies section
(Also, for best results, generally the referenced output should be the last thing generated in the action -- so if the action fails half way it will re-run automatically.)
Co-authored-by: Marc Durdin <[email protected]>
github-actions
bot
added
common/web/
core/
github-actions
bot
added
common/web/
core/
|
Changes in this pull request will be available for download in Keyman version 19.0.8-alpha |
Fixes: #13175
@keymanapp-test-bot skip