Skip to content

What's the intended behavior of IStringLocalizer in regards to format strings? #2642

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

Closed
aspnet-hello opened this issue Jan 2, 2018 · 6 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Comments

@aspnet-hello
Copy link

From @hiiru on Monday, March 20, 2017 2:44:38 PM

Background:
The reason of this question is a difference between Localization's IStringLocalizer and Mvc.Localization's IViewLocalizer which I noticed last week, or rather in their official implementations (ResourceManagerStringLocalizer and ViewLocalizer).
Initially I created a PR in Localization (aspnet/Mvc#5962) since I think of this as a bug, during review @dougbu @pranavkm pointed out that intended behavior isn't clear. Before deciding on a way to change this, since it would break the currently shipped behavior, it should be clarified here in the Localization repo so the right approach can be chosen.

The ResourceManagerStringLocalizer behaves like this:
this[name] -> returns raw string
this[name, arguments] -> returns string.format of raw string and arguments

The ViewLocalizer like this:
this[name] -> returns string.format of raw string with empty arguments
this[name, arguments] -> returns string.format of raw string and arguments
Actually a LocalizedHtmlString, however during response rendering it's changed to the described behavior when WriteTo is called.

There is also a code example of this here: https://github.com/hiiru/LocalizationBugExample

Before creating this issue I looked at the tests, comments and documentation of Localization, however I didn't find anything that explicitly describes the behavior.
Also I didn't see any test for the this[name, arguments] signature at all (based on dev branch), nor is any test related to formatted strings.

In my opinion, I think the IStringLocalizer implies the current behavior of ResourceManagerStringLocalizer due to the minor comment difference between both this[] comments.
Also I wouldn't expect a string.format when I don't provide arguments.

Question:
What is the intended behavior of IStringLocalizer's this[name]?
Should this return a raw string or a formatted string?

Copied from original issue: aspnet/Localization#346

@aspnet-hello
Copy link
Author

From @dougbu on Monday, March 20, 2017 4:05:55 PM

@DamianEdwards says "no, I don't want to type". He also mentioned this sounds like "just a bug" when described this way. @DamianEdwards prefers to fix ViewLocalizer and to provide a flag or option somewhere to control it. He left it as an implementation detail whether we change the default behaviour or not.

In other words, he asked Triage to do the Right Thing:tm:.

@aspnet-hello
Copy link
Author

From @hiiru on Tuesday, March 21, 2017 1:09:12 PM

Ok, I'll update my PR so that the behavior is managed by a flag/option, any further detail we can discuss inside the PR in Mvc.

Since this doesn't directly involve Localization anymore, I close this issue here.

@aspnet-hello
Copy link
Author

From @dougbu on Tuesday, March 21, 2017 1:21:32 PM

Reopening because Triage members have not weight in on this issue or the milestone where it might be addressed.

@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
Contributor

Closing this issue as there was no community involvement for quite a while now.

ryanbrandenburg pushed a commit that referenced this issue Nov 27, 2018
Adding HubConnection State Enum (#2627)
@anisite
Copy link

anisite commented Nov 7, 2019

Hi, I have this issue with .net core 3.0, why don't simply add a condition to skip the string.format when no arguments?

@mkArtakMSFT
Copy link
Contributor

Hi @anisite.

It looks like you are posting on a closed issue!
We're very likely to lose track of your bug/feedback/question unless you:

  1. Open a new issue
  2. Explain very clearly what you need help with
  3. If you think you have found a bug, include detailed repro steps so that we can investigate the problem

@dotnet dotnet locked and limited conversation to collaborators Nov 11, 2019
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
Projects
None yet
Development

No branches or pull requests

4 participants