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: Parser - Equal key name replace conflict #9246

Merged
merged 4 commits into from
Nov 3, 2024

Conversation

CosDiabos
Copy link
Contributor

Description
Fixes the conflict when replacing View Parser {variables} in the provided data with the same key name, even when on different levels.

This fix focus on parse() method, in which the return of parsePair and parseSingle methods are stored independently , then merged together (Pairs with Single), and finally replaced. With this approach, the replacement happens from the Pairs to Single and the output is as expected.

More details and examples in issue #9245

Fixes #9245

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@CosDiabos CosDiabos changed the title Fix: Parser - Equal key name replace conflict Fix Parser - Equal key name replace conflict Oct 30, 2024
@samsonasik
Copy link
Member

Run Rector and cs fix:

vendor/bin/rector && composer cs-fix

also, a test case seems needed.

@ddevsr ddevsr added the tests needed Pull requests that need tests label Oct 31, 2024
@samsonasik
Copy link
Member

You can rename PR title with fix: prefix

@CosDiabos CosDiabos changed the title Fix Parser - Equal key name replace conflict fix: Parser - Equal key name replace conflict Nov 1, 2024
@datamweb
Copy link
Contributor

datamweb commented Nov 1, 2024

@CosDiabos To ensure the security and authenticity of your commits, please sign all of them with GPG. This shows others that the commits were genuinely made by you and can be trusted.

For a detailed guide on signing commits with GPG, please refer to gpg-signing-old-commits.

@CosDiabos
Copy link
Contributor Author

Thank you all for your help!

@samsonasik samsonasik removed the tests needed Pull requests that need tests label Nov 2, 2024
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

@CosDiabos Looks great! We need one last thing - a changelog entry.

Please add it under the Bugs Fixed section, here: https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/changelogs/v4.5.6.rst

Something like:

Parser: Fixed a bug that caused equal key names to be replaced by the key defined first.

Thanks.

@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 2, 2024
@michalsn michalsn merged commit 3d0f9f2 into codeigniter4:develop Nov 3, 2024
42 checks passed
@michalsn
Copy link
Member

michalsn commented Nov 3, 2024

@CosDiabos Thank you for your first contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Parser - Equal key name replace conflict
6 participants