Remove use of deprecated /e-modifier of preg_replace #3376

Closed
marc-mabe opened this Issue Jan 7, 2013 · 5 comments

Comments

Projects
None yet
3 participants
Member

marc-mabe commented Jan 7, 2013

... use preg_replace_callback instead.

There is a list of failed tests because of the deprecated message:
https://travis-ci.org/marc-mabe/zf2/jobs/4007331

Summery:

  • Zend\Filter\Word\* extends Zend\Filter\PregReplace and uses the /e-modifier.
    • This should be changed to not extend Zend\Filter\PregReplace but use preg_replace_callback directly
    • Because of BC beak this would be great for 2.1 ?
  • Zend\Ldap failes on Zend\Ldap\Converter\Converter::hex32ToAsc()

It could be possible that the /e-modifier will be used elsewhere not catched by travis tests so a manual search would be good idea.

@weierophinney weierophinney added a commit to weierophinney/zendframework that referenced this issue Jan 14, 2013

@weierophinney weierophinney [#3376] Removed usage of /e modifier with preg_replace in Zend\Filter
- Modified PregReplace to test for the "e" modifier and raise an
  exception when found
- Modified SeparatorToCamelCase to use preg_replace_callback instead of
  /e modifier
cae6998

@weierophinney weierophinney added a commit to weierophinney/zendframework that referenced this issue Jan 14, 2013

@weierophinney weierophinney [#3376] Replace PCRE /e modifier with preg_replace_callback()
- Replaced usage of preg_replace with "/e" modifier with
  preg_replace_callback() using a closure
2a74cdb

This was referenced Jan 14, 2013

@weierophinney weierophinney added a commit to weierophinney/zendframework that referenced this issue Jan 14, 2013

@weierophinney weierophinney [#3376] Added test for new behavior
- Added a test displaying that an exception is raised when passing the
  "e" modifier in a pattern
f5b463d

@weierophinney weierophinney added a commit to weierophinney/zendframework that referenced this issue Jan 15, 2013

@weierophinney weierophinney [#3376] Better fix
- The main point is that we ensure hasItems() internally calls
  internalHasItem(). As such, removing the multiple keys ensures we
  don't have issues with index order of mock objects. Test now passes on
  both 3.7.12 and 3.7.13, on all PHP versions used.
b2d307a

@weierophinney weierophinney added a commit that referenced this issue Jan 16, 2013

@weierophinney weierophinney [#3376] Better fix
- The main point is that we ensure hasItems() internally calls
  internalHasItem(). As such, removing the multiple keys ensures we
  don't have issues with index order of mock objects. Test now passes on
  both 3.7.12 and 3.7.13, on all PHP versions used.
bc36d9f
Member

marc-mabe commented Jan 16, 2013

@weierophinney Does Zend\Filter\Word\AbstractSeparator need to extend Zend\Filter\PregReplace anymore?

Owner

weierophinney commented Jan 16, 2013

I'm not sure; I didn't want to introduce more potential for regression than
necessary.

On Wednesday, January 16, 2013, Marc Bennewitz wrote:

@weierophinney https://github.com/weierophinney Does
Zend\Filter\Word\AbstractSeparator need to extend Zend\Filter\PregReplaceanymore?


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/issues/3376#issuecomment-12336846.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

Member

marc-mabe commented Jan 16, 2013

@weierophinney As I understand it's used only to call preg_replace using the parent::filer(). This could be done with a direct call. Additionally the PregReplace filter has public methods [set|get]Pattern and [set|get]Replacement which are called before parent::filer() will be called - so the pattern and replacement are internal configurations but are defined as public by PregReplace and the both setter validates the pattern each time filter() will be called.

Btw. On a quick look other the word filter I notices SeparatorToSeparator extends PregFilter directly (without AbstractSeparator) and SeparatorToSeparator::filter() calls the protected method _separatorToSeparatorFilter() without any other logic which looks needless to me.

Owner

weierophinney commented Jan 18, 2013

Closing, as the initial work is done and merged. @marc-mabe -- if you want to tackle the points you raise in a separate PR, please do. :)

Contributor

carlosbuenosvinos commented May 1, 2013

Are you going to fix it also for zendframework1?

@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3376] Removed usage of /e modifier with …
…preg_replace in Zend\Filter

- Modified PregReplace to test for the "e" modifier and raise an
  exception when found
- Modified SeparatorToCamelCase to use preg_replace_callback instead of
  /e modifier
37d3acb

@weierophinney weierophinney added a commit to zendframework/zend-filter that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3376] Added test for new behavior
- Added a test displaying that an exception is raised when passing the
  "e" modifier in a pattern
482716d

@weierophinney weierophinney added a commit to zendframework/zend-ldap that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3376] Replace PCRE /e modifier with preg…
…_replace_callback()

- Replaced usage of preg_replace with "/e" modifier with
  preg_replace_callback() using a closure
8517056

@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3376] Better fix
- The main point is that we ensure hasItems() internally calls
  internalHasItem(). As such, removing the multiple keys ensures we
  don't have issues with index order of mock objects. Test now passes on
  both 3.7.12 and 3.7.13, on all PHP versions used.
bbaf9d9

@weierophinney weierophinney added a commit to zendframework/zend-cache that referenced this issue May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3376] Better fix
- The main point is that we ensure hasItems() internally calls
  internalHasItem(). As such, removing the multiple keys ensures we
  don't have issues with index order of mock objects. Test now passes on
  both 3.7.12 and 3.7.13, on all PHP versions used.
37699fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment