-
-
Notifications
You must be signed in to change notification settings - Fork 223
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 destructoring nested tuples on "let" and "for" statement target #506
Conversation
If this PR is accepted, my next two PRs will
|
Nice 🎉 Didn't look at the diffs yet, but from the title I assume it doesn't apply for e.g. |
"for … in" uses the new parsing function: https://github.com/djc/askama/pull/506/files#diff-48297b9b3a3f70c8521bedbaa0601f9e2b405eaa7a99fbffe0d9131e3207e478L904-L907 I'll add a test for nested tuples in for-loops. |
So, that's a yes. Nice 🎉 It's much appreciated, that you've arrived and implemented all these nice to have features :) |
Hehe, you're welcome. Well, the library is well written, you guys answer fast, so it's quite easy to contribute. Please have a look at the added test if I understood #451 correctly. |
That's probably mostly due to luck + all of us being in the same timezone.
Yes, that looks great. Optionally, a simple |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a few comments :)
I applied your suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scond commit looks great, but I can't quite make sense of what's going on with the first one. Left a bunch more specific questions/suggestions, below.
I split the 1st commit into 6 commits. :-/ → #510 |
Rebased on #510. |
Rebased on main. I could not reproduce the error in askama_rocket at home. |
Yeah, I think the askama_rocket issue was a recent nightly regression. I think it has already been fixed, so hopefully it will disappear soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This PR implements the parsing of nested tuples for the target of
{% let %}
statements. E.g.