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

Fix foreachelse #531

Closed
wants to merge 2 commits into from
Closed

Fix foreachelse #531

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 8, 2019

This Fix try to address issue #506. See commit message in cda5bb9

Try to address issue #506. After this patch the foreach logic always
count the total number of elements when the underlying structure can
easily be queried. E.g.: arrays and Countables can be queried with count()
for the total number of elements. For all iterators we do not
know the total element count in advance and calculating it is a wasteful
operation. And it can be dangerous, too: forward-iterators cannot be
rewinded and the foreach-loop will break. So with this patch the @ToTal
property will be simply set to -1 for iterators.
This has implications to other properties as well, when foreach is
used with iterarors: The @show property is
always true (even when there are no elements), and @last is always false.

This patch changes a runtime function call. So all templates that uses
foreach must be recompiled.
@wisskid
Copy link
Contributor

wisskid commented Jan 27, 2020

this doesn't fix the issue in the current master. working on another implementation.

@wisskid wisskid closed this Jan 27, 2020
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

2 participants