Localizable presenter #513
Conversation
It's a utility for localizing specific route based on route parameter or query string
| public static Func<LocalizablePresenter> BasedOnParameter(string name) | ||
| { | ||
| var presenter = new LocalizablePresenter( | ||
| context => context.Parameters.TryGetValue(name, out var value) && !string.IsNullOrEmpty(value as string) ? new CultureInfo((string)value) : null, |
There was a problem hiding this comment.
if value is not a valid culture name the statement new CultureInfo((string)value) will throw CultureNotFoundException. We should put the statement into a try block.
There was a problem hiding this comment.
Hmm, I don't think so, silently eating the error may be pretty nasty. I think that displaying Error 500 when you access non-existent culture is not that bad option. Unfortunately, in the presenter it's too late for a "not found" response, you can only do that in a middleware, but you could catch the CultureNotFound exception and rewrite it to 404 response elsewhere.
There was a problem hiding this comment.
Yes, silently eating the error is pretty nasty, but IMHO 500 Internal Server Error is worse. Error 500 should indicate that something is bad with your code. We can add culture constraint but that doesn't solve the problem. Returning 404 would be nice, but IMHO we shouldn't require for user to implement this behavior.
There was a problem hiding this comment.
Maybe I could do a redirect to a default culture. What would you think about that?
…nvalid culture is specified I also has to fix a bug DotvvmQueryCollection and add a missing Add(virtualPath, presenter) overload to the RouteTable.
CultureInfo.CurrentCulture has only getter in .NET451
| /// A DotVVM Presenter that reads culture by <see cref="getCulture" />, | ||
| /// sets the thread culture and invokes the default dotvvm presenter (obtained from IServiceProvider) | ||
| /// </summary> | ||
| public class LocalizablePresenter : IDotvvmPresenter |
There was a problem hiding this comment.
To be honest, I don't like that at all. I don't want to mess the codebase with VisualStudio hacks. The summary element is simply totally ugly and it's not required.
- By specification - the specification actually does not force any rules here, the comment just has to be valid XML. There are just recommended tags. (that also means that VS behavior is correct, but in the same way notepad is valid C# IDE)
- By all sane tools I know - Omnisharp and JetBrains rider work.
And if it does not work with any other sane IDE, it can't be a problem to write a plugin or fork and compile :P
|
Is there any way of telling DotVVM to use this presenter on all routes by default? |
|
You can use IServiceCollection to register your presenter
implementation, adding a property to configuration seems like
duplicating the DI's work. The problem is, that this presenter is going
to invoke the default one when the culture is set, which will simply
cause stack-overflow.
I think that it does not make much sense to register it globally - this
presenter is intended for per-route usage, if you want a global rule
you can use the default Asp.Net middleware.
|
|
Is there any type you can register for the default presenter? I don't think |
I don't understand what is the problem with it. This way you can register your custom presenter as a default one, or you can use it just locally, I don't see any problem. |
|
Oh, I understand - custom presenters can be registered as specific types and the default one will be registered as |
Added
LocalizablePresenterclass - a utility for localizing specific route based on route parameter or query string