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

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

Closed
darcywong00 opened this issue May 1, 2024 · 6 comments
Closed

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

darcywong00 opened this issue May 1, 2024 · 6 comments

Comments

@darcywong00
Copy link
Contributor

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

  1. 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
  1. 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
Copy link
Member

mcdurdin commented May 1, 2024

Can you include:

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

@mcdurdin mcdurdin added this to the A18S1 milestone May 1, 2024
@mcdurdin mcdurdin self-assigned this May 1, 2024
@mcdurdin mcdurdin changed the title bug(developer/compilers): Misc kmc crashes for 17.0 bug(developer): Misc kmc crashes for 17.0 May 1, 2024
@darcywong00
Copy link
Contributor Author

Can you include:

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

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

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

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

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
Copy link
Member

mcdurdin commented May 1, 2024

I think I need to update that to beta.

Oooh yeah!

@darcywong00
Copy link
Contributor Author

After updating to kmc 17.0.316-beta:

<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
Copy link
Member

mcdurdin commented May 3, 2024

Point 1:

Point 2:

  • The error is a little obtuse but it technically is not valid XML because whitespace is significant in XML: <flags></flags> == <flags/> but != <flags> </flags> ...

Point 3:

  • I don't think that we've ever supported multiple author emails in the URL field. So that can be a feature to be addressed post-release.

@mcdurdin
Copy link
Member

mcdurdin commented May 6, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants