Req #64730 [Asn->Csd]: preg_replace_callback vs. preg_replace eval related

From: Date: Sun, 27 Mar 2016 20:44:20 +0000
Subject: Req #64730 [Asn->Csd]: preg_replace_callback vs. preg_replace eval related
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-200171@lists.php.net to get a copy of this message 
Edit report at https://bugs.php.net/bug.php?id=64730&edit=1

 ID:                 64730
 Updated by:         nikic@php.net
 Reported by:        imbolk at gmail dot com
 Summary:            preg_replace_callback vs. preg_replace eval related
-Status:             Assigned
+Status:             Closed
 Type:               Feature/Change Request
 Package:            Regexps related
 Operating System:   Mac OS X 10.8.3
 PHP Version:        5.5.0beta4
 Assigned To:        laruence
 Block user comment: N
 Private report:     N

 New Comment:

PHP 7 adds preg_replace_callback_array() to cover this use case: http://php.net/manual/en/function.preg-replace-callback-array.php


Previous Comments:
------------------------------------------------------------------------
[2013-08-09 13:55:42] joel at umbrellasource dot com

I don't like the idea of passing the regex as a second callback argument. The main argument
against it is having to test for the regex itself means you have to either globalize an array of
regex or duplicate code to add a test inside the callback. Neither should be considered a good
coding solution.

The idea behind preg_replace is that you have two arrays and the index of the matched regex is the
index we need to use for the replace (i.e. we matched $search[2] so we're going to use
$replace[2]). So the second argument of the callback should be the index of the regex in the array
that was matched. Here's what I would like to see, expressed in code form

$str = preg_replace_callback(['/\d/', '/\s/'], function(Array $match, $index) { 
    $replace = ['digit', 'space'];
    return $replace[$index];
}, $str);

This is more intuitive, it's cleaner code-wise, it avoids any complicated solutions(no global
array of callbacks or regex), it keeps the same basic functionality of preg_replace (array in, array
out), and it's still simple enough where you could use an anonymous function inline (which the
documentation currently recommends).

------------------------------------------------------------------------
[2013-05-04 12:35:28] laruence@php.net

The following patch has been added/updated:

Patch Name: second_arg_rege_key.patch
Revision:   1367670928
URL:        https://bugs.php.net/patch-display.php?bug=64730&patch=second_arg_rege_key.patch&revision=1367670928

------------------------------------------------------------------------
[2013-05-01 02:08:19] imbolk at gmail dot com

Yes, you are quite right.

------------------------------------------------------------------------
[2013-04-30 21:09:35] ww dot galen at gmail dot com

Accepting an array of callbacks can lead to unreconcilable ambiguities. For example:

    class A {
        function __toString() { ... }
        function __invoke($a) { ... }
        function foo($a) { ... }
    }
    function foo($a) { ... }
    
    $a = new A;
    preg_replace_callback([..., ...], [$a, 'foo'], $subject);

There are three different ways of interpreting the callback argument, all equally valid:

1. (string)$a and foo(...)
2. $a(...) and foo(...)
3. $a->foo(...)

------------------------------------------------------------------------
[2013-04-29 18:03:49] imbolk at gmail dot com

I think it would be better if prey_replace_callback function will accept array of 
callbacks as a 2nd argument.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=64730


--
Edit this bug report at https://bugs.php.net/bug.php?id=64730&edit=1

Thread (11 messages)

« previous php.bugs (#200171) next »