feat(developer): VSCode plugin for building 🗼 #12757

srl295 · 2024-12-02T22:16:31Z

initial checkin
README
build.sh

Fixes: #12756

@keymanapp-test-bot skip

keymanapp-test-bot · 2024-12-02T22:16:53Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Developer
Keyboards
- Test Keyboards

mcdurdin · 2024-12-03T00:22:05Z

Should this be based on epic/ldml-editor?

srl295 · 2024-12-03T15:26:06Z

Should this be based on epic/ldml-editor?

Oops, yes, thanks!

developer/src/keyman-vscode-plugin/build.sh

developer/src/keyman-vscode-plugin/package.json

developer/src/keyman-vscode-plugin/src/test/extension.test.ts

developer/src/keyman-vscode-plugin/tsconfig.json

developer/src/keyman-vscode-plugin/.gitignore

mcdurdin

I feel like this PR is a good draft. But it needs a lot of work before it is ready to merge into the epic branch. Code that lands in an epic needs to be production ready -- with neat attachment points for follow-on PRs. We don't want commented-out code, test code, or code that will need re-review in general to land in the epic branch.

I've given some initial feedback. Hope this is not too depressing! I am excited for where this is going.

Finally, we should back-burner this work and focus on the LDML keyboard editor work as a priority. I think it might be sensible to refactor the .kpj handling in kmc into a kmc-project module and build on that. I can certainly take the changes in this PR into account if I take that on (or you can).

developer/src/build.sh

mcdurdin · 2024-12-04T03:30:43Z

developer/src/keyman-vscode-plugin/README.md

