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 having macro variables starting with an underscore #4475

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

fabpot
Copy link
Contributor

@fabpot fabpot commented Nov 25, 2024

Closes #4452

@stof
Copy link
Member

stof commented Nov 25, 2024

What if the original name starts and ends with underscores ?

@stof
Copy link
Member

stof commented Nov 25, 2024

What was the reason for the trimming anyway ?

@fabpot
Copy link
Contributor Author

fabpot commented Nov 26, 2024

What was the reason for the trimming anyway ?

We allow people to use variables like this, context, ... So, we need to add a prefix on the PHP side, and trim it when using it in the Twig code.

@fabpot
Copy link
Contributor Author

fabpot commented Nov 26, 2024

What if the original name starts and ends with underscores ?

Fixed

@fabpot fabpot merged commit c384fb4 into twigphp:3.x Nov 26, 2024
49 of 50 checks passed
@fabpot fabpot deleted the regression-fix branch November 26, 2024 20:41
@alexislefebvre
Copy link

"\u{035C}" is used in several places, which could lead to unexpected behaviours if one instance is updated but not the others. What do you think about using a constant to define and reuse it? I could open a PR if you agree to this proposal.

@fabpot
Copy link
Contributor Author

fabpot commented Dec 1, 2024

"\u{035C}" is used in several places, which could lead to unexpected behaviours if one instance is updated but not the others. What do you think about using a constant to define and reuse it? I could open a PR if you agree to this proposal.

That won't happen and in any case, we have tests, so I don't think we need a constant here.

njoubert-cleverage added a commit to cleverage/ui-process-bundle that referenced this pull request Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Macro arguments using leading/tailing underscore (BC introduced in v3.15)
4 participants