Convert LangID from u16 enum to tuple struct containing u16 #141
Conversation
Relax lang_id checks provided by enum so that any lang_id could be used, not only values listed in the enum. These checks increased the code size considerably without much benefit. Resolves #139. Language identifiers like LangID::EN_US have a new type, but this should not require source code changes in a code which uses them. Provide two traits to convert LangID to and from u16 freely. This implicitly provides TryFrom trait which previously was derived using TryFromPrimitive. There's no special constructor for LangID for values outside of the provided constants, but it can be converted from any u16. As TryFromPrimitive in no longer used, remove num_enum dependency.
There was a problem hiding this comment.
Thanks for this PR, I was just starting to look into this and forgot someone had submitted a PR for it already.
While this isn't technically a semver-compatible change without bumping the minor version (since some of the LangID API has changed), I don't think anyone was actively using that API since it was rather new. Given that, I'm fine just releasing this as a patch version and we can yank it later if it causes anyone trouble.
We need a CHANGELOG entry here, but I don't mind adding it myself later. Thanks for doing the leg work here. I'm going to test it out on some hardware and merge if it works :)
|
It worked great for me and definitely reduced the code size by 8KB upon analysis. Thanks for the change! |
Relax lang_id checks provided by enum so that any lang_id could be used, not only values listed in the enum. These checks increased the code size considerably without much benefit. Resolves #139.
Language identifiers like LangID::EN_US have a new type, but this should not require source code changes in a code which uses them.
Provide two traits to convert LangID to and from u16 freely. This implicitly provides TryFrom trait which previously was derived using TryFromPrimitive.
There's no special constructor for LangID for values outside of the provided constants, but it can be converted from any u16.
As TryFromPrimitive in no longer used, remove num_enum dependency.