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

locale #47

Closed
saleyn opened this issue Oct 23, 2012 · 5 comments
Closed

locale #47

saleyn opened this issue Oct 23, 2012 · 5 comments
Milestone

Comments

@saleyn
Copy link
Contributor

saleyn commented Oct 23, 2012

In templates I frequently need access to the CurrentLocale variable that is passed to the template's render/3 function as {locale, CurrentLocale} parameter. However, it's not available for access and can only be made available if one also passes it in the list of bound variables, which is redundant.

I can see three ways to solve it:

  1. Add artificial variable to the Bindings passed to a template behind the scene.
  2. Introduce another tag {% locale %}
  3. Introduce a special variable {{ locale }} which will resolve to current locale if the variable with the same name is not passed by the user.

I believe solution #2 is the best, though it extends the Django grammar, and if that's a problem than #3 is the best.

Upon getting your feedback, I'll submit a patch with this implementation as I believe this feature would be useful for other users of erlydtl.

Thoughts?

@evanmiller
Copy link
Contributor

In my opinion, option 2 is undesirable because it is a non-standard extension of the grammar (and not very flexible), and option 3 is undesirable because it introduces a global variable without the template author's knowledge. I am not sure what option 1 means, so I won't comment.

The redundancy of explicitly passing in the locale as a variable is not a big deal.

@saleyn
Copy link
Contributor Author

saleyn commented Oct 26, 2012

Actually option #2 is not quite so problematic. Django allows custom tags (see {% current_time %} example in https://docs.djangoproject.com/en/dev/howto/custom-template-tags), and this suggestion does fall within the scope of a custom tag syntax, so it's not really extending the Django syntax, but implementation. One semantic problem is that erlydtl doesn't properly support custom tags as it requires that custom tags arguments are mandatory:
CustomTag -> open_tag identifier Args close_tag : {tag, '$2', '$3'}.
whereas Django template syntax doesn't have such a requirement, so {% locale %} would be a perfect example of a custom tag notation in classic Django, and would be the same in erlydtl if the following parser addition were implemented:
CustomTag -> open_tag identifier close_tag : {tag, '$2'}. But even this addition toerlydtlwould be insufficient to be able to implement{% locale %}becauseCurrentLocaleis not passed to a function handling a custom tax (it only gets two parameters: Variables and Context). So there's no way to implement access to the locale within the current scope oferlydtl` and the only option is to pass it as a variable, which I believe is an unjustifiable inconvenience.

So in addition to the above if custom tag handling implementation is extended so that the compiler would check if a custom-tag handling module exports Tag/3 function, it would call: Module:Tag(Vars, CurrentLocale, Context), and otherwise it would call Module:Tag(Vars, Context), in which case {% locale %} could easily be implemented as a custom tag.

What do you think about this? It sounds to me that this would be a completely backward compatible addition that wouldn't require to hard code handling of the {% locale %} tag that could simply be implemented by `erlydtl' user as a custom tag if needed.

@evanmiller
Copy link
Contributor

I think that:

  1. As you suggest, ErlyDTL should support argument-less custom tags e.g. {% current_time %}
  2. At some point, filters and tags should be passed locale information. This will require a fair amount of planning and work. But at the end it will make ErlyDTL much more useful, since the filters and tags tend to assume American English. I think your Tag/3 proposal is a reasonable start, but I would preserve the argument order (passing in CurrentLocale as argument 3).

@saleyn
Copy link
Contributor Author

saleyn commented Oct 26, 2012

Ok, I'll submit a patch that implements it.

@evanmiller
Copy link
Contributor

As a word of warning, please consider the API change carefully. E.g., are there other things we should be passing in? TranslationFun maybe? I haven't thought about it much, but I think any change needs to be carefully considered.

@saleyn saleyn closed this as completed Nov 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants