search

DamianEdwards / i18nStarterWeb

ASP.NET 5 Starter Web project template converted to use proposed i18n/l10n system
Apache License 2.0
16 stars 7 forks source link

Feedback #8

Closed glen-84 closed 9 years ago

glen-84 commented 9 years ago

StringLocalizer

  1. Have you considered adding an option to quietly handle formatting exceptions? I think it's possible that the resources could come from an external location and contain formatting errors. Maybe the default message could be used if there is a formatting exception (opt-in)?
  2. Is it necessary to have ResourceManagerLocalizer in addition to ResourceManagerWithCultureLocalizer? Is it not possible to just have ResourceManagerLocalizer(ResourceManager resourceManager, CultureInfo culture = null) (or two constructors)
  3. The parameter name "key" is used in some places (f.e. in LocalizedString), and "name" is used elsewhere (f.e. in IStringLocalizer). Maybe it should be "key" everywhere?
  4. Won't there be a lot of duplication if you have one resource file per view?
  5. With regard to locale-specific views: I've seen this type of thing before, but I think that (a) there will be loads of duplication, and (b) it will get messy [especially with more than just a few supported locales]. There may be performance implications as well, with file existence checks, fallbacks, etc. I would definitely keep this as "disconnected" as possible, I don't think that many people will use it (but I could be wrong).
  6. Will it be possible to configure validation messages without using attributes? (defaults in addition to per-type overrides) I think that putting the validation messages in the models will get messy quite quickly.
  7. I created a proof of concept here which allows you to create a string localizer with a custom string "provider" (source of translated strings/data) and an optional string "formatter" (examples are included for SmartFormat.NET and messageformat.net). The DI stuff is not implemented, as this is just to demonstrate the idea. The Index.cshtml file contains all of the instantiation code for now. The main examples start from line 109. The example string provider just contains a Dictionary<string, string>, but this would usually load strings from a data source such as a database, resource files, JSON, CSV, web service, etc. Let me know what you think, and if something like this might be included.
  8. With the above, almost all translation functionality can be implemented, with the exception of text/message domains (unless this can somehow be set statically). Text domains are supported in PHP (gettext), Zend Framework (PHP), Symfony (PHP) Ruby (seems to support "namespaces"), Perl, etc. If you want to support this, I guess IStringLocalizer would need overloads that accepted a textDomain parameter of type string. The validation and metadata model provider system would also need to accept this (optional) parameter.

RequestLocalizationMiddleware

  1. How will you handle prioritization? For example if both a cookie is set, as well as a header or delegate, which takes priority? Would using a priority queue and a culture selection interface take a lot of time to implement? (for v1 you could leave out the update/set-locale side of the process). The other benefit here is that users could easily create other "strategies" (although I guess they could write their own middleware if necessary).
  2. Sometimes you only want to set the UICulture for translation, and use another culture for number formatting, etc., so it may be best to allow the developer to specify which culture settings to set from the RequestLocalizationMiddleware. (see here)

(some feedback copied from my comment here)

DamianEdwards commented 9 years ago

Thanks for taking the time to offer so much feedback! Let me say that for our initial version, we intend to keep the feature set small. We can always build it out from there. That said, let me try to address each suggestion:

StringLocalizer

  1. We could, or the app developer could easily wrap our type and do that themselves. I'm not convinced ignoring format exceptions is the right thing to do. Regardless of where the messages come from, they should be owned by the app and be in the right format.
  2. Either would work, but the pattern we have now means any IStringLocalizer can be turned into a culture specific one easily, which is nice when the primary way people will get them is via DI on that interface, not from the concrete types.
  3. Yeah that should be consistent. We'll clean that up.
  4. That's totally optional. IViewLocalizer has that behavior right now in the sample, but the /Shared/_Layout.cshtml uses a an IStringLocalizer directly. Any view can just create an IHtmlLocalizer from IHtmlLocalizerFactory for any source they like. We could look at making IViewLocalizer a little more flexible by looking in multiple places for resources instead of just a file named after the view.
  5. People do locale-specific views all the time. There are cases when it's just a better choice. You'll be free to use it or not.
  6. You can specify a different source for your validation information so that it doesn't have to be on the same type as your model. That's the same as it is today. Note that ValidationAttribute is in the .NET Framework and can't really be changed to do something else for validation messages, so this is the best approach we could come up with working within those boundaries.
  7. So this is basically splitting up the concerns of the source of localized strings vs. the formatting of them? If so, cool, and you plumbed it through IStringLocalizer which shows apps would be free to make composition choices like this.
  8. I'm not familiar with text domains, but they appear to simply be further classification of where the strings come from. In our case, the IStringLocalizer is that domain. When you create one from the IStringLocalizerFactory you can provide a location which is just a string that can be used to represent any source. In our default implementation which is based on .NET's ResourceManager, it represents an AssemblyName.

RequestLocalizationMiddleware

  1. The middleware will likely to a search in priority order, with the first one being satisfied being used (order will likely be cookie then Accept-Language header). Then there'll be a delegate the app can set to have a chance to change it to whatever it likes, e.g. set from a user profile DB. This is the simplest thing we can do that enables the scenarios. The app is of course free to make its own middleware that does whatever it likes. It just needs to set the culture on the current thread and anywhere else it cares to.
  2. The middleware will likely have an options to configure which of the culture properties it sets (CurrentCulture and/or CurrentUICulture). Again, it won't be a very complex middleware and could be easily replaced by the app with whatever custom code it likes. We'll make sure things like the Accept-Language header parsing logic can be easily reused.
glen-84 commented 9 years ago

StringLocalizer

1) Maybe you're right. It's just that for a non-developer, it's very easy to forget a brace, or to add too few/too many placeholders. If it's going to be strict, then developers will need to create a tool to validate the messages before putting them into production.

2) Okay. To be honest, I'm not really familiar with this pattern (creating an instance of a type from an instance of the same/similar type), but it's not important. :smile:

6) "Note that ValidationAttribute is in the .NET Framework and can't really be changed ..." – how does the ErrorMessage get localized? Are you suggesting that you won't be able to use a different type of string formatting, for example?

7) Pretty much. For example, you could have an ILocalizedStringProvider that loads strings from resource files, and then the developer could switch between using regular string formatting, SmartFormat.NET, messageformat.net, etc. I should add caching somewhere as well.

8) It's for grouping messages into logical "domains". For example, you could have a text domain for each MVC area, and one for the application as a whole. If the same message is translated in more than one domain, you could prioritize the messages in the current area, or always favour the application-level translation (depending on your needs). However, I can't really say how important this is, since I haven't used it myself.

RequestLocalizationMiddleware

1) I may write a strategy-based culture selection middleware later, but it's not a very high priority right now. :smile:

2) "We'll make sure things like the Accept-Language header parsing logic can be easily reused." – that would be great.

DamianEdwards commented 9 years ago

The localization work is over at https://github.com/aspnet/Localization now