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

Things may consider for Localization 3.0.0 #3289

Closed
hishamco opened this issue Jun 30, 2018 · 11 comments
Closed

Things may consider for Localization 3.0.0 #3289

hishamco opened this issue Jun 30, 2018 · 11 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one

Comments

@hishamco
Copy link
Member

hishamco commented Jun 30, 2018

There 're some points I'd like to share with you guys, that may we consider for localization 3.0.0:

  • Move resource manager classes into a new namespace Microsoft.Extensions.Localization.ResourceManager, this is useful for APIs organization and if we add another localization source provider such as POStringLocalizer also to align with other ASP.NET Core repos such as Configuration, Security .. etc
  • Use ASP.NET Core MemoryCache instead of using IResourceNamesCache & ResourceNamesCache, no need to reivent the wheel, while we are have a rich and tested APIs
  • Using ResourceManager resource set directly from within ResourceManagerStringLocalizer instead of introducing IResourceStringProvider & ResourceManagerStringProvider. If I'm not wrong those APIs was introduced because the lack and limitation of the ResourceManager resource set APIs support in .NET Core in the past
  • Support Pluralization APIs out-of-the-box
  • Support another localization resource provider such as POStringLocalizer
  • I think we need to revisit the IStringLocalizerFactory the Create(baseName, assembly) is not used except testing, also the Create() needs to be more generic rather it is used for ResourceManager
  • Move WithCulture as extension method (if it's needed)
  • Remove IStringLocalizer extensions methods - except GetAllString(), because I didn't see any value except calling the indexer from within a method
  • Can we rid off SearchedLocation property from LocalizedString and move it into a common place, because all the resource should searched in a certain location(s)
  • These are some that raised on my head and I will modify the list if I have other once ..

/cc @mkArtakMSFT

@hishamco
Copy link
Member Author

hishamco commented Jul 5, 2018

@ryanbrandenburg could you please have a look

@hishamco
Copy link
Member Author

hishamco commented Jul 9, 2018

@pranavkm could you please have a look, or forward it to one of your team if you don't mind

@hishamco
Copy link
Member Author

  • LocalizedHtmlString has IsResourceNotFound while LocalizedString has ResourceNotFound, we should align those and remove the confusion while both are localizer
  • Why IHtmlLocalizer doesn't implement IStringLocalizer? the class seems a clone copy

@bgribaudo
Copy link

Any chance that support could be added for refreshing cached localizers so that localization strings can be updated live?

Currently, after a cshtml file is modified, the changes made are displayed the next time the relevant page is refreshed. However, due to the way localizer caching works, data annotation translations are cached when they are first loaded. Restarting the site is required to refresh the cache. It would be nice to have symmetry of behavior here--save a cshtml file or a translation file and the update is reflected live.

From the localizer developer's perspective, the easiest way to do this might be allowing the localizer to specify a caching strategy when it's created/registered(singleton, per request, etc.).

Reference: aspnet/Mvc#7887 (comment)

@Eilon Eilon added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed repo:Localization labels Nov 7, 2018
@mkArtakMSFT
Copy link
Member

Sorry for not seeing this earlier, @hishamco.
We'll review this proposal and update the issue appropriately soon.

@mkArtakMSFT mkArtakMSFT added super-triage enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Nov 7, 2018
@damienbod
Copy link
Contributor

damienbod commented Nov 8, 2018

I would also welcome support for multiple shared resources, for DataAnnotationsLocalization, view localization and localization. Then it we be easy to use resources from multiple packages, projects, where each brings it's own localization. For example I could have 1 shared resource for each feature X, and use this in my main project.

@hishamco
Copy link
Member Author

hishamco commented Nov 8, 2018

@mkArtakMSFT no problem, I knew you 're busy with the team
@damienbod good point, do you mean using a resx from one project to multiple projects, which is suitable for modules / packages scenarios? If it's YES, I will add it in my list above

@damienbod
Copy link
Contributor

@hishamco yes

@hishamco
Copy link
Member Author

BTW I started a prototype to do such thing two weeks ago, hope I can share it with you and make a blog post about it, nothing but have a built-in an optimal solution will be handy for all the developers

@mkArtakMSFT
Copy link
Member

Thanks for sharing your ideas folks.
In general, we're not making breaking changes for just cleanup. Only high-value changes are being considered in this bucket. It doesn't sound like any of the above mentioned breaking changes would meet the bar.
The two suggestion, thought, we debated about, were the pluralization and PO file support. While that's the case, it seems there is a good community support for both of these. Specifically, Orchard Core has support for both of these, which are exposed as packages for consumption by any application. You can find more details regarding those here: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/portable-object-localization?view=aspnetcore-2.1

@bgribaudo, can you please file a separate issue for the following ask. We would like to review it further:

Any chance that support could be added for refreshing cached localizers so that localization strings can be updated live?

@damienbod, please file a separate issue with more details regarding your scenarios, as it's not really clear what the ask is.

@hishamco
Copy link
Member Author

@mkArtakMSFT regarding the pluralization, we need to provide a base infrastructure to support it, after that we can build a new provider or use Orchard Core package on top of the suggested APIs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

5 participants