Skip to content

Conversation

@Kijewski
Copy link
Member

@Kijewski Kijewski commented Oct 17, 2024

Before this PR, the whole condition in {% if cond %} got wrapped in as_bool(&cond). With is PR, the individual terms in a binary operation && or ||, or unary operation ! get wrapped in as_bool().

Resolves #201.

Before this PR, the whole condition in `{% if cond %}` got wrapped in
`as_bool(&cond)`. With is PR, the individual terms in a binary operation
`&&` or `||`, or unary operation `!` get wrapped in `as_bool()`.
@GuillaumeGomez
Copy link
Collaborator

In travel currently. Will take a look when I'm back next week. :)

@Kijewski
Copy link
Member Author

No problem at all! Enjoy your trip, I wish you good weather! :) 🌞

compare(
"{% if let Some(query) = s && !query.is_empty() %}{{query}}{% endif %}",
r"if let Some(query,) = &self.s && !query.is_empty() {
r"if let Some(query,) = &self.s && !rinja::helpers::as_bool(&(query.is_empty())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In such cases, I'm not sure it's an improvement since the ! operator tells us that it's already a boolean.

"{% if y == 3 || (x == 12 || true) %}{{x}}{% endif %}",
r"if *(&(self.y == 3 || (self.x == 12 || true)) as &rinja::helpers::core::primitive::bool) {
r"
if rinja::helpers::as_bool(&(self.y == 3))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same in this case, it's a comparison so we already know for sure it's a boolean.

@GuillaumeGomez
Copy link
Collaborator

It's work travel so the weather doesn't really matter. 😆

@Kijewski
Copy link
Member Author

Kijewski commented Oct 20, 2024

! is implemented for bool and &bool, but not &&bool, &mut bool, Rc<bool>, …

rinja::helpers::as_bool() follows all references to get the right type, so the user doesn't have to know or care what the actual type is, only that you can add some amount of * in front of it to get a bool. The previous *((&value) as &bool) trick did this halfway: You still have to care what kind of reference wrapper it is, and e.g. Rc<_> won't work.

Also, as seen in redlib-org/redlib#290 (comment), the current implementation can be confusing. In the condition post_should_be_blurred && prefs.unblur_on_hover != "on", why do I need to add an * in front of the variable post_should_be_blurred?

@GuillaumeGomez
Copy link
Collaborator

Oh I see, makes sense. Thanks for the extra explanation. :)

@GuillaumeGomez GuillaumeGomez merged commit 35607ee into askama-rs:master Oct 20, 2024
34 checks passed
@Kijewski Kijewski deleted the pr-boolify-2 branch October 21, 2024 03:50
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.

In an if condition with conjuctions && and disjunctions ||, should each term be "boolified"?

2 participants