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

fix f-string syntax error in code generation #1852

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

sisp
Copy link
Contributor

@sisp sisp commented May 26, 2023

I've fixed a bug that caused an f-string syntax error in the template compilation when a template, that imports a macro, contains curly braces in its name. The bug was caused by the code-generation of an f-string in which the template name is inserted, and when the template name contains curly braces and the generated code gets executed, Python treats the curly braces as f-string braces. I've changed the code-generation to construct the string differently, such that the affected substring is not an f-string. Instead, the different substrings are concatenated using the + operator.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@sisp
Copy link
Contributor Author

sisp commented May 26, 2023

I don't think any of the errors were introduced by this PR.

@sisp
Copy link
Contributor Author

sisp commented Dec 19, 2023

I've rebased this PR onto the latest state of the 3.1.x branch. All tests are passing except pre-commit.ci - pr, but the reported errors originate from 3.1.x and weren't introduced by this PR.

@davidism (or any other maintainer) – may I ask you for a review? 🙏

@sisp sisp force-pushed the fix/fstring-syntax-error-in-codegen branch from 65e00c4 to a442d84 Compare August 28, 2024 10:52
@sisp
Copy link
Contributor Author

sisp commented Aug 28, 2024

I've performed another rebase onto the latest state of the 3.1.x branch. All tests are passing now.

@sisp sisp force-pushed the fix/fstring-syntax-error-in-codegen branch from a442d84 to cf62e59 Compare October 11, 2024 08:33
@davidism davidism added this to the 3.1.5 milestone Dec 18, 2024
@davidism davidism force-pushed the fix/fstring-syntax-error-in-codegen branch 2 times, most recently from 2a6be86 to bb490dd Compare December 20, 2024 02:06
@davidism davidism force-pushed the fix/fstring-syntax-error-in-codegen branch from bb490dd to 56a7246 Compare December 20, 2024 02:09
@davidism
Copy link
Member

davidism commented Dec 20, 2024

I've refactored the fix so that the curly braces are escaped rather than trying to keep track of nested quote levels and string concatenation. I also fixed the test, as it didn't seem to trigger an error regardless of the fix. Also added comments explaining the code.

@davidism davidism merged commit 767b236 into pallets:stable Dec 20, 2024
12 checks passed
@sisp sisp deleted the fix/fstring-syntax-error-in-codegen branch December 21, 2024 19:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants