Add optional culture param to indexer of IStringLocalizer

[vaclavholusa-LTD]

Summary

Now when WithCulture() is gone there is no simple method to get localized resource from different culture without changing the application global CurrentUICulture.

Motivation and goals

In my ASP.NET Core App i I'm using translated URLs (stored in resource files). On every page I need to create "Alternate Links" tags that lead to the similar page with a translation. For this I'm iterating through supported cultures and getting translated link for every alternate lang page.

In scope

Since ResourceManagerStringLocalizer internally uses ResourceManger's GetString() method that already supports specifying culture, we should add the option to specify such culture instead of relying on global CurrentUICulture.

Out of scope

none

Risks / unknowns

ResourceManagerStringLocalizer.GetStringSafely() already accepts specific culture so this amendment should be safe to use.

Examples

I implemented this feature by creating my own inherited ResourceManagerStringLocalizer that adds such method.

Extra methods should be added to IStringLocalizer interface

LocalizedString this[string name, CultureInfo culture] { get; }
LocalizedString this[string name, CultureInfo culture, params object[] arguments] { get; }

and example implementation of the first one in ResourceManagerStringLocalizer (similar approach would apply to the second method)

public virtual LocalizedString this[string name, CultureInfo culture]
{
    get
    {
        if (name == null)
        {
            throw new ArgumentNullException(nameof(name));
        }

        var value = GetStringSafely(name, culture);

        return new LocalizedString(name, value ?? name, resourceNotFound: value == null, searchedLocation: _resourceBaseName);
    }
}

I can submit PR if you are happy with the suggestions.

[javiercn]

@vaclavholusa-LTD thanks for contacting us.

@DamianEdwards do you have any thoughts?

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[odinnou]

@vaclavholusa-LTD can you transmit a gist of your ResourceManagerStringLocalizer implementation ?

Currently the remove of WithCulture if the only one thing, that is blocking us to migrate to 5.0

thanks in advance

[vaclavholusa-LTD]

@odinnou basically this https://gist.github.com/vaclavholusa-LTD/2a27d0bb0af5c07589cffbf1c2fff4f4 just replace "Ltd" with your custom name :-) Had to inherit some built-in classes/interfaces since it doesn't exist in the original IStringLocalizer.. and naturally you need to pass ILtdStringLocalizer<T> as your dependency

[DamianEdwards]

We used to have this and it was removed: https://github.com/dotnet/aspnetcore/issues/3324

The reasons cited were basically that it didn't belong on the interface as it was confusing to imply that all implementations had to support a specific culture or that any instance was culture-specific.

Given that history I'm inclined to suggest we investigate a different approach to solving this, perhaps with a new interface that itself derives from IStringLocalizer that can also be implemented by the default ResourceManagerStringLocalizer, e.g. IStringLocalizerWithCultureFactory or some such.

[dIeGoLi]

I would assume the expectation of any programmer is, that you can specify a specific culture when retrieving a resource. Fallback to thread culture is fine. But having to change thread culture to achieve this effect is quite sad. Completely not understandable, why that feature was removed without a proper replacement. (down & upvotes seem to have been ignored) . I really hope a solution will arise. I would suggest not to make it unnecessary complicated with extra interfaces etc. There is a StringLocalizer with a method to retrieve strings... Confusion can be dealt with in different ways. As well you need to put in relation how many people will be using IStringLocalizer and how many people have to implement their own. A little more complexity for the later is justified instead of making it complicated for the majority.

[odinnou]

Hi, we made a radical choice. To allow us to switch to .NET 5, we stopped using IStringLocalizer and .resx files. We now work with .json files in i18next format.

[dIeGoLi]

Hi, we made a radical choice. To allow us to switch to .NET 5, we stopped using IStringLocalizer and .resx files. We now work with .json files in i18next format.

That is radical :). For me the framework works in most cases. (But as well i am not too happy with the organisation of resx files). There are just a handful of cases where i need to specify an explicit culture.

I like the suggestion of @vaclavholusa-LTD from a user perspective. But as "quick fix" i go with overriding thread culture, since it's easier to integrate.

    public static LocalizedString GetString(this IStringLocalizer stringLocalizer, User user, String name) 
      => GetString(stringLocalizer, user, name, Array.Empty<Object>());

    public static LocalizedString GetString(this IStringLocalizer stringLocalizer, User user, String name, params Object[] arguments) {
      String culture = user?.DefaultCultureName;
      CultureInfo cultureInfo = String.IsNullOrEmpty(culture) ? CultureInfo.CurrentUICulture : CultureInfo.GetCultureInfo(culture);
      CultureInfo cultureInfoOriginal = CultureInfo.CurrentUICulture;
      try {
        CultureInfo.CurrentUICulture = cultureInfo;
        CultureInfo.CurrentCulture = cultureInfo;
        return stringLocalizer.GetString(name, arguments);
      }
      finally {
        CultureInfo.CurrentUICulture = cultureInfoOriginal;
        CultureInfo.CurrentCulture = cultureInfoOriginal;
      }
    }

