Fixed #24063 -- Locale code validation too strict #3815
Conversation
| @@ -1424,8 +1424,11 @@ def test_check_for_language(self): | |||
| self.assertTrue(check_for_language('en')) | |||
| self.assertTrue(check_for_language('en-us')) | |||
| self.assertTrue(check_for_language('en-US')) | |||
| self.assertTrue(check_for_language('be')) | |||
| self.assertTrue(check_for_language('be@latin')) | |||
There was a problem hiding this comment.
Could you add more tests? For example, "tr_TR.UTF-8", "tr_TR.UTF8" (without the dash), "de_DE.utf-8" (lowercase encoding), "sr_RS@latin", "sr_RS@12345".
There was a problem hiding this comment.
Added, but specifying charset is currently not supported (and IMHO does not make sense as Django enforces UTF-8 on locales).
|
Could you also add a release note to |
|
Added release note and squashed commits as well. |
|
buildbot, test this please. |
| self.assertTrue(check_for_language('be')) | ||
| self.assertTrue(check_for_language('be@latin')) | ||
| self.assertTrue(check_for_language('sr-RS@latin')) | ||
| self.assertTrue(check_for_language('sr-RS@12345')) |
There was a problem hiding this comment.
Looks like there is no standard for @modifier, but I'd reject all digits ones. Or to be more strict, we can only accept iqtelif, euro, cyrillic, abegede, devanagari, latin: https://sourceware.org/git/?p=glibc.git;a=blob;f=localedata/SUPPORTED;h=9bf2a4f3840925c38bdefeaba28d2077835c7f07;hb=HEAD
There was a problem hiding this comment.
I've quite intentionally kept it matching anything (same as language or country codes), but we can indeed be more strict.
Anyway there can be more in use, for example ca@valencia: https://github.com/translate/translate/blob/master/translate/lang/data.py#L44
There was a problem hiding this comment.
Anyway I don't think that hardcoding the list is good idea, for example here is list of variants available on my system:
# sed -n '/@/ s/.*@\(.*\) .*/\1/p' /usr/share/i18n/SUPPORTED | sort -u
abegede
cyrillic
devanagari
euro
iqtelif
latin
saaho
valencia
|
buildbot, test this please |
| @@ -374,6 +374,10 @@ Internationalization | |||
| reusable apps. It also allows overriding those custom formats in your main | |||
| Django project. | |||
|
|
|||
| * Django now allows to use locale variants as supported by Gettext. These are | |||
|
Release note needs to be moved to 1.9 at this point. |
|
Okay, I've rebased changes on current master, moved the release notes and made suggested improvements in them. Any chance to get this small change merged soon? |
| @@ -146,6 +146,10 @@ Internationalization | |||
| * The :func:`django.views.i18n.javascript_catalog` view now works correctly | |||
| if used multiple times with different configurations on the same page. | |||
|
|
|||
| * Django now allows to use locale variants supported by Gettext. These are | |||
There was a problem hiding this comment.
- "Django" is a bit redundant give these are the Django release notes. How about "You can now use...". Also, can you add details about where in Django these locale variants can be used?
- "gettext" shouldn't be capitalized
The locale code can contain variant after @, so let's allow the validation of it to pass. Also add more complete coverage for language code variants. Signed-off-by: Michal Čihař <michal@cihar.com>
|
buildbot, add to whitelist. |
|
merged in 76d26d8, thanks! |
The locale code can contain variant after @, so let's allow the
validation of it to pass.