Skip to content

Fix unique filter implementation#417

Merged
Kijewski merged 1 commit intoaskama-rs:masterfrom
GuillaumeGomez:fix-unique
Apr 22, 2025
Merged

Fix unique filter implementation#417
Kijewski merged 1 commit intoaskama-rs:masterfrom
GuillaumeGomez:fix-unique

Conversation

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator

I also removed the "code-in-doc" cfg from doctests to be sure that they are run all the time.

Also fixed the wrong implementation of the unique filter (it was supposed to return a Result).

@GuillaumeGomez GuillaumeGomez requested a review from Kijewski April 22, 2025 13:26
Copy link
Copy Markdown
Member

@Kijewski Kijewski left a comment

Choose a reason for hiding this comment

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

#[cfg(feature = "code-in-doc")] { is needed for tests that use ```jinja block

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

Why using jinja blocks btw?

@Kijewski
Copy link
Copy Markdown
Member

Because we had no filter tests whatsoever, and when they were added, they were added using this format.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

Do you mind if I migrate them to source instead?

@Kijewski
Copy link
Copy Markdown
Member

Yes, I do. The problem is not even related to code-in-doc, and the fix is readily available in #416.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

My issue with the current doctests is that they require a feature to run, which is not great. I won't force if you prefer it this way but I'd rather having them run all the time if we run tests, even if the code-in-doc feature is not available.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

Just to confirm: do you want to keep them like this before I make the change? 😄

@Kijewski
Copy link
Copy Markdown
Member

Just to confirm: do you want to keep them like this before I make the change? 😄

Yes, I want to keep them as they are. IMHO it's much more readable docs like this, without the mental overhead of counting backslashes \ or hashes # inside and around strings.

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

A debate for another day then. Reverting these changes then!

@GuillaumeGomez
Copy link
Copy Markdown
Collaborator Author

Removed other changes.

@Kijewski Kijewski merged commit a5b43c0 into askama-rs:master Apr 22, 2025
41 checks passed
@GuillaumeGomez GuillaumeGomez deleted the fix-unique branch April 23, 2025 12:01
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.

2 participants