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

Reconsider returning the configured undefined for else-less ifs #2050

Open
adrian-zon opened this issue Nov 25, 2024 · 3 comments
Open

Reconsider returning the configured undefined for else-less ifs #2050

adrian-zon opened this issue Nov 25, 2024 · 3 comments

Comments

@adrian-zon
Copy link

(I would have just commented on #710 but it's locked)

The current behavior of always returning stock Undefined, regardless of the configured undefined class, is highly surprising. I understand the wish to be able to write {{ 'foo' if false }}, but we use a lot of if clauses for setting variables or function parameters, and I would not have expected to find Undefined instances when having undefined set to another class.

I suggest to either return a different value depending on the context (this is probably difficult or at least annoying to implement), or always return a defined value like None.

@davidism
Copy link
Member

@ThiefMaster can you comment/decide on this?

@ThiefMaster
Copy link
Member

ThiefMaster commented Dec 20, 2024

See #1079 for details. Initially I had implemented it to be configurable, but then we decided to keep it simple and use standard Undefined for this particular case.

Can you come up with an actual example where this is a problem for you?

I think {{ 'foo' if false }} should never be a problem, because getting a different Undefined there would not give you any benefits.

Are you doing something like {% set foo = 'foo' if false %}? If yes, I'm not even sure if that should be allowed at all - the shortcut makes sense for display purposes bug in assignments (or function calls) it looks like something that's pretty much always not intended.

@davidism
Copy link
Member

I do like the explanation in the changelog for that PR:

omitting the else clause is a valid shortcut and not actually an undefined variable that should cause an error

Hard to believe that was all the way back in 2019! I'm personally fine with the current behavior. Perhaps we need to add more explanation to the docs?

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

No branches or pull requests

3 participants