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(developer): ldml - support local imports #10649

Closed
mcdurdin opened this issue Feb 6, 2024 · 5 comments · Fixed by #12750
Closed

feat(developer): ldml - support local imports #10649

mcdurdin opened this issue Feb 6, 2024 · 5 comments · Fixed by #12750
Assignees
@srl295
Labels
blocked Blocked by another issue or PR developer/ epic-ldml feat
Milestone
A18S16

Comments

@mcdurdin
Copy link
Member

mcdurdin commented Feb 6, 2024

only base=cldr is currently supported but spec allows local relative/absolute imports.

keyman/common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts

Lines 187 to 190 in 5dbda00

if (base !== constants.cldr_import_base) {
this.callbacks.reportMessage(CommonTypesMessages.Error_ImportInvalidBase({base, path, subtag}));
return false;
}

Reported by @sovichet
@mcdurdin mcdurdin added this to the 18.0 milestone Feb 6, 2024
@srl295
Copy link
Member

srl295 commented Feb 6, 2024

  • define project-relative imports? kmc-relative?
  • command line option to add additional -I include paths?

@mcdurdin mcdurdin modified the milestones: 18.0, A18S4 Apr 29, 2024
@darcywong00 darcywong00 modified the milestones: A18S4, A18S5 Jun 21, 2024
@darcywong00 darcywong00 modified the milestones: A18S5, A18S6 Jul 8, 2024
@darcywong00 darcywong00 modified the milestones: A18S6, A18S7 Jul 19, 2024
@darcywong00 darcywong00 modified the milestones: A18S7, A18S8 Aug 2, 2024
@srl295
Copy link
Member

srl295 commented Aug 6, 2024

blocked by #11236

@darcywong00 darcywong00 modified the milestones: A18S8, A18S10 Aug 6, 2024
@darcywong00 darcywong00 added the blocked Blocked by another issue or PR label Aug 6, 2024
@darcywong00 darcywong00 modified the milestones: A18S10, A18S11 Sep 14, 2024
@darcywong00 darcywong00 modified the milestones: A18S11, A18S12 Sep 28, 2024
@darcywong00 darcywong00 modified the milestones: A18S12, A18S13 Oct 11, 2024
@darcywong00 darcywong00 modified the milestones: A18S13, A18S14 Oct 26, 2024
@darcywong00 darcywong00 removed this from the A18S14 milestone Nov 9, 2024
@darcywong00 darcywong00 added this to the A18S15 milestone Nov 9, 2024
@darcywong00 darcywong00 modified the milestones: A18S15, A18S16 Nov 24, 2024
@mcdurdin mcdurdin moved this to Todo in Keyman Nov 28, 2024
@srl295
Copy link
Member

srl295 commented Nov 28, 2024

for now, just XML-file-relative

@srl295
Copy link
Member

srl295 commented Nov 28, 2024

definite crowd pleaser

@srl295
Copy link
Member

srl295 commented Nov 29, 2024 •
edited
Loading

Ugh- there's a CLDR DTD bug here. https://unicode-org.atlassian.net/browse/CLDR-18138 - base="cldr" is implied.

I think for now I will need to patch the schema.
Will affect #12420 #11307

Update (and updated the upstream issue): Looks like <import base="" path="somePath.xml" /> is sufficient to work around the DTD issue. I will support that as a workaround and make a separate issue to work around the CLDR issue.

@srl295 srl295 mentioned this issue Nov 29, 2024
Open
srl295 added a commit that referenced this issue Nov 29, 2024
@srl295
feat(developer): support local imports
01a47c3 
- added a new reader callback option, localImportsPaths
- due to the CLDR issue #12749 use base=""
- add tests
- some bugfixes in import messages
- add an ImportStatus section to determine if something is a local import

Fixes: #10649
srl295 added a commit that referenced this issue Nov 29, 2024
@srl295
feat(developer): support local imports in kmc
e99f6ac 
- basic changes to support local imports

Fixes: #10649
srl295 added a commit that referenced this issue Nov 29, 2024
@srl295
feat(developer): test local imports in kmc-ldml
01fed4b 
- also improve callback visibility when XML fails to load

Fixes: #10649
srl295 added a commit that referenced this issue Nov 29, 2024
@srl295
feat(core): test local imports in kmc
b1b6587 
Fixes: #10649
@srl295 srl295 mentioned this issue Nov 29, 2024
Merged
@srl295 srl295 linked a pull request Nov 30, 2024 that will close this issue
feat(developer,core): local imports 🙀 #12750
Merged
srl295 added a commit that referenced this issue Dec 2, 2024
@srl295
feat(developer): local imports directory fixes
f4f0355 
- use path.join instead of path.resolve
- rename importsPath to cldrImportsPath
- remove k_015_importlocal from core, as it duplicated tests in common and developer

Fixes: #10649
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srl295 srl295

Labels
blocked Blocked by another issue or PR developer/ epic-ldml feat
Projects
Keyman
Archived in project
Milestone
A18S16
Development

Successfully merging a pull request may close this issue.

feat(developer,core): local imports 🙀 keymanapp/keyman
3 participants
@srl295 @mcdurdin @darcywong00