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

/Localization/[CultureName]/[ModuleId].po PO file location doesn't work #16153

Closed
hyzx86 opened this issue May 25, 2024 · 14 comments · Fixed by #16419
Closed

/Localization/[CultureName]/[ModuleId].po PO file location doesn't work #16153

hyzx86 opened this issue May 25, 2024 · 14 comments · Fixed by #16419

Comments

@hyzx86
Copy link
Contributor

hyzx86 commented May 25, 2024

Describe the bug

To Reproduce

Hi all, I've found that this rule mentioned in this document https://docs.orchardcore.net/en/latest/reference/modules/Localize/ doesn't seem to work
/Localization/[CultureName]/[ModuleId].po
This is a module based on OC 1.8.3
image

But when I use Localisation/zh-CN.po it works fine

Since this template Localisation/[CultureName].po will overwrite the translation files of the other modules when compiled and packaged.
I would prefer that the template /Localisation/[CultureName]/[ModuleId].po works.
When I run it on 1.8x it doesn't work as expected.

hyzx86 added a commit to hyzx86/OrchardCore that referenced this issue May 25, 2024
@hishamco
Copy link
Member

Seems you started a PR for this, if you are interested it's in your plate otherwise I will create another one

@hyzx86 hyzx86 closed this as completed May 26, 2024
@hyzx86
Copy link
Contributor Author

hyzx86 commented May 26, 2024

Seems you started a PR for this, if you are interested it's in your plate otherwise I will create another one

I am not familiar with this aspect.

@hyzx86 hyzx86 reopened this May 26, 2024
@Piedone
Copy link
Member

Piedone commented May 26, 2024

Shouldn't these locations rather work, @hishamco?

@hishamco
Copy link
Member

They should, but we need to ensure the case that Tony reported still worked as expected, @hyzx86 I suggest creating a minimal repo, and then we can check.

@Piedone Piedone changed the title Are some of the internationalised file location rules mentioned in the documentation out of date? /Localization/[CultureName]/[ModuleId].po PO file location doesn't work May 26, 2024
@hyzx86
Copy link
Contributor Author

hyzx86 commented May 27, 2024

They should, but we need to ensure the case that Tony reported still worked as expected, @hyzx86 I suggest creating a minimal repo, and then we can check

I think that test item should be included in the repository, along with automated tests to ensure the stability of the functionality

@hishamco
Copy link
Member

What I'm asking for is a repo to ensure that there's a bug, but not miss configuration by you :)

@sebastienros
Copy link
Member

The code that loads these files is in ModularPoFileLocationProvider. A breakpoint should help understand why the file is not loaded.

@sebastienros sebastienros added this to the 2.x milestone May 30, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@JulioAmestica
Copy link

the problem could be the ZH-CN language is not present in DefaultPluralRuleProvider. question, why are the languages hard coded??
By the way (the iso 639-1 defines languages codes)[https://www.andiamo.co.uk/resources/iso-language-codes/]

@hishamco
Copy link
Member

This should handle Chinese cultures

/// <example>zh-Hans-CN -> zh-Hans -> zh.</example>
private static CultureInfo GetBaseCulture(CultureInfo culture)
{
var returnCulture = culture;
while (returnCulture.Parent.Name != "") // Stop at Invariant culture
{
returnCulture = returnCulture.Parent;
}
return returnCulture;
}
}

@hishamco
Copy link
Member

@hyzx86 please is this still happening with you

@hyzx86
Copy link
Contributor Author

hyzx86 commented Jun 28, 2024

the problem could be the ZH-CN language is not present in DefaultPluralRuleProvider. question, why are the languages hard coded??

Like Hisham mentioned code, because there's also a concept of hierarchy here

@hishamco It may be necessary to add some unit tests to ensure that zh-CN is recognised as zh.

@hishamco
Copy link
Member

hishamco commented Jun 29, 2024

[Theory]
[InlineData("en-US", "en")]
[InlineData("zh-TW", "zh")]
[InlineData("zh-HanT-TW", "zh")]
[InlineData("zh-CN", "zh")]
[InlineData("zh-Hans-CN", "zh")]
[InlineData("zh-Hans", "zh")]
[InlineData("zh-Hant", "zh")]
public void TryGetRuleShouldReturnRuleForTopCulture(string culture, string expected)
{
var expectedCulture = CultureInfo.GetCultureInfo(expected);
var testCulture = CultureInfo.GetCultureInfo(culture);
var pluralProvider = new DefaultPluralRuleProvider();
pluralProvider.TryGetRule(expectedCulture, out var expectedPlural);
pluralProvider.TryGetRule(testCulture, out var testPlural);
Assert.NotNull(expectedPlural);
Assert.NotNull(testPlural);
Assert.Same(expectedPlural, testPlural);
}

@hyzx86
Copy link
Contributor Author

hyzx86 commented Jul 9, 2024

First , we do not support the format /Localisation/[CultureName]/[ModuleId].po as mentioned in the documentation.

So I added this logic here, but it still doesn't seem to work https://github.com/OrchardCMS/OrchardCore/pull/16154/files#r1670823332

The debugging information is as follows

image

The full test code is here
https://github.com/OrchardCMS/OrchardCore/pull/16154/files#diff-96bb404a36e5e90d674923486169524bd90cc1a6a7d6f58f1f9aa7f3b22968ba

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

Successfully merging a pull request may close this issue.

6 participants