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

Make PortableObjectStringLocalizer simpler for inheritance #9569

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

hishamco
Copy link
Member

Fixes #9542

@hishamco hishamco changed the title Make POStringLocalizer simpler for inheritance Make PortableObjectStringLocalizer simpler for inheritance May 28, 2021
@hishamco hishamco requested a review from Skrypt June 16, 2021 09:25
@Skrypt
Copy link
Contributor

Skrypt commented Jun 26, 2021

I'm not sure if this is necessary. Why would you want to inherit the PortableObjectStringLocalizer?

@hishamco
Copy link
Member Author

There are some case to allow developers to subclass the localizer. I remember one example is to allow multiple path to load the localization resources from. Make the members open some extensibility

@amolodenov you can mention your use case if it's possible

@amolodenov
Copy link

There are some case to allow developers to subclass the localizer. I remember one example is to allow multiple path to load the localization resources from. Make the members open some extensibility

@amolodenov you can mention your use case if it's possible

I need to use few cultures at the same time. For this, I planned to implement an analogue https://github.com/aspnet/Localization/blob/master/src/Microsoft.Extensions.Localization/ResourceManagerWithCultureStringLocalizer.cs, but PortableObjectStringLocalizer did not allow inherit correctly. I know ResourceManagerWithCultureStringLocalizer is obsolete, but changing CurrentCulture doesn't suit for me.
My example

public class PortableObjectWithCultureStringLocalizer : PortableObjectStringLocalizer
{
    private readonly ILocalizationManager _localizationManager;
    private readonly bool _fallBackToParentCulture;
    private readonly ILogger _logger;
    private readonly string _context;
    private readonly CultureInfo _culture;

    public PortableObjectWithCultureStringLocalizer(
        string context,
        ILocalizationManager localizationManager,
        bool fallBackToParentCulture,
        CultureInfo culture,
        ILogger logger)
        : base(context, localizationManager, fallBackToParentCulture, logger)
    {
        _context = context;
        _localizationManager = localizationManager;
        _fallBackToParentCulture = fallBackToParentCulture;
        _culture = culture;
        _logger = logger;
    }

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

            var translation = GetTranslation(name, _context, _culture, null);

            return new LocalizedString(name, translation ?? name, translation == null);
        }
    }

    public override LocalizedString this[string name, params object[] arguments]
    {
        get
        {
            var (translation, argumentsWithCount) = GetTranslation(name, arguments);
            var formatted = string.Format(translation.Value, argumentsWithCount);

            return new LocalizedString(name, formatted, translation.ResourceNotFound);
        }
    }
   
   ...
}

@hishamco
Copy link
Member Author

hishamco commented Jan 3, 2022

Any update on this, I think the idea to make the localizer more extensible nothing else

@Skrypt
Copy link
Contributor

Skrypt commented Jan 3, 2022

/cc @sebastienros

@hishamco
Copy link
Member Author

hishamco commented Sep 6, 2022

Can we triage this?

@sebastienros
Copy link
Member

Merging, but we should not make it a habit. It's exposing things we might regret as it's increasing the burden on breaking changes for future versions. One can easily copy the code if necessary. There isn't much value in inheriting directly from it.

@sebastienros sebastienros merged commit 14e13df into main Sep 8, 2022
@sebastienros sebastienros deleted the hishamco/po-localizer branch September 8, 2022 18:06
@hishamco
Copy link
Member Author

hishamco commented Sep 8, 2022

Sometimes there's a need to customize the default behavior, Copy paste is not good all the time ;)

@hishamco hishamco added this to the 1.5 milestone Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify PortableObjectStringLocalizer to enable correct inheritance
4 participants