Localisation with Razor #861

Merged
merged 25 commits into from Dec 29, 2012

Projects

None yet

2 participants

@jchannon
Member

As the NancyContext hopefully now has Culture on it it would be useful for views to be able to output translations.

I have added ILocalisation to RazorViewBase and then in a View you can call @Localisation["Greeting"]

This then goes to the implementation whether that be Resx, Database etc to return the correct translation.

At present there is a DefaultLocalisation to show an example however I'm not sure whether we need to include this and just let users have an implementation that TinyIOC picks up automatically

Also added are a new set of ViewLocation conventions which are duplicates of the exisintg ones with culture tagged on but given higher priority.

For example

(viewName, model, viewLocationContext) => {
    return string.Concat("views/", viewName, "-", viewLocationContext.Context.Culture);
 },

 (viewName, model, viewLocationContext) => {
     return string.Concat("views/", viewName);
 },

So in the Module you specify return View["Index"] and then based on the DefaultCultureConventions it determines the culture and then the view locator looks for "Index-en-GB" and if it doesnt find it, it looks for "Index"

@thecodejunkie
Member

This needs rebasing. Since you branched from your culture branch, you're going to have a lot of duplication in here which will cause merge conflicts

Even though @grumpydev tries his best.. I think we should stick the the US spelling here

@thecodejunkie
Member

Maybe @Localize would be a better name? Not sure if we want to hang it of the @Html helper or not? Like @Html.Localize("string")

@jchannon
Member

After discussions in Jabbr I have put a spike branch here https://github.com/jchannon/Nancy/branches/TextResourceSpike

In the view you now use @Text.Greeting as it uses dynamic/DynamicObject

I have also put in a sample app (Nancy.Demo.Localisation). Run it and it will show the usage and also view location convention changes so if you call return View["Index"] it will look at your culture and by default look for Index-en-GB.cshtml else Index.cshtml

@thecodejunkie
Owner

Good point in fact it should be removed

@thecodejunkie

Good thing this is a spike! ;)

Owner

LOL!

@thecodejunkie

I wonder what we should return if you access an invalid name? Or perhaps that should be up to each ITextResource ?

Owner

In my implementation it returns null because the key doesn't exist. I guess we leave it to the implementation?

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.Demo.Localization/Modules/HomeModule.cs
@@ -0,0 +1,31 @@
+using System;
@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.Demo.Localization/Modules/HomeModule.cs
@@ -0,0 +1,31 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using System.Threading;
+using System.Web;
+
+namespace Nancy.Demo.Localization
+{
+ public class HomeModule : NancyModule
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

The indenting in this module is a bit.. psychedelic 😸

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.Demo.Localization/Modules/HomeModule.cs
@@ -0,0 +1,31 @@
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

I think this whole project might be better being called Nancy.Demo.Razor.Localization because that's what it is showing off, not localization in general

@thecodejunkie
thecodejunkie Dec 28, 2012 Member

I think this was the only comment you skipped =)

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
...ncy.ViewEngines.Razor.Tests/RazorViewEngineFixture.cs
@@ -1,4 +1,5 @@
using System.Threading;
+using Nancy.Culture;
@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.ViewEngines.Razor/RazorViewEngine.cs
@@ -36,7 +36,7 @@ public IEnumerable<string> Extensions
/// Initializes a new instance of the <see cref="RazorViewEngine"/> class.
/// </summary>
/// <param name="configuration">The <see cref="IRazorConfiguration"/> that should be used by the engine.</param>
- public RazorViewEngine(IRazorConfiguration configuration)
+ public RazorViewEngine(IRazorConfiguration configuration, ITextResource textResource)
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Missing XML documentation of the textResource parameter

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.ViewEngines.Razor/TextResourceFinder.cs
@@ -0,0 +1,31 @@
+namespace Nancy.ViewEngines.Razor
+{
+ using System.Dynamic;
+
+ /// <summary>
+ /// Returns text from an implemented ITextResource
+ /// </summary>
+ public class TextResourceFinder : DynamicObject
+ {
+ private readonly ITextResource textResource;
+ private readonly NancyContext context;
+
+ public TextResourceFinder(ITextResource textResource, NancyContext context)
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Missing XML comment of ctor and parameters

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy.ViewEngines.Razor/TextResourceFinder.cs
+ {
+ private readonly ITextResource textResource;
+ private readonly NancyContext context;
+
+ public TextResourceFinder(ITextResource textResource, NancyContext context)
+ {
+ this.textResource = textResource;
+ this.context = context;
+ }
+
+ /// <summary>
+ /// Finds text resource
+ /// </summary>
+ /// <param name="binder">GetMemberBinder with dynamic text key</param>
+ /// <param name="result">Text item</param>
+ /// <returns></returns>
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Returns what? =)
I also think there needs to be some clarification that result will have its value fetched from the ITextResource and as such, the behavior for a non-existing value if defined by the implementation and not the TextResourceFinder itself

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy/ViewEngines/DefaultTextResource.cs
@@ -0,0 +1,36 @@
+namespace Nancy.ViewEngines
+{
+ using System;
+ using System.Linq;
+ using System.Reflection;
+ using System.Resources;
+
+ public class DefaultTextResource : ITextResource
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Missing XML comments of type and it's members

@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Also think that this should be called something like ResourceBasedTextResource or similar, to communicate that it will pick stuff up from resource files. There might need to be a remark tag that describes the naming convention(s) that are used

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy/ViewEngines/DefaultTextResource.cs
+ {
+ var assemblies = AppDomain.CurrentDomain.GetAssemblies();
+
+ culturedAssembly = assemblies.FirstOrDefault(x => x.GetManifestResourceNames().Any(y => y.Contains(".Resources.Text")));
+ if (culturedAssembly != null)
+ {
+ resourceManager = new ResourceManager(culturedAssembly.GetName().Name + ".Resources.Text",
+ culturedAssembly);
+ }
+ }
+
+ public string this[string key, NancyContext context]
+ {
+ get
+ {
+ if (resourceManager == null)
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Single-line if-statements should be wrapped in curly braces

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy/ViewEngines/DefaultTextResource.cs
+ culturedAssembly = assemblies.FirstOrDefault(x => x.GetManifestResourceNames().Any(y => y.Contains(".Resources.Text")));
+ if (culturedAssembly != null)
+ {
+ resourceManager = new ResourceManager(culturedAssembly.GetName().Name + ".Resources.Text",
+ culturedAssembly);
+ }
+ }
+
+ public string this[string key, NancyContext context]
+ {
+ get
+ {
+ if (resourceManager == null)
+ return null;
+
+ return resourceManager.GetString(key, context.Culture);
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

What's the behavior of there's no match? I think this needs to be documented

@thecodejunkie thecodejunkie commented on an outdated diff Dec 27, 2012
src/Nancy/ViewEngines/ITextResource.cs
@@ -0,0 +1,7 @@
+namespace Nancy.ViewEngines
+{
+ public interface ITextResource
@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Missing XML comments of interface and members

@thecodejunkie
thecodejunkie Dec 27, 2012 Member

Not sure that the ITextResource and DefaultTextResource (or what ever it ends up being called) should be stored in the ViewEngines folder. It's not view engine specific and could be used for other stuff. Maybe something like a Nancy.Localization namespace might be a better fit?

@thecodejunkie thecodejunkie merged commit f5c7b0f into NancyFx:master Dec 29, 2012
@thecodejunkie
Member

I'm doing to go out on a limb here and say that you didn't run all tests locally ;)

Nancy.Testing.Tests.ConfigurableBootstrapperFixture.Should_provide_configuration_for_all_base_properties Types missing from configurable versions: DefaultTextResource at Nancy.Testing.Tests.ConfigurableBootstrapperFixture.Should_provide_configuration_for_all_base_properties() in d:\Builds\Nancy\src\Nancy.Testing.Tests\ConfigurableBootstrapperFixture.cs:line 108

@jchannon
Member

Shit, my bad :(

On 29 December 2012 13:43, Andreas Håkansson notifications@github.comwrote:

I'm doing to go out on a limb here and say that you didn't run all tests
locally ;)

Nancy.Testing.Tests.ConfigurableBootstrapperFixture.Should_provide_configuration_for_all_base_properties Types missing from configurable versions: DefaultTextResource at Nancy.Testing.Tests.ConfigurableBootstrapperFixture.Should_provide_configuration_for_all_base_properties() in d:\Builds\Nancy\src\Nancy.Testing.Tests\ConfigurableBootstrapperFixture.cs:line 108


Reply to this email directly or view it on GitHubhttps://github.com/NancyFx/Nancy/pull/861#issuecomment-11752750.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment