Skip to content
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

Remove language normalization - use original language #3114

Merged
merged 1 commit into from Oct 27, 2021

Conversation

dmitrylyzo
Copy link
Contributor

Otherwise pt-PT is replaced by pt, but they have different translations.

Changes
Remove language normalization - use original language

Issues
Fixes #3103

Otherwise `pt-PT` is replaced by `pt`, but they have different translations.
@sonarcloud
Copy link

sonarcloud bot commented Oct 26, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@@ -60,7 +60,7 @@ const dateLocales = (locale) => ({
})[locale];

export function getLocale() {
return dateLocales(globalize.getCurrentLocale()) || enUS;
return dateLocales(globalize.getCurrentLocale()) || dateLocales(globalize.getCurrentLocale().replace(/-.*/, '')) || enUS;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we need to update to the correct names:

const dateLocales = (locale) => ({
'af': af,
'ar': arDZ,
'be-by': be,
'bg-bg': bg,
'bn': bn,
'ca': ca,
'cs': cs,
'da': da,
'de': de,
'el': el,
'en-gb': enGB,
'en-us': enUS,
'eo': eo,
'es': es,
'es-ar': es,
'es-do': es,
'es-mx': es,
'fa': faIR,
'fi': fi,
'fr': fr,
'fr-ca': frCA,
'gl': gl,
'gsw': de,
'he': he,
'hi-in': hi,
'hr': hr,
'hu': hu,
'id': id,
'is': is,
'it': it,
'ja': ja,
'kk': kk,
'ko': ko,
'lt-lt': lt,
'ms': ms,
'nb': nb,
'nl': nl,
'pl': pl,
'pt': pt,
'pt-br': ptBR,
'pt-pt': pt,
'ro': ro,
'ru': ru,
'sk': sk,
'sl-si': sl,
'sv': sv,
'ta': ta,
'th': th,
'tr': tr,
'uk': uk,
'vi': vi,
'zh-cn': zhCN,
'zh-hk': zhCN,
'zh-tw': zhTW
})[locale];

@thornbill thornbill merged commit 1ddcb97 into master Oct 27, 2021
@thornbill thornbill deleted the fix-lang-loading branch October 27, 2021 04:00
@ptalmeida
Copy link
Contributor

Honestly I'm looking to opening this issue but I don't know where. The PT locale should not exist, there are only two versions of PT: - Pt_pt and Pt_Br. Having PT only raises confusion as it is not specific to European or Brazilian Portuguese (Angola, etc. use European portuguese).

And the translations in the app reflect this problem with some apps having ptpt and no pt and some having pt and no ptpt.

Any Idea of where to raise this issue?

@ptalmeida
Copy link
Contributor

@thornbill tag

@dmitrylyzo
Copy link
Contributor Author

@ptalmeida in jellyfin-web, imo

This will require removing pt from several places in web (Weblate should also be updated).


const languages = ['af', 'ar', 'be-by', 'bg-bg', 'bn_bd', 'ca', 'cs', 'da', 'de', 'el', 'en-gb', 'en-us', 'eo', 'es', 'es-419', 'es-ar', 'es_do', 'es-mx', 'fa', 'fi', 'fil', 'fr', 'fr-ca', 'gl', 'gsw', 'he', 'hi-in', 'hr', 'hu', 'id', 'it', 'ja', 'kk', 'ko', 'lt-lt', 'mr', 'ms', 'nb', 'nl', 'pl', 'pr', 'pt', 'pt-br', 'pt-pt', 'ro', 'ru', 'sk', 'sl-si', 'sq', 'sv', 'ta', 'th', 'tr', 'uk', 'ur_pk', 'vi', 'zh-cn', 'zh-hk', 'zh-tw'];

and the file itself.

At the same time date-fns uses some rules for locales: https://date-fns.org/v2.25.0/docs/I18n-Contribution-Guide#choosing-a-directory-name-for-a-locale
For example, pt is pt-PT, ru is ru-RU.

@ptalmeida
Copy link
Contributor

ptalmeida commented Oct 27, 2021

@dmitrylyzo Yes indeed, I just made a post on reddit about that. There are more problems such as pt having pt-BR content due to the confusion, and it affects many translation components.

https://www.reddit.com/r/jellyfin/comments/qgzi6j/there_is_a_locale_for_jellyfin_translations_that/

@dmitrylyzo
Copy link
Contributor Author

Just to note: pt was added in #2280 to match Weblate.

@ptalmeida
Copy link
Contributor

Yep, the problem is not with the code but with the content in Weblate imo.

@ptalmeida
Copy link
Contributor

@joshuaboniface @anthonylavado @mcarlton00 I see you as project maintainers in the weblate project, sorry if spam but I'm not sure if you browse reddit where I made this post about the issue.

Please address this when you can. There are some more comments there but I'll copy the post here for convenience.

--------- POST ---------

I'm speaking out here to raise awareness as there is no Github page "central" to talk about translations". I'll try to explain clearly.

This affects translations for European Portuguese.


What is happening:

In the Jellyfin weblate page there are locale options (pages where community members translate Jellyfin for their mother tongues) for 3 versions of Portuguese: European (pt_PT), Brazilian (pt_BR), and simply (pt).

As you can see inside the Jellyfin project, these are the situations:


Here's the problem:

There are only 2 main subdivisions of Portuguese, and application locales generally follow this metric: Brazilian Portuguese (pt-br), and European Portuguese (pt-pt). See Microsoft's locale codes and Signal Messaging App Translation Page.

(There may be more such as pt_MZ for Mozambique, but not a general (pt) locale as the main variations pt_BR and pt_PT are drastically different).

By having (pt) as a locale Jellyfin makes it very confusing, per example, when choosing a locale for the web version, and it is not clear for contributors what version of Portuguese (pt) should be. Furthermore, (pt_PT) becomes incomplete because of it as contributors don't know where to add translations, while (pt) has both European Portuguese and Brazilian Portuguese text. Also problems in the code such as this.


What I'm asking for

To avoid confusion please @admins convert the components /Jellyfin Android Fastlane and /Jellyfin Glossary from (pt) to (pt_PT) and remove the (pt) locale entirely.

(some content in (pt) is Brazilian Portuguese due to this confusion but it should be fixed over time, I'm willing to contribute)

(the other translation components such as for Android or Web already exist in pt_PT)

I hope I made the point easy to understand, it may seem like a small thing but for pt_PT speakers who want a good experience for their instance's users it is quite important.

Thanks, ptalmeida.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many translation strings not applied.
3 participants