+This package is a standalone [VSCode](https://code.visualstudio.com) plugin which offers:
+
+- a Build Task for building .kpj files into a package
+- when building the .kpj file, all .kmn and .xml (LDML keyboard) files will be compiled as well


needs to also build .kps and .model.ts.

build .kps (actually I think it does, it just uses the .kpj to locate them?)

build .model.ts

developer/src/keyman-vscode-plugin/README.md

developer/src/keyman-vscode-plugin/build.sh

mcdurdin · 2024-12-05T06:59:48Z

developer/src/keyman-vscode-plugin/src/kpjBuild.mts

+        };
+        const compiler = new kmcLdml.LdmlKeyboardCompiler();
+        if (!await compiler.init(callbacks, ldmlCompilerOptions)) {
+            msg(`Compiler failed init\r\n`);


I think we should avoid embedding NL in the messages -- this should be in the message function. Also, we are trying to keep all messages in separate files for future localization

update msg to have nl by default.

mcdurdin · 2024-12-05T07:00:28Z

developer/src/keyman-vscode-plugin/src/kpjBuild.mts

+ * @param msg callback for writing messages
+ * @returns accept on OK, otherwise throws
+ */
+export async function buildProject(workspaceRoot: string,


This function is very long and should be broken into smaller chunks

mcdurdin · 2024-12-05T07:00:59Z

developer/src/keyman-vscode-plugin/src/kpjBuild.mts

+        if (didCompilePkg) {
+            throw Error(`Error: two packages were encountered.`);
+        }


Why do you error on multiple packages?

i think this was a mistake.

mcdurdin · 2024-12-05T07:01:32Z

developer/src/keyman-vscode-plugin/src/kpjBuild.mts

+    if (!didCompileSrc) {
+        throw Error(`Error: no source files were compiled.`);
+    }


It is legitimate to have a project with only a .kps and no .kmn or .xml

remove error

mcdurdin · 2024-12-05T07:03:08Z

package-lock.json

7500 additional lines in package-lock.json is a little concerning in terms of dependency creep; I know that's almost all in devDependencies but we need to be monitoring

srl295 · 2024-12-05T19:22:29Z

I feel like this PR is a good draft. But it needs a lot of work before it is ready to merge into the epic branch. Code that lands in an epic needs to be production ready -- with neat attachment points for follow-on PRs. We don't want commented-out code, test code, or code that will need re-review in general to land in the epic branch.

I've given some initial feedback. Hope this is not too depressing! I am excited for where this is going.

Finally, we should back-burner this work and focus on the LDML keyboard editor work as a priority. I think it might be sensible to refactor the .kpj handling in kmc into a kmc-project module and build on that. I can certainly take the changes in this PR into account if I take that on (or you can).

I got the rename in and will move this back to draft for now.

100% on quality.

srl295 · 2024-12-05T19:24:10Z

I feel like this PR is a good draft. But it needs a lot of work before it is ready to merge into the epic branch. Code that lands in an epic needs to be production ready -- with neat attachment points for follow-on PRs. We don't want commented-out code, test code, or code that will need re-review in general to land in the epic branch.
I've given some initial feedback. Hope this is not too depressing! I am excited for where this is going.
Finally, we should back-burner this work and focus on the LDML keyboard editor work as a priority. I think it might be sensible to refactor the .kpj handling in kmc into a kmc-project module and build on that. I can certainly take the changes in this PR into account if I take that on (or you can).

I got the rename in and will move this back to draft for now.

100% on quality.

erm. The one thing about tabling this is, it's also the substrate for the visual editor.

But, I think I can make a draft PR off this draft PR. And can work on the rest separately.

- basic build of .kpj, .kps, .kmn, .xml Fixes: #12756 Co-authored-by: Marc Durdin <[email protected]>

srl295 · 2024-12-10T17:18:10Z

OK. I'm going to let this one sit for a bit. It'll have some conflicts to work through.

srl295 self-assigned this Dec 2, 2024

keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Dec 2, 2024

github-actions bot added developer/ feat labels Dec 2, 2024

keymanapp-test-bot bot added this to the A18S16 milestone Dec 2, 2024

mcdurdin changed the base branch from master to epic/ldml-editor December 3, 2024 00:51

keymanapp-test-bot bot added the epic-ldml-editor label Dec 3, 2024

srl295 force-pushed the feat/developer/12756-vscode-build branch 2 times, most recently from b93b28f to b65ba85 Compare December 3, 2024 18:22

keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Dec 3, 2024

srl295 commented Dec 3, 2024

View reviewed changes

srl295 force-pushed the feat/developer/12756-vscode-build branch 3 times, most recently from 5c3984f to 85dbd3a Compare December 3, 2024 22:49

srl295 marked this pull request as ready for review December 3, 2024 22:49

srl295 requested review from mcdurdin, darcywong00, SabineSIL and markcsinclair as code owners December 3, 2024 22:49

srl295 commented Dec 3, 2024

View reviewed changes

developer/src/keyman-vscode-plugin/.gitignore Outdated Show resolved Hide resolved

mcdurdin self-assigned this Dec 4, 2024

mcdurdin requested changes Dec 5, 2024

View reviewed changes

srl295 marked this pull request as draft December 5, 2024 19:22

This was referenced Dec 6, 2024

feat(developer): scaffolding for VSCode plugin 🗼 #12798

Open

feat(developer): scaffolding for Keyman Developer for VSCode🗼 #12799

Merged

srl295 force-pushed the feat/developer/12756-vscode-build branch from a1e73a5 to cb65131 Compare December 6, 2024 15:58

srl295 changed the base branch from epic/ldml-editor to feat/developer/12798-vscode-ext-scaffolding-epic-ldml-editor December 6, 2024 15:59

srl295 force-pushed the feat/developer/12756-vscode-build branch from cb65131 to f890a57 Compare December 6, 2024 16:02

feat(developer): VSCode plugin for building

1aee4ed

- basic build of .kpj, .kps, .kmn, .xml Fixes: #12756 Co-authored-by: Marc Durdin <[email protected]>

srl295 force-pushed the feat/developer/12756-vscode-build branch from f890a57 to 1aee4ed Compare December 6, 2024 16:34

darcywong00 modified the milestones: A18S16, A18S17 Dec 7, 2024

Base automatically changed from feat/developer/12798-vscode-ext-scaffolding-epic-ldml-editor to epic/ldml-editor December 10, 2024 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(developer): VSCode plugin for building 🗼 #12757

feat(developer): VSCode plugin for building 🗼 #12757

srl295 commented Dec 2, 2024 •

edited

Loading

keymanapp-test-bot bot commented Dec 2, 2024 •

edited

Loading

mcdurdin commented Dec 3, 2024

srl295 commented Dec 3, 2024

mcdurdin left a comment

mcdurdin Dec 4, 2024

srl295 Dec 5, 2024

mcdurdin Dec 5, 2024

srl295 Dec 5, 2024

mcdurdin Dec 5, 2024

mcdurdin Dec 5, 2024

srl295 Dec 5, 2024

mcdurdin Dec 5, 2024

srl295 Dec 5, 2024

mcdurdin Dec 5, 2024

srl295 commented Dec 5, 2024

srl295 commented Dec 5, 2024

srl295 commented Dec 10, 2024

feat(developer): VSCode plugin for building 🗼 #12757

Are you sure you want to change the base?

feat(developer): VSCode plugin for building 🗼 #12757

Conversation

srl295 commented Dec 2, 2024 • edited Loading

keymanapp-test-bot bot commented Dec 2, 2024 • edited Loading

User Test Results

Test Artifacts

mcdurdin commented Dec 3, 2024

srl295 commented Dec 3, 2024

mcdurdin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srl295 commented Dec 5, 2024

srl295 commented Dec 5, 2024

srl295 commented Dec 10, 2024

srl295 commented Dec 2, 2024 •

edited

Loading

keymanapp-test-bot bot commented Dec 2, 2024 •

edited

Loading