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

add take_while, take_until filters #535

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sooda
Copy link

@sooda sooda commented Jul 21, 2020

Add a take_while filter that mirrors Iterator::take_while in a similar fashion to the "filter" filter, comparing attributes against a value or against null if no value is given.

Add also take_until to do the reverse, producing elements that do not match a given value.

take_until in particular is very useful to produce blog article listings and to find the older and newer posts than the current one; implementing the same with template control structures would be rather cumbersome, and these two filters are fairly general so they could have many other uses.

Jinja2 doesn't seem to have matching filters.

Add a take_while filter that mirrors Iterator::take_while in a similar
fashion to the "filter" filter, comparing attributes against a value or
against null if no value is given.

Add also take_until to do the reverse, producing elements that do not
match a given value.
@Keats
Copy link
Owner

Keats commented Jul 22, 2020

The usecases in the docs look like something that should be done before rendering the templates no? Having all the posts when rendering each post for example is going to be very slow and it would be better for each post to have a previous/next field.
The cargo example is also probably wrong? filter would be more appropriate there if something like Finland, Finland, France, France happens for some reasons, the order doesn't matter?

@sooda
Copy link
Author

sooda commented Jul 22, 2020

Thanks for the quick response! I don't mind keeping these in my site engine if the filters aren't general enough; I'm hoping to eventually see more usecases than just the few examples (Iter::take_while got into the standard library, so other people have concluded how general that is). Would having just the posts' metadata for rendering each post be very heavy? Probably not useful to have all site content available everywhere, but it'd be nice to render links to neighboring articles. (I'll soon have a ~100-page blog converted to benchmark with.)

My intention would be to have the site engine extremely site-agnostic and "pure" simply for experimental reasons, although the prev/next links could be so common that they could be some sorts of extensions.

The point in the cargo example is that the orders would be on some sort of conveyor belt and the next ship would give harbor space to the next one once it runs out of a sequential feed. Maybe that's not obvious.

A side note: it would be interesting to be able to filter and take_while based on not just equal values but based on entire expressions, or at least a tester, similarly to how Jinja does with select().

@Keats
Copy link
Owner

Keats commented Jul 23, 2020

Would having just the posts' metadata for rendering each post be very heavy? Probably not useful to have all site content available everywhere, but it'd be nice to render links to neighboring articles. (I'll soon have a ~100-page blog converted to benchmark with.)

I think at that scale it's fine. For Zola, I was benchmarking with 10k-100k pages site and that's not working at that scale. Zola just passes the metadata for the previous/next page to the current page that's being rendered so you can display the title/description etc as well as link to it.
If we could use references in Zola instead of having to use Value, the point will be moot and it would be much better to have everything but that's not the case :(

A side note: it would be interesting to be able to filter and take_while based on not just equal values but based on entire expressions, or at least a tester, similarly to how Jinja does with select().

I'm not against it

@sooda
Copy link
Author

sooda commented Jul 23, 2020

Getting a bit off topic, but this is interesting

I was benchmarking with 10k-100k pages site and that's not working at that scale.

Sure. My plan is to track dependencies and build the site incrementally when small changes have been done, so a "cold start" would be very rare in any case. I still want reasonable perf for a clean build though.

Zola just passes the metadata for the previous/next page to the current page that's being rendered so you can display the title/description etc as well as link to it.

Thanks, I guess it's sensible to implement such perf optimizations for common cases.

If we could use references in Zola instead of having to use Value, the point will be moot and it would be much better to have everything but that's not the case :(

This came across my mind as well. Assuming that the whole site is built, all content is in any case going to be Value'd and all. What would it take to ensure that each piece of data would be there only once? Could a context or a group of contexts internally store all values in some central map to which others would be ref'ing to, or provide an API to do this in the user side? This might be a simpler question in a GC'd language...

I'm not against it

Cool, I might be interested in looking into implementing the select() family of filters. They don't even collide with the name of filter() so that should be backwards-compatible.

@Keats
Copy link
Owner

Keats commented Jul 23, 2020

My plan is to track dependencies and build the site incrementally when small changes have been done, so a "cold start" would be very rare in any case. I still want reasonable perf for a clean build though.

You should be careful to not create Tera functions for users to use. In Zola they can grab any page from any templates, which makes incremental builds impossible short of analysing the templates AST. An example is someone having a small box on every page linking to the most recent articles. Any new page requires a full rebuild and that's only an obvious example, some would be much harder to detect.

This came across my mind as well. Assuming that the whole site is built, all content is in any case going to be Value'd and all. What would it take to ensure that each piece of data would be there only once?

It would be pretty tricky. Ideally we could use a serde format that uses Cow so we can pass a &str and it will just be a Cow::Borrowed in that case.
Maybe there's another way to do it without going through Serde at all but I'm not aware of a way to handle while keeping a nice UX.

@sooda
Copy link
Author

sooda commented Jul 23, 2020

You should be careful to not create Tera functions for users to use

Thanks, good point. Such data piping would need to track the origin of the information which wouldn't be easy to do in the general case (but likely ~doable at least conservatively). I'm planning to draw same ideas from makefiles and gcc's -M et al.

An example is someone having a small box on every page linking to the most recent articles

Indeed; design-wise this would be an user error if the user would want small iterative updates. My next personal blog style will be fairly minimal, and this is one of the reasons. Stuff to update for a blog post would be that post, the prev and next pages, category listings that the page touches, and the index page. Such dependency chains can get tricky in more complicated websites, so it wouldn't be possible to know manually with confidence which files to rebuild. (A cheat out of this would be to not consider that box a great first-class citizen and to implement it in client-side (e.g., some javascript).)

But back to the PR: do you want me to invent simpler example code that wouldn't suggest template design that can get too slow, or should we let this feature sit around for a while or perhaps abandon it?

@Keats
Copy link
Owner

Keats commented Jul 24, 2020

But back to the PR: do you want me to invent simpler example code that wouldn't suggest template design that can get too slow, or should we let this feature sit around for a while or perhaps abandon it?

Let's wait until we get a few more people asking for it.

@categulario
Copy link

something simpler that would be very useful: a take(n) filter. I suspect the same functionality can be achieved with the loop variables and break, but it would be way simpler with take(n). What do ye think?

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

Successfully merging this pull request may close these issues.

None yet

3 participants