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

Allow colon inside Twig.expression.type.key.brackets. #879

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

antoineveldhoven
Copy link
Contributor

Fixes #854

Not sure why the colon is needed in de regexp for Twig.expression.type.key.brackets, @willrowe can you elaborate?

If it is not needed, removing the colon in the regexp will fix this issue.

@willrowe
Copy link
Collaborator

It appears that came for #362, but the tests from that PR appear to be passing, so it may have not been necessary to alter the regex originally. Can you double-check that PR and ensure that slicing still works?

@antoineveldhoven
Copy link
Contributor Author

It appears that came for #362, but the tests from that PR appear to be passing, so it may have not been necessary to alter the regex originally. Can you double-check that PR and ensure that slicing still works?

Thanks for elaborating @willrowe !

As far as I can tell, the regexp with the colon is defined in Twig.expressions.js (line 578) Twig.expression.type.slice, so it is part of type slice and type.key.brackets are not used in this expression.

LGTM!

Copy link
Collaborator

@RobLoach RobLoach left a comment

Choose a reason for hiding this comment

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

The tests are passing. Think this is good to go, @willrowe ?

@willrowe
Copy link
Collaborator

willrowe commented Nov 6, 2023

It should be, I think @antoineveldhoven is right and the regex must have been updated by mistake in the previous PR>

@willrowe willrowe self-requested a review November 6, 2023 19:12
@willrowe willrowe added this to the v1.17.0 milestone Nov 7, 2023
@willrowe willrowe merged commit dbdaf47 into twigjs:master Nov 7, 2023
19 checks passed
@antoineveldhoven antoineveldhoven deleted the colon-array-element branch November 7, 2023 19:25
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.

accessing an array element with a colon
3 participants