-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add support for iso-639-2t/b #46
Conversation
Could you change Also, would you please update the README? |
Readme is updated. I hope the wording changes are ok. Regarding the binary size: I don't think a feature gate is necessary. The generated functions for converting between iso 639-3 and iso 639-2 are pretty small - only 64 LOC in total. In release mode this seems to come out at a slightly less than 2KiB binary size increase. Considering they're so small I'm also not sure if it makes sense to move them to another file - I'm also not very keen on duplicating the code generation logic. |
Thanks for the update.
What is the purpose of `iso_639_3_to_2t`? It is effectively the identity map
and could be removed, right? Same for the reverse.
Regarding the functions: I would like to have the data autogenerated. The
atcual code resides in lib.rs and just indexes into the tables, the same as
the other functions already do. Where do you see the code duplication?
|
True, I just wasn't sure if the standard guarantees iso 639-3 to be backward compatible (also in the future), so I generated these functions as well. Looking at it again though, I don't see how it would break compatibility, so I think it's safe to not generate the
The
I thought you indicated that Let me know if you want me to change anything in particular, in general I think the PR is in a pretty mergable state now. |
What are the data sources for your changes? I.e. could we automate the import of |
The data was already part of the iso-639-3.tab file that is being used for the iso-639-3 language codes. |
Adds support for is iso-639-2t and iso-639-2b
This implementation avoids adding extra tables for 2t/b, with the tradeoff, that the functions for 639-2 will incorrectly accept 639-3 codes, and will also generate 639-3 codes if a
Language
doesn't have a code in that standard.So for example, even though there is no 639-2 code for "Ghotuo" (iso 639-3 code "aaa") the following works:
I can rewrite it to use phf tables if this behavior is a problem.