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

Some of the functions provided here should be filters to respect the Twig semantic #15

Open
stof opened this issue Oct 7, 2015 · 4 comments

Comments

@stof
Copy link
Contributor

stof commented Oct 7, 2015

tourl, toHtml and t at least should be filters rather than functions.

Note that it is possible to keep the function but deprecate it, to avoid breaking BC.

@moufmouf
Copy link
Member

moufmouf commented Oct 7, 2015

I understand for toUrl but I'm not sure about t and toHtml.

t can take several parameters (the translation key + substitution parameters), so I think I need it as a function, as filters do not take parameters (unless I'm wrong).
toHtml takes an object that has a toHtml method has a parameter. It feels weird to make it a filter as the input is not really some text.

However, I was not aware of the possibility to deprecate filters. That's really good news as at least half the functions here have been added a bit carelessly... :s I'm gonna deprecate a bunch of them right now!

moufmouf added a commit to moufmouf/html.renderer.twig-extensions that referenced this issue Oct 7, 2015
@stof
Copy link
Contributor Author

stof commented Oct 7, 2015

@moufmouf the translation key would be the filtered value, and substitution parameters would be filter parameters. Twig filters are taking parameters.

@moufmouf
Copy link
Member

moufmouf commented Oct 7, 2015

Ok, I was not aware that filters could accept parameters (it might be obvious, but there is no sample in the Twig doc: http://twig.sensiolabs.org/doc/advanced.html#filters

I'll change the t implementation then!

@stof
Copy link
Contributor Author

stof commented Oct 7, 2015

One of the examples in the doc is using them:

{{ now|date('d/m/Y') }}

moufmouf added a commit to moufmouf/html.renderer.twig-extensions that referenced this issue Oct 7, 2015
moufmouf added a commit to moufmouf/html.renderer.twig-extensions that referenced this issue Oct 7, 2015
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