I have been working on MDL-19764, and I think it now basically works. At least the original problem, as reported, is now fixed, which will make the Taiwanese very happy.
However, things have changed in Moodle 2.0, so there the scope to do something better there with regards to parent languages and local edits.
How should this work, ideally?
In Moodle 1.9, it works by finding countries.php from either the current language, the parent language or en_utf8, looking in both moodledata/lang and moodle/lang.
Then, it loads the list there, and applies any changes from moodle/lang/<whateverlang>_local/countries.php and moodledata/lang/<whateverlang>_local/countries.php.
As an example of that, fr_ca has parent lang fr. moodledata/lang/fr_ca_utf8/countries.php does not exist, so in Canadian French, the list you get is the one from moodledata/lang/fr_utf8/countries.php, with any amendments found in moodle/lang/fr_utf8_local/countries.php or moodledata/lang/fr_utf8_local/countries.php
However, that is not really consistent with how get_string works. For example, with get_string('CA', 'countries'), it would also look for edits in moodledata/lang/fr_ca_utf8_local/countries.php.
An alternative implementation of get_list_of_countries() do something like:
1. Get the list of known two-letter country codes from the array keys in moodle/lang/en_utf8/countries.php
2. For each of those do get_string(<country code>, 'countries')
So, anyway, I don't know which of those is better, and it is getting late, so I thought I would ask here, and then continue tomorrow.
It's getting late here, too However, I would expect it checks moodledata/lang/fr_ca_utf8_local/countries.php before the parent or the parent's local modification. So the search order would be similar to what get_string() does (or at least what I expect it should do, I haven't seen the get_string()'s logic for some time):
d.
- moodledata/lang/fr_ca_utf8_local/countries.php
- moodle/lang/fr_ca_utf8_local/countries.php
- moodledata/lang/fr_ca_utf8/countries.php
- moodle/lang/fr_ca_utf8/countries.php
- moodledata/lang/fr_utf8_local/countries.php
- moodle/lang/fr_utf8_local/countries.php
- moodledata/lang/fr_utf8/countries.php
- moodle/lang/fr_utf8/countries.php
- moodledata/lang/en_utf8_local/countries.php
- moodle/lang/en_utf8_local/countries.php
- moodledata/lang/en_utf8/countries.php
- moodle/lang/en_utf8/countries.php
d.
Irrespective of the details, there are really three key questions about what should be possible.
1. At the moment, you could create a custom language pack, say en_ou, with parent lang en, and in en_ou define just the countries
EN => England
SC => Scotland
WA => Wales
NI => Northern Ireland
Then, in your Moodle, users would only get a choice of those four countries on the sign-up page. Is this useful functionality to keep? (With my alternate proposal, you would have to hack the en language pack to do this.)
2. At the moment, you could add
$string['XX'] = 'The Republic of Moodle';
to en_utf8_loca/countries.php to add that additional choice to the countries dropdown. Should this sort of customisation be possible without hacking en_utf8?
3. At the moment, if you are using fr_ca and you want to customise the countries list, you have to know to edit fr_utf8_local/countries.php. Is that such a big problem?
I think that the answer to those three questions (which hopefully are slightly less technical, and closer to the end user - or at least administrator) really pin down what happens technically in the code.
1. At the moment, you could create a custom language pack, say en_ou, with parent lang en, and in en_ou define just the countries
EN => England
SC => Scotland
WA => Wales
NI => Northern Ireland
Then, in your Moodle, users would only get a choice of those four countries on the sign-up page. Is this useful functionality to keep? (With my alternate proposal, you would have to hack the en language pack to do this.)
2. At the moment, you could add
$string['XX'] = 'The Republic of Moodle';
to en_utf8_loca/countries.php to add that additional choice to the countries dropdown. Should this sort of customisation be possible without hacking en_utf8?
3. At the moment, if you are using fr_ca and you want to customise the countries list, you have to know to edit fr_utf8_local/countries.php. Is that such a big problem?
I think that the answer to those three questions (which hopefully are slightly less technical, and closer to the end user - or at least administrator) really pin down what happens technically in the code.
My opinions (based on real use cases):
- yes, admins want to restrict the list of offered countries.
- yes, admins want to extend the list of offered countries (like to add counties, districts or whatever, cantons or whatever). This is a potential issue when an user profile with the country value like UK_WLS is backed up and restored at another server. However, it is not so common to transfer users' profile data from server to server, so we should support it.
- it's not big problem. However, it is just not intuitive enough and the reason for it is the current implementation, not a conceptual aspect.
+1 for the three answers of David (also based on real life cases).
I had an alternative thought while cycling into work.
Instead of making the list implicit from the list of keys in countries.php, why not make it explicit.
Either a list
$string['allcountries'] = 'AF,AX,AL,DZ,...';
in lang/en_utf8/langconfig.php. (The idea would be that this would not be copied into other language packs, just like other language packs have parentlang, but en doesn't.)
Or an admin setting
$CFG->allcountries.
I think an admin setting is cleaner, except for upgrades when the ISO list of languages changes.
Then get_list_of_countries would explode that and call get_string a lot. That is clear simple and robust. It would be a little bit slower but not much, and it is only used in one performance critical place, admin/settings/location.php, and that should be changed to do a lazy-load anyway.
Instead of making the list implicit from the list of keys in countries.php, why not make it explicit.
Either a list
$string['allcountries'] = 'AF,AX,AL,DZ,...';
in lang/en_utf8/langconfig.php. (The idea would be that this would not be copied into other language packs, just like other language packs have parentlang, but en doesn't.)
Or an admin setting
$CFG->allcountries.
I think an admin setting is cleaner, except for upgrades when the ISO list of languages changes.
Then get_list_of_countries would explode that and call get_string a lot. That is clear simple and robust. It would be a little bit slower but not much, and it is only used in one performance critical place, admin/settings/location.php, and that should be changed to do a lazy-load anyway.
I just implemented the $CFG->allcountrycodes solution, and 240 calls to get_string takes 0.1 seconds, which I think it too slow, even for something not performance critical.
I think we need to use the fact that we know these strings can only come from countries.php files in various places to do a more optimal implementation. However, I should be able to make it work in exactly the same way, with $CFG->allcountrycodes. We just need more efficient loading of the strings.
There is now a patch on MDL-19764 that seems to work. My improved implementation takes about the same amount of time as 20 get_string calls, which is plenty fast enough.
Please can someone review that patch. Thanks.
Please can someone review that patch. Thanks.