Skip to content

Conversation

skyflyer
Copy link
Contributor

This change adds two unit tests that remove the project name from the and actually removes the project name from the generated context only for Razor metadata.

I created a sample project that this can be tested on: https://github.com/skyflyer/DotnetPoLocalizationDemo

The sample projects includes localization in .cshtml as well as localization in .cs (HomeController), which must preserve the project name (as it is part of the fully qualified class name).

@hishamco
Copy link
Member

hishamco commented Aug 10, 2025

Make sure your demo contains more than one module that contains at least one shared localization string, then check the generated output

@skyflyer
Copy link
Contributor Author

What do you mean by module? Like another project?

@hishamco
Copy link
Member

Exactly, because in Orchard Core, we have a lot of modules and some themes. So, one localization string could be used in many projects that's why the context is handy

@skyflyer
Copy link
Contributor Author

The solution now contains a solution with a Web (MVC) and a Razor Class Library (RCL) which is shared.

Food for thought: generate translation template looks like this:

#: PoLocalizationDemo.Lib/Views/Demo/Index.cshtml:6
#. <p>Localized string: @T["Welcome"]</p>
msgctxt "Views.Demo.Index"
msgid "Welcome"
msgstr ""

#: PoLocalizationDemo.Web/Controllers/HomeController.cs:32
#. ViewData["Message"] = T["Welcome from controller"];
msgctxt "PoLocalizationDemo.Web.Controllers.HomeController"
msgid "Welcome from controller"
msgstr ""

#: PoLocalizationDemo.Web/Views/Home/Index.cshtml:8
#. <h1 class="display-4">@T["Welcome"]</h1>
msgctxt "Views.Home.Index"
msgid "Welcome"
msgstr ""

As you can see, the library name is stripped from the views... but this leads me to wonder what would happen if two distinct razor class libraries would have the same view in the same directory and file name? E.g. Views.Home.Index

But, if the msgctxt is prefixed with the project name, the translation is not found by the Orchard localiation infrastructure, which generates the context based on the filename without the project name.

@hishamco
Copy link
Member

@skyflyer, I'm ready to ship the next version ASAP after merging #110, I'm wondering to include this; could you please do a quick test and apply your changes on the OC repo, then compare the extracted entries with the current ones

@skyflyer
Copy link
Contributor Author

@hishamco, sure, no problem, but either I am dense or there are no .pot files in the OrchardCore repo.

Ok, so after digging about a bit more, there appears to be a https://github.com/OrchardCMS/OrchardCore.Translations project that contains the translations...

I've ran it on the OrchardCore repo and there are many changes - most of which are of this type (path separator changes):

-#: OrchardCore.AdminMenu\AdminMenu.cs:28
+#: OrchardCore.AdminMenu/AdminMenu.cs:28

Some translations are removed (they don't exist anymore in the source files), which is OK.

The msgctxt of the Razor views has changed e.g.:

-#: OrchardCore.Admin\Views\AdminSettings.Edit.cshtml:24
+#: OrchardCore.Admin/Views/AdminSettings.Edit.cshtml:24
 #. <span class="hint">@T["Whether a New menu is displayed in the admin menu."]</span>
-msgctxt "OrchardCore.Admin.Views.AdminSettings.Edit"
+msgctxt "Views.AdminSettings.Edit"
 msgid "Whether a New menu is displayed in the admin menu."
 msgstr ""

Debugging the PortableObjectStringLocalizerFactory, I can see that this is the place where the project name is removed: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore.Localization.Core/PortableObject/PortableObjectStringLocalizerFactory.cs#L63

Why and how it works in the Orchard.Core codebase (where msgctxt contains the project name for the views as well) I do not know.

@skyflyer
Copy link
Contributor Author

Some more Orchard digging...

Taking OrchardCore.Setup.Views.Setup.Index as an example:

StringLocalizer is created with two arguments:

  • location:"OrchardCore.Cms.Web.Areas.OrchardCore.Setup.Views.Setup.Index"
  • and basename: "OrchardCore.Cms.Web"

which leads me to think that the PO localization is sometimes trimming the project name and sometimes not, depending on the project layout. If the project is part of an Area, then the project name is not trimmed (only the parent project is trimmed)...

... which is IMHO inconsistent.

What do you think the correct approach should be? IMHO, the localization infra should always leave the project name in place.

@hishamco
Copy link
Member

If you download the OC code-base, then run the tool, it will generate all the localization resources, and the message becomes handy when you have the same key available across multiple projects

@skyflyer
Copy link
Contributor Author

@hishamco, perhaps I have worded my comment in way that was not understandable, sorry for that. To summarize:

  • I have cloned both the Orchard.Core and OrchardCore.Translations repos
  • I ran the tool on the Orchard.Core and stored the generated POT files in the OrchardCore.Translations repo, where I did the diff and the analysis above
  • I believe that this PR should not be merged, as it will break the existing functionality that (OrchardCore.Translations and Orchard.Core) depend on
  • I also believe that the real problem lies in the Orchard.Core string localizer factory

I will open an issue with the OC repo.

I think this PR should be closed, but I'm interested to hear you opinion on the matter, since you seem to be more involved with both projects.

@hishamco
Copy link
Member

I commented here for the OC.Translations @agriffard will let you know about all the details, coz he is the one who worked on that.

BTW, I need to check this PR one more time, but I need to revise it with a clear mind :) I might defer the release until the weekend or push it ASAP then will decide if there's a new minor or major release if it's required

If you want to chat on Discord, feel free to join me there, and thanks for your effort and initiative

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.

2 participants