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

Implement for-else #518

Merged
merged 3 commits into from Nov 11, 2021
Merged

Implement for-else #518

merged 3 commits into from Nov 11, 2021

Conversation

Kijewski
Copy link
Collaborator

This PR implements for-else statements like in Jinja. They make it easy
to print an alternative message if the loop iterator was empty. E.g.

{% for result in result %}
  <li>{{ result }}</li>
{% else %}
  <li><em>no results</em></li>
{% endfor %}

@djc
Copy link
Owner

djc commented Jul 19, 2021

I'm not sure I'm a fan of this, since Rust does not have for-else itself. In most cases, you could probably handle this by calling is_empty() on the source of the iterator (for example, for slice or Vec). Any thoughts?

@Kijewski
Copy link
Collaborator Author

I implemented for-else like Jinja does, which is not equivalent to Python's semantics.

In Python the else block is executed unless a break statement was hit. That's trivially true for empty ranges, but might also be the case for non-empty ranges.

In Jinja the else block is only executed if the inner block was never entered. I guess I should implement for … in … if …, too, then the feature is vastly more useful, too.

@Kijewski Kijewski force-pushed the pr-for-else branch 2 times, most recently from de69985 to 1c22e3f Compare July 30, 2021 22:06
@Kijewski
Copy link
Collaborator Author

I added for … in … if …. I don't quite like how the source for that particular feature looks like, but I don't know to to make it "prettier" right now.

@djc
Copy link
Owner

djc commented Aug 5, 2021

Do you actually have use cases for this syntax, or are you just aiming for feature parity with Jinja?

@vallentin
Copy link
Collaborator

vallentin commented Aug 5, 2021

For what it's worth, I technically have a use case for both :)

  1. For for ... in ... if ... that would be nice (for me) when generating filtered listings. As they always need that extra {% if ... %}, which then causes all the inner code to be indented even further.
  2. For the for else that would be nice (for me) also when generating listings and wanting and "else" case. Specifically for handling search results.

On one hand, I always think less code is nice. On the other hand, it only requires 1 extra set of {% if ... %} + {% endif %} (and {% else %} for the first one.) So it isn't specifically "needed", but it would be nice, but again, that's just my opinion.

askama_shared/src/generator.rs Outdated Show resolved Hide resolved
askama_shared/src/generator.rs Outdated Show resolved Hide resolved
askama_shared/src/parser.rs Outdated Show resolved Hide resolved
askama_shared/src/parser.rs Outdated Show resolved Hide resolved
testing/tests/loop_else.rs Outdated Show resolved Hide resolved
askama_shared/src/generator.rs Outdated Show resolved Hide resolved
Copy link
Owner

@djc djc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, sorry for the extremely slow review cycle. I've been conflicted about whether to accept this, but in the end I think it makes sense. Some minor style nits to discuss and then hopefully we can get this merged.

askama_shared/src/parser.rs Outdated Show resolved Hide resolved
askama_shared/src/parser.rs Outdated Show resolved Hide resolved
askama_shared/src/generator.rs Outdated Show resolved Hide resolved
@djc
Copy link
Owner

djc commented Nov 11, 2021

Ah, this has a conflict now. Otherwise looking good; for bonus points, also add some docs?

This PR implements for-else statements like in Jinja. They make it easy
to print an alternative message if the loop iterator was empty. E.g.

```rs
{% for result in result %}
  <li>{{ result }}</li>
{% else %}
  <li><em>no results</em></li>
{% endfor %}
```
@djc djc merged commit f6b79cd into djc:main Nov 11, 2021
@Kijewski
Copy link
Collaborator Author

Thank you! I'll have a look at the book. I guess the changes of some of the older PRs need to be mentioned, too.

@Kijewski Kijewski deleted the pr-for-else branch November 11, 2021 14:37
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