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

Filters to implement #46

Closed
Keats opened this Issue Aug 25, 2016 · 23 comments

Comments

7 participants
@Keats
Copy link
Owner

Keats commented Aug 25, 2016

Intro

Follow the examples in filters/string.rs with tests.
Need to add all common and expected filters from Jinja2 and Django.
Feedback on the API wanted as well!
Each filter should have tests for all the possible options.
Tera filters can only be applied to identifers and their arguments have to be named.

Strings

Numbers

Arrays

Common

Those should be in filters/mod.rs or in a filters/common.rs file.

Dates

Date filters will wait until Tera has its own serialization instead of using JSON


Note: I left out some filters that I don't really the use for like ljust in Django, but happy to include more than the currently listed if they are needed. I also probably forgot some

@Peternator7

This comment has been minimized.

Copy link
Contributor

Peternator7 commented Sep 1, 2016

Hey I saw this on this-week-in-rust and thought I might give helping out a shot. I just put in a PR for first,last, and join on the array filters. Let me know if you have any feedback, and if they're okay, I might be able to help with a few others!

@orhanbalci

This comment has been minimized.

Copy link
Contributor

orhanbalci commented Sep 7, 2016

@Keats Should title filter respect whitespaces? Documentation does not state anything about whitespaces.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Sep 7, 2016

afaik it's something like split on whitespace, uppercase first char of each and join them with " "

Jinja impl: https://github.com/pallets/jinja/blob/8a49e066af3d88d1edb34aa1680b668c959dfcf7/jinja2/filters.py#L183-L190
Jinja tests: https://github.com/pallets/jinja/blob/f4092239019d9a9130dd84ef6278e58061d6bb93/tests/test_filters.py#L201-L223

(Jinja is a bit smarter since it takes into account thinks like <, { that are not letters)

@orhanbalci

This comment has been minimized.

Copy link
Contributor

orhanbalci commented Sep 7, 2016

@Keats For escape filter can we use an external crate like https://github.com/veddan/rust-htmlescape

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Sep 7, 2016

Yeah I've seen of couple on crates.io, need to see which one would be the better fit

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Sep 27, 2016

I just released 0.2.0 which contains all the filters done so far, thanks @orhanbalci @foophoof @Peternator7 !

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Nov 18, 2016

Added escape_js and escape_sql to the list, tempted to rename escape to escape_html once autoescape is in
Note that I'm not super convinced on escape_sql yet

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Dec 19, 2016

Was looking at docs and IMO the pluralize filter is kind of confusing and not easily generalisable to multiple languages. I personally think it'd be best to force people to do if statements, because the code is more clear to me to see:

{% if num == 1 %}
    1 message
{% else %}
    {{ num }} messages
{% endif %}

Rather than:

{{ num }} message{{ num | pluralize }}

Especially considering the number of irregular plurals for which this won't work even in English.

Of course, this is just my opinion, but I think that Django having this filter isn't worth adding it for Tera. That's my 2¢. :)

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Dec 19, 2016

Another comment I have on sort is that it should be split into strsort and numsort where strsort sorts lexically and numsort sorts by a version-based sort (i.e. 12.34.56 > 1.2.3) and then a lexcal sort for any non-numeric characters. I feel like this would be a lot clearer than just a sort.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Dec 19, 2016

I agree for the pluralize, it's probably going to get changed when/if I have time for i18n.

For sort, I'm still unsure whether I want to add it at all. The only sort I can see myself doing is a string sort, anything else I'd do it in Rust itself I think

@gyscos

This comment has been minimized.

Copy link

gyscos commented Dec 21, 2016

Adding the "precision" parameter for the round filter would be nice.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Dec 22, 2016

@gyscos agreed, opened #99 to track that. PR welcome!

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Dec 24, 2016

Added precision, it will be in 0.6

@DemiMarie

This comment has been minimized.

Copy link

DemiMarie commented Jun 20, 2017

It would be awesome to have an HTML sanitizing filter (only allowed in content, not in attributes) that allows only a whitelisted safe subset of HTML (for the given context).

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Jun 20, 2017

I'm rather against escape_sql because it encourages client-side query sanitisation which seems bad to me.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Jun 21, 2017

@clarcharr I added the escape_sql after seeing https://github.com/hashedin/jinjasql but I guess we can just replace the variables by ? or whatever pg/mysql expects for prepared statements.

@DemiMarie how would you know the context? I assume you mean for example li only allowed in ul? If there is a HTML sanitization lib in Rust like https://pypi.python.org/pypi/bleach, it should be easy to write your own but I don't think it would be possible for Tera to know the context

@DemiMarie

This comment has been minimized.

Copy link

DemiMarie commented Jun 21, 2017

@Keats Such a library does exist (https://crates.io/crates/ammonia). The context would need to be determined by parsing the template.

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Jun 23, 2017

But then it requires Tera to parse invalid/incomplete HTML (and CSS, JSON etc from #190) which doesn't seem achievable imo

@DemiMarie

This comment has been minimized.

Copy link

DemiMarie commented Jul 3, 2017

@Keats Servo's parsers are browser-grade and are available as libraries.

@batisteo

This comment has been minimized.

Copy link

batisteo commented Oct 10, 2017

Trying to access or render a variable that doesn't exist will result in an error.

I’d like to see the default filter (and maybe default_if_none).

{{ lang | default(value="en") }}

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Oct 11, 2017

I believe that should be an easy one to add and would be a "fake" filter, otherwise the expression would fail with lang not being defined if it's not in the context.
If anyone wants to add it, please do it in the 0.11 branch or I'll add it myself next week or so

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Nov 21, 2017

@batisteo I've added a default in the 0.11 branch which is basically the default_if_none from Django. I don't really see the usecase of default over default_if_none so if you have some examples, I'll take them!

@Keats

This comment has been minimized.

Copy link
Owner

Keats commented Nov 24, 2017

Still missing some like truncate but this issue has served its purpose!

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