search

keymanapp / keyman

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

Compiler should use FollowKeyboardVersion property to fill in Version info property from lexical model #1859

Open mcdurdin opened 5 years ago

mcdurdin commented 5 years ago

Technically, the lexical model is developed independently to the package that contains it. In this repo, it makes sense for the .model.kps version to follow the lexical model version precisely... we have that functionality available with the <FollowKeyboardVersion /> property in the <Options> key -- which is badly named but too late to fix.

Looking at the compiler, it understands that property but does not yet use it ... so that's something we need to implement.

_Originally posted by @mcdurdin in https://github.com/keymanapp/lexical-models/pull/25/review_comment/create_

mcdurdin commented 5 years ago

This may not make any sense. The lexical models do not currently have version information embedded. This metadata is only in packages. I think we can probably close this issue with a hurrah.

darcywong00 commented 5 years ago

The lexical models do not currently have version information embedded

Is that going to continue long-term? For maintenance, if we encounter two .model.js files, how will we know which one is newer?

atm, we're strongly recommending one lexical model per lexical model package. Wouldn't the model be the best place to control version info?

eddieantonio commented 5 years ago

The lexical models do not currently have version information embedded

Is that going to continue long-term? For maintenance, if we encounter two .model.js files, how will we know which one is newer?

atm, we're strongly recommending one lexical model per lexical model package. Wouldn't the model be the best place to control version info?

I agree with this, however, it would imply creating a change to the lexical model compiler to embed version information in every model. @mcdurdin, should we make that a feature for 12.0?

mcdurdin commented 5 years ago

After discussion, we decided to postpone this until a future version. The biggest reason is that additional complexity that FollowKeyboardVersion would add to the package compile -- it would need to interpret the lexical model in some way, which adds a bunch of risk. We accepted that risk with the keyboard compile but note that the keyboard compiler emits a regular format for the version string that we effectively grep for; the lm compiler offers no such guarantee :) Loading the model in a Javascript interpreter to find the version number may be an appropriate solution but creates a different set of complications!

As models will only be distributed in packages with 12.0, we can accept them not having version strings at this time.