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.

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

[Developer] Lexical model compiler CLI #1983

Merged
merged 15 commits into from
Aug 23, 2019
Merged

Conversation

eddieantonio
Copy link
Contributor

This adds kmlmc — the Keyman lexical model compiler. It's a command line utility that takes in a model.ts file in the format described in #1973 and produces the compiled JavaScript model.

kmlmc model.ts -o example.qaa.wordlist.js

I can now practically describe how to build models in the tutorial on help.keyman.com! And it makes creating example lexical models a whole lot easier!

Note: this is a fixed version of #1982. Please do not touch that PR — bad things happened there :(

@keyman-server
Copy link
Collaborator

OSK renders for Windows

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

I'm not sure the best way to resolve versioning. One of the challenges at present is that this still requires the source repo to build a model, which is something we want to avoid. We may be best off cloning the developer/js folder into the Keyman Developer installer and providing a standalone version with the kmcomp command line zip when we build those packages. What do you think?

@@ -1,6 +1,6 @@
{
"name": "@keymanapp/developer-lexical-model-compiler",
"version": "1.0.0",
"version": "12.0.0-beta+20190819",
Copy link
Member

Choose a reason for hiding this comment

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

Gosh we kinda need to have this automated ...

@eddieantonio
Copy link
Contributor Author

One of the challenges at present is that this still requires the source repo to build a model, which is something we want to avoid.

Yeah. We'll want to somehow disentangle this code from, e.g., common/predictive-text.

We may be best off cloning the developer/js folder into the Keyman Developer installer and providing a standalone version with the kmcomp command line zip when we build those packages. What do you think?

I'm not sure I understand. Is that a different repo?

I was thinking of the following for building @keymanapp/developer-lexical-models-compiler:

  1. clone this repo
  2. run ./build.sh in this directory. This issue npm install and tsc. This generates dist/
  3. run npm pack to create a tarball of dist/ and its dependencies.
  4. bundle the tarball/zip with Keyman Developer as well as standalone on https://keyman.com/developer/download

@mcdurdin
Copy link
Member

One of the challenges at present is that this still requires the source repo to build a model, which is something we want to avoid.

Yeah. We'll want to somehow disentangle this code from, e.g., common/predictive-text.

We may be best off cloning the developer/js folder into the Keyman Developer installer and providing a standalone version with the kmcomp command line zip when we build those packages. What do you think?

I'm not sure I understand. Is that a different repo?

kmcomp-a.b.c.d.zip is generated from the Keyman source, and is a binary distribution of just the command line compiler tools. The Keyman Developer installer includes kmcomp as well. Currently kmcomp-a.b.c.d.zip contains kmcomp.exe and other dependencies.

I was thinking of the following for building @keymanapp/developer-lexical-models-compiler:

  1. clone this repo
  2. run ./build.sh in this directory. This issue npm install and tsc. This generates dist/
  3. run npm pack to create a tarball of dist/ and its dependencies.
  4. bundle the tarball/zip with Keyman Developer as well as standalone on https://keyman.com/developer/download

The main problem with this is that it doesn't resolve the node dependency -- and still requires npm etc etc on end user machines. I have a prototype bundling of node that I am testing now with Keyman Developer. It's a bit more roll-your-own but I just want to avoid bundling unnecessary files. As soon as I have it working, I'll put it up in a dependent PR so you can comment :)

@eddieantonio
Copy link
Contributor Author

eddieantonio commented Aug 21, 2019

The main problem with this is that it doesn't resolve the node dependency -- and still requires npm etc etc on end user machines. I have a prototype bundling of node that I am testing now with Keyman Developer. It's a bit more roll-your-own but I just want to avoid bundling unnecessary files. As soon as I have it working, I'll put it up in a dependent PR so you can comment :)

Ah, okay! So I was mistaken: npm pack does NOT bundle node_modules but we can add that as well in the build step using npm-bundle. Once that is run, we have a single archive containing all the things node (and only node) needs to run our code:

package/dist/cli.js
package/dist/index.js
package/dist/lexical-model-compiler/build-trie.js
package/dist/lexical-model-compiler/lexical-model.js
package/dist/lexical-model-compiler/model-info-file.js
package/dist/lexical-model-compiler/stub.js
package/dist/package-compiler/kmp-compiler.js
package/dist/package-compiler/kmp-json-file.js
package/dist/package-compiler/kps-file.js
package/package.json
package/node_modules/...

@eddieantonio
Copy link
Contributor Author

Hey @mcdurdin! What needs to be done to have this PR approved?

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

This is nearly ready I think, just the one change to how the compiler references files...

I have a dependent PR of how I propose to deploy node with Keyman Developer in PR #1986. This has been tested on a clean VM; see the PR for more details.

@@ -0,0 +1,5 @@
const source: LexicalModelSource = {
format: 'trie-1.0',
sources: ['tests/fixtures/example.qaa.trivial/wordlist.tsv'],
Copy link
Member

Choose a reason for hiding this comment

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

One last thing: this needs to be a path relative to model.ts, not to where the build is run from.

@eddieantonio
Copy link
Contributor Author

I merged beta into this branch, and now the compileModel() utility function compiles the model.ts file relative to its containing directory, rather than the current working directory!

@eddieantonio eddieantonio merged commit 728e9e5 into beta Aug 23, 2019
@jahorton jahorton deleted the developer-compiler-cli branch September 4, 2019 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants