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

Removed needless borrow of range #452

Merged
merged 2 commits into from
Feb 22, 2021
Merged

Removed needless borrow of range #452

merged 2 commits into from
Feb 22, 2021

Conversation

vallentin
Copy link
Collaborator

@vallentin vallentin commented Feb 22, 2021

Before .zip(10..20) would result in .zip(&(10..20)) which is not valid.

  • Removed needless borrow of range
  • Added test case for (0..10).zip(10..20).zip(20..30)

When adding the test case, I realized that Askama cannot parse {% for ((i, j), k) in (0..10).zip(10..20).zip(20..30) %}, specifically the for loop pattern ((i, j), k).

Reference #451

@vallentin vallentin added the bug label Feb 22, 2021
@vallentin vallentin requested a review from djc February 22, 2021 02:02
@vallentin vallentin mentioned this pull request Feb 22, 2021
@djc djc merged commit 7609f00 into main Feb 22, 2021
@delete-merged-branch delete-merged-branch bot deleted the range branch February 22, 2021 12:05
@djc
Copy link
Collaborator

djc commented Feb 22, 2021

Thanks! I think there are likely cases where this is wrong (given that Range is unfortunately not Copy), but it likely makes sense for the common cases.

@vallentin
Copy link
Collaborator Author

Yeah true. But I really hope people aren't doing that kind of logic inside templates 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants