2.0: Reintroduce Value::from_iterator about minijinja HOT 5 CLOSED

Perhaps you saw my note, but to me Value::from_iterator() seemed incredibly problematic in that it was a single-use value. If I wrote for x in some_iter { ... } and then wrote for x in some_iter { ... } again, I would never expect for the second iteration to be completely empty. Or perhaps it didn't work this way and the code just read this way?

In any case, my take-away was that we don't need to store iterators themselves, you need to store the thing that produces the iterator. This is the Value::from_object_iter() constructor I added.

from minijinja.

Perhaps you saw my note, but to me Value::from_iterator() seemed incredibly problematic in that it was a single-use value. If I wrote for x in some_iter { ... } and then wrote for x in some_iter { ... } again, I would never expect for the second iteration to be completely empty. Or perhaps it didn't work this way and the code just read this way?

Iterators are the only way in which you can stream endless templates. See #416 for the motivation.

I was thinking originally even when writing the changes to introduce iterators to only allow "factories" to be passed to the context which create the iterator and the iterator is hidden, but then you cannot use any of the filters such as |list without introducing yet another problem.

from minijinja.

Was added in #461

from minijinja.

I was thinking originally even when writing the changes to introduce iterators to only allow "factories" to be passed to the context which create the iterator and the iterator is hidden, but then you cannot use any of the filters such as |list without introducing yet another problem.

What problem does that introduce?

I really feel that the iterator objects being single-use is a huge foot-gun that needs to be resolved. I would never expect that functionality from any value.

from minijinja.

What problem does that introduce?

The original plan I had (pre introducing of streaming iterators) was a bit different. There the idea was that the iterator is created exclusively as part of the {% for %} loop and to hide it within that loop (completely hidden iterator). That's why |list and other operators did not work at all. They would just error out saying that an iterator is not something that can be iterated over. The iterator when held in a Value was thus non extractable by filters.

As for iterators and footguns: they are sort of. When you want to stream content via the template engine you cannot "restart" without refactoring all of what you have on the Rust side into a factory function that can trigger that stream more than once. The benefit of this is not entirely unclear. At that point it seems a lot more sense to me ask the user to say {% set iter = start_streaming() %} rather than magically creating the stream from scratch every time someone starts iterating.

However I do agree that iterators made with Value::from_iterator are not a great idea for the general use and they should be discouraged compared to restart-able iterators as created by Value::from_object_iter.

I will open a separate issue for the iterator question.

from minijinja.