Skip to content
This repository has been archived by the owner on Jun 10, 2019. It is now read-only.

More changes found with updated regex #21

Closed
wants to merge 2 commits into from
Closed

More changes found with updated regex #21

wants to merge 2 commits into from

Conversation

benjamw
Copy link
Contributor

@benjamw benjamw commented Mar 3, 2016

Using this page (http://php.net/manual/en/migration70.incompatible.php#migration70.incompatible.variable-handling),
I generated a more in-depth regex to search for variable names that might cause issues.

That regex is here:

(?:->\s*\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\s*\[.+\](?:\s*\(.*\))?|::\s*\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\s*\[.+\]\s*\(.*\)|\$\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*\s*(?:\[|\())

There was an additional file found in downloader/lib/Mage/Connect/Packager.php that requires the same changes as the ones in app/code/local/Mage/Connect/Packager.php which overrides lib/Mage/Connect/Packager.php. Because I am uncertain how to override that file, I was unable to include it in this pull request. But it should still be noted and/or added in the future.

NOTE: there is also a file at /downloader/lib/Mage/Connect/Packager.php
that needs the same change that cannot be easily overridden by any
means that I am aware of
@benjamw
Copy link
Contributor Author

benjamw commented Mar 3, 2016

There are numerous more (230 more) instances of $$var in Magento CE, but because they have no array indices or function calls, they can be ignored.

@benjamw
Copy link
Contributor Author

benjamw commented Mar 3, 2016

The other pull request I created (#22) has these changes as well. So you may want to just merge that one and disregard this one if that's the way you want to go. There's a comment on that other pull request regarding this as well. Sorry about that.

@icurdinj
Copy link
Contributor

icurdinj commented Mar 7, 2016

Disregarding this pull request, as recommended by author.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants