Pkp-lib: enable longer locale names

Created on 22 Oct 2016  路  20Comments  路  Source: pkp/pkp-lib

To be able to deal with locale names that differ from the standard xx_YY pattern we've used exclusively so far, the PKP applications should be able to deal with longer locale names. Review of OJS (higher priority) and OMP code is necessary to make sure introducing longer locale names will not break the applications.

Most helpful comment

Hey Kassim and Alec, thank you so much! This is a very good thing to finally have, and we are more flexible with translations now. Thank you, @kaschioudi!

All 20 comments

There are still areas of the code that try to parse bits out of xx_YY (predominantly PKPLocale but also elsewhere). Those will need review at the same time.

@kaschioudi, do you think this is something you could look at?

Sure. Any available documentation for locale somewhere?

Not really, just that we assume throughout the code that it's xx_YY format. We need to relax that to support more diversity, and make sure that places assuming xx_YY will still behave nicely. @mtub, I think you were throwing around some proposals via email; would you mind putting the notes here for Kassim?

Hi @kaschioudi, our first use case would be going for these two locale names:

sr_RS@latin
sr_RS@cyrillic

(following the gettext example, see https://www.gnu.org/software/gettext/manual/html_node/Locale-Names.html)

I looked at the source code and the only place I found which actually tries to parse xx_YY locale is here (https://github.com/pkp/pkp-lib/blob/master/classes/i18n/PKPLocale.inc.php#L333).

Considering that syntax for locales is lang[_territory][.encoding][@variant], we currently have the following mapping (cf config.inc.php):
lang[_territory] => i18n.locale
encoding => i18n.client_charset

I suggest introducing a new configuration field under i18n category for the locale variant. Then update PKPLocale::initialize (https://github.com/pkp/pkp-lib/blob/master/classes/i18n/PKPLocale.inc.php#L123) to work with locale variant if defined.

Finally we could modify AppLocale::getLocale function to optionally return the locale variant if defined static function getLocale($withVariant = false).

@asmecher what do you think?

@kaschioudi, there are a few places that parse locale strings: get3LetterIsoFromLocale, getIso3FromIso1, getIso1FromIso3, getIso3FromLocale, and getIso1FromLocale (search PKPLocale.inc.php for substr). We're probably also reporting the xx_YY locales out to other places, like via the various OAI metadata formats, Dublin Core meta tags (plugins/generic/dublinCoreMeta), Google Scholar meta tags (plugins/generic/googleScholar), etc.

For the most part we won't want to have any variant, but some journals may want to operate with a mix of languages sometimes including a variant (e.g. en_US and sr_RS@latin) or even with different variants of the same language (sr_RS@latin and sr_RS@cyrillic).

I'd suggest allowing for locales to optionally specify a variant in registry/locales.xml, extending the various locale columns in the database to permit the additional data, then running through with a couple of test locales (e.g. the ones mentioned above) to ensure that the data is propagated well, e.g. all the way out to the OAI interface and Google Scholar meta tags mentioned above.

Merged into OJS and pkp-lib master; back-ported to OMP master.

Thanks, @kaschioudi! Don't forget to drop links to the pull requests here for easy reference (see e.g. https://github.com/pkp/pkp-lib/issues/1911#issuecomment-291261570), and when opening PRs that reference both an application and the pkp-lib shared library, include a commit updating the submodule hash with a specially-formulated commit message (https://pkp.sfu.ca/wiki/index.php?title=Github_Documentation_for_PKP_Contributors#PKP_library_submodule_changes).

@kaschioudi, some of the escaping changes broke the tests. I reverted the parts that seemed to make the difference: 38f4f95ecae4db94cd577810c37bb8a06cf53b73

Could you investigate this and see if you can track it down?

Here's an example of where the tests broke. It appears to be around the assignment of a second reviewer -- tests where a single reviewer is assigned seem OK. But I haven't investigated this much.

https://travis-ci.org/pkp/ojs/jobs/218229274

Also, the JS linter is complaining about types: https://travis-ci.org/pkp/ojs/jobs/218557251
I resolved one earlier complaint in this commit, but it has probably cascaded to cause these others: https://github.com/pkp/pkp-lib/commit/7aa00f8f05a1a7b84be1180712b2117cee9c7162

Could you have a look at these? (Using the submodule commit to link the app and submodule repos is helpful in causing the right pkp-lib branch to be checked out for testing purposes.)

Okay. I will take a look at this.

https://github.com/pkp/ojs/pull/1346
https://github.com/pkp/pkp-lib/pull/2416

@asmecher : I hope I have completed submodule commits properly.

Almost -- the submodule commit needs to be the last commit (use e.g. git rebase -i official/master to manage your commit list) and the comment needs to have ##account/mybranch## in it -- you're missing the end ##.

Phew! Thanks, @kaschioudi -- good to have this one closed.

Hey Kassim and Alec, thank you so much! This is a very good thing to finally have, and we are more flexible with translations now. Thank you, @kaschioudi!

@kaschioudi, how should be such a locale (sr_SR@latin for example) be defined in order to be usable? -- I am not able to install sr_SR@latin (for example) and to see it displayed in the UI and to switch to it... :-( I used this PR for the test: https://github.com/pkp/ojs/pull/1309 and I've changed the "key" attribute in "registry/locales.xml" to key="sr_SR@latin" (instead of the earlier "sr_SR").

Ah, sorry @kaschioudi, I had a typo and language toggle block unselected :-P Works well! Great! :-)))

Was this page helpful?
0 / 5 - 0 ratings