The user object specifies the desired culture, others may prefer to pass the culture info.

[NekkoDroid]

I personally would say something like this would be really useful to be able to explicitly state a culture for certain applications, but the suggested overloads with the CultureInfo as second parameter after the format-string could cause problems in cases where you used a culture in the message as first argument, I would suggest switching around the 2 parameters if it were ever implemented

LocalizedString this[CultureInfo culture, string name] { get; }
LocalizedString this[CultureInfo culture, string name, params object[] arguments] { get; }

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[maskalek]

Just following up on this. We are still waiting for the solution without chaning CurrentUICulture

[rjgotten]

The reasons cited were basically that it didn't belong on the interface as it was confusing to imply that all implementations had to support a specific culture or that any instance was culture-specific.

I would honestly like to see concrete metrics regarding that. Because I have a very hard time believing that.

I have an easier time believing in confusion over why a piece of code has to temporarily switch thread culture to fetch resources from a different culture. E.g. to fit translated links for other languages, as has been cited as a real-world usecase in this issue.

There was nothing wrong with WithCulture and it should never have been removed the way it was. What should have happened -- mind: if there actually was provably notable confusion over it -- is to update and strengthen its documentation to make clear what it does.

Actual (bad)

Creates a new IStringLocalizer for a specific CultureInfo.

Expected (good)

Creates a new StringLocalizer that is locked to the specified CultureInfo instead of observing CultureInfo.CurrentUICulture.

And possibly rename the method to WithLockedCulture or WithFixedCulture.

[schmitch]

wouldn't it make more sense with the api to actually use keyed services with IStringLocalizer to get a specific locale? like:

scope.ServiceProvider.GetRequiredKeyedService<IStringLocalizer>(new CultureInfo("de-De")) ?

[rjgotten]

@schmitch That would mean you need to statically register individual keyed IStringLocalizer services in the service container. Which is a very, very - pardon my French - crappy thing to force on users.

Moreover, you might not even know all the languages to be supported statically upfront.

[hishamco]

I might disagree with bringing this API back, fortunately, you remind me of CultureScope that I wrote a long time back in Orchard Core https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore.Abstractions/Localization/CultureScope.cs

Please let me know if you have any further questions or if we need to close this

[rjgotten]

@hishamco I might disagree with bringing this API back, fortunately, you remind me of CultureScope that I wrote a long time back in Orchard Core https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore.Abstractions/Localization/CultureScope.cs

Please let me know if you have any further questions or if we need to close this

The only thing that does is paper over things. While- yes; it is a neat little abstraction with an IDisposable that avoids having to manually do the culture swap and swap back or write try-finally blocks to ensure a thrown exception doesn't screw the pooch,

... it still doesn't resolve the cognitive disconnect of having to switch cultures in the first place. And the actual API surface is still an ugly wart.

How would you resolve something that needs to combine multiple resources from multiple languages in a single expression? Compute out everything beforehand? Any idea how ugly that becomes?

string en, de, fr, es;
using (CultureScope.Create("en")) 
{
  en = localizer["some.resource.name"];
}
using (CultureScope.Create("de")) 
{
  de = localizer["some.resource.name"];
}
using (CultureScope.Create("fr")) 
{
  fr = localizer["some.resource.name"];
}
using (CultureScope.Create("es")) 
{
  es = localizer["some.resource.name"];
}

return FormatCombinedSomehow(en, de, fr, es);

Compare that to:

return FormatCombinedSomehow(
  localizer.WithCulture("en")["some.resource.name"],
  localizer.WithCulture("de")["some.resource.name"],
  localizer.WithCulture("fr")["some.resource.name"],
  localizer.WithCulture("es")["some.resource.name"]
 );

Humorously, the proposed alternative that uses a CultureInfo as the first parameter, while it may work just as well for some other cases as the old and removed API would, is actually slightly worse for this particular degenerate case as well; though at least still more acceptable than the IDisposable-based swaperoo.

return FormatCombinedSomehow(
  localizer[CultureInfo.GetCultureInfo("en"), "some.resource.name"],
  localizer[CultureInfo.GetCultureInfo("de"), "some.resource.name"],
  localizer[CultureInfo.GetCultureInfo("fr"), "some.resource.name"],
  localizer[CultureInfo.GetCultureInfo("es"), "some.resource.name"],
);

[hishamco]

@rjgotten could you please elaborate on what are you trying to do with the above example?

[rjgotten]

@hishamco That example is meant to illustrate that the CultureScope based approach is not a solution, because it still has far more overhead for callers than simply passing a culture to the IStringLocalizer indexer or GetString method, or than constructing a derived localizer with a fixed culture as the old WithCulture API did.

It papers over the details of the overhead and lifts it to a higher level of abstraction, but doesn't resolve it anywhere near as elegantly as the removed WithCulture API did, or the proposed alternative based on passing the culture directly.

[hishamco]

I didn't mention this above as a solution in your case, that is why I told you in my last comment what are you trying to do, so I can help