keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
372 stars 102 forks source link

bug(developer): Misc kmc crashes for 17.0 #11330

Closed darcywong00 closed 1 week ago

darcywong00 commented 2 weeks ago

While merging a follow-on branch to keymanapp/keyboards#2423

  1. While resolving merge conflicts in .kps files, sometimes I missed the following fields in tags

      <Name>...</Name>
      <Description>...</Description>

    kmc would crash

  2. In the .kvks files, if I had empty <flags></flags>, kmc would give a confusing error. (affected several kreative_ keyboards)

    error KM02908: Error encountered parsing kreative_sitelenpona_ucsur.kvks: TypeError: SchemaValidators.kvks.errorsText is not a function
  3. If there's multiple author emails listed (e.g. https://github.com/keymanapp/keyboards/blob/9a7accf0b1a4d25d41b7ff94c812cf1083a2b016/release/y/yezidi/source/yezidi.kps#L21) kmc would crash with

    "error": [
    {
      "instancePath": "/authorEmail",
      "schemaPath": "#/properties/authorEmail/format",
      "keyword": "format",
      "params": {
        "format": "email"
      },
      "message": "must match format \"email\""
    }
    ]
    }
    at KeyboardInfoCompiler.writeKeyboardInfoFile (file:///C:/src/keyboards/node_modules/@keymanapp/kmc-keyboard-info/build/src/index.js:234:19)
mcdurdin commented 2 weeks ago

Can you include:

darcywong00 commented 2 weeks ago

Can you include:

  • the files in question or a git commit hash?
  • kmc version?

Commit hash of filling in missing tags keymanapp/keyboards@b7de1cf1d9f5904e23a273cf6774cdc48c1add36

Commit hash of removing empty in .kvks files keymanapp/keyboards@63dbc5870f57558847ac0a6d4466d36a78d564d7

Commit hash of multiple author emails keymanapp/keyboards@1a9ee49e4c3665477314557e41ddf934bd35216e

kmc version

oops, I was using what was in keymanapp/keyboards#2423

package.json says it's

"@keymanapp/kmc": "17.0.194-alpha"

I think I need to update that to beta.

mcdurdin commented 2 weeks ago

I think I need to update that to beta.

Oooh yeah!

darcywong00 commented 2 weeks ago

After updating to kmc 17.0.316-beta:

1.

<Name>...</Name>
<Description>...</Description>

Missing these elements still causes kmc to crash


Call stack:
TypeError: Cannot read properties of undefined (reading 'trim')
    at file:///C:/src/keyboards/node_modules/@keymanapp/kmc-package/build/src/compiler/kmp-compiler.js:210:51
    at Array.map (<anonymous>)
    at KmpCompiler.transformKpsFileToKmpObject (file:///C:/src/keyboards/node_modules/@keymanapp/kmc-package/build/src/compiler/kmp-compiler.js:207:56)
    at KmpCompiler.transformKpsToKmpObject (file:///C:/src/keyboards/node_modules/@keymanapp/kmc-package/build/src/compiler/kmp-compiler.js:112:26)
    at KmpCompiler.run (file:///C:/src/keyboards/node_modules/@keymanapp/kmc-package/build/src/compiler/kmp-compiler.js:61:34)
    at BuildPackage.runCompiler (file:///C:/src/keyboards/node_modules/@keymanapp/kmc/build/src/commands/buildClasses/BuildActivity.js:9:39)
    at async BuildPackage.build (file:///C:/src/keyboards/node_modules/@keymanapp/kmc/build/src/commands/buildClasses/BuildPackage.js:10:16)
    at async ProjectBuilder.buildTarget (file:///C:/src/keyboards/node_modules/@keymanapp/kmc/build/src/commands/buildClasses/BuildProject.js:103:22)
    at async ProjectBuilder.buildProjectTargets (file:///C:/src/keyboards/node_modules/@keymanapp/kmc/build/src/commands/buildClasses/BuildProject.js:89:26)
    at async ProjectBuilder.run (file:///C:/src/keyboards/node_modules/@keymanapp/kmc/build/src/commands/buildClasses/BuildProject.js:50:18)

  1. Having <flags></flags> in the .kvks file still causes this compiler error (not a crash) error KM02908: Error encountered parsing vm_tamil_typewriter.kvks: Error: [{"instancePath":"/visualkeyboard/header/flags","schemaPath":"#/type","keyword":"type","params":{"type":"object"},"message":"must be object"}]

  1. Having multiple author emails would cause the same crash above
mcdurdin commented 2 weeks ago

Point 1:

Point 2:

Point 3:

mcdurdin commented 1 week ago

I am not sure we want to support multiple author emails in a single package -- it causes a cascade of changes all the way through to the website which seems unnecessary. Just update the package to have one of the emails in the URL and if they want more, they can put them into the Description field.

That means I think all three of these scenarios are resolved, so will close this issue.

EDIT: #11362 is an issue to turn crash into a compiler error message for scenario 3 above.