[PROPOSAL]Add second to callback of preg_replace_callback

Hey:
there comes a FR: https://bugs.php.net/bug.php?id=64730

the main idea is, in 5.5 we remove support of 'e' modifier.

then comes a problem, the old codes(a real use case see
https://github.com/php/php-src/blob/PHP-5.4/Zend/zend_vm_gen.php#L390):

preg_replace(array(

    "/pattern1(.*)/",
    "/pattern2(.*)/"

),
array(
"/replace1/e",
"/replace2/e"
)
..),

can not be easier convert to the "callback" style.

then I have to change it to something very ugly like(a real use case
see:
https://github.com/php/php-src/blob/PHP-5.5/Zend/zend_vm_gen.php#L390):

function callback($subject) {
if (!strncmp($subject, "pattern1", 8)) {
//function for pattern 1
} else if(!strncmp($subject, "pattern2", 8)) {
//function for pattern 2
} else .....

}

so I propose to add a second argument to callback(aim to php-5.5.1),
which is the matched regex string self.

then I can simplify the previous codes to:

function callback ($subject, $regex) {
$replace_funcs = array(
"/pattern1(.)" => function ($subect) { //function for
parttern 1; },
"/pattern2(.)" => function($sbuect) { //function for
pattern
2; }
);

   $replace_funcs[$regex]($subject);

}

what do you think(of course, the second argument can also easily change
to be the regex idx in the regex array)?

patch is here:

https://bugs.php.net/patch-display.php?bug_id=64730&patch=sencode_argument.patch&revision=latest

What's wrong with this?

$replacements = ['regex' => 'callback'];
foreach ($replacements as $regex => $callback) {
$str = preg_replace_callback($regex, $callback, $str);

hey:

efficiency & simplicity

do you use foreach even there is array_map?

thanks

}

Nikita

--
Laruence Xinchen Hui
http://www.laruence.com/

what about patch preg_replace to accept callbacks?

example:
preg_replace(array(
"/pattern1(.)/",
"/pattern2(.)/"
),
array(
function ($pat) { ... } ,
"replace"
)
, ... );

Hey:
then I think perg_replace_callback become useless

thanks

--

--
Laruence Xinchen Hui
http://www.laruence.com/

ALeX,

what about patch preg_replace to accept callbacks?

example:
preg_replace(array(
"/pattern1(.)/",
"/pattern2(.)/"
),
array(
function ($pat) { ... } ,
"replace"
)
, ... );

There are some weird questions that come up with that. For example, if an
array item is an object that implements both __invoke() and __toString(),
which should be fired? Is it a string? Or a callback? What if I pass a
literal string "strlen", is that a callback, or a replacement?

The point is not that it's a bad idea and that we can't make a distinction,
but more that there are pretty severe edge-cases that we'd have to work
around, and if we don't get it right security could suffer significantly
(especially when user-input is allowed to be a replacement)...

Anthony

Hey:
Sorry for the delay, the new patch, which make the second argument the
regex array keys is attached.

https://bugs.php.net/patch-display.php?bug_id=64730&patch=second_arg_rege_key.patch&revision=latest

with this patch, preg_replace_callback will call user callback with two
arguments, the first one is the same, the second is the regex key if the
regex is an array or NULL if the regex is a string.

then we can do something like:

$code = preg_replace_callback(

array(
"foo" => "/some very complex regex/",
"bar" => "/another one/",
...
),
function($matches, $idx) {
switch ($idx) {
case 'foo'
...
case 'bar':
...
}
},
$code);

if no objections, I will commit this patch after 5.5. 0 final release..

thanks

I object. If this were using the preg_replace_callback(Array, Array,
String) syntax [which is not practically possible due to ambiguity], I
would agree that this is a nice feature which makes the function consistent
with preg_replace and str_replace. But this implementation with some weird
additional identifier that you need to switch over makes absolutely no
sense to me and only complicated the API. Better to just use a loop for
this.

One thing that might be nicer API wise but without ambiguity is an
preg_replace_callback(['regex' => callback], $string) overload. Though
personally I'm not really a fan of doing overloads with completely
different argument types.

Nikita

Le 04/05/2013 14:39, Laruence a écrit :

with this patch, preg_replace_callback will call user callback with two
arguments, the first one is the same, the second is the regex key if the
regex is an array or NULL if the regex is a string.

Here is another example from a real case (Horde_Date)

return preg_replace(
array('/%b/e',
'/%B/e',
'/%C/e',
'/%([-#]?)d/e',
'/%D/e',
'/%e/e',
'/%([-#]?)H/e',
'/%([-#]?)I/e',
'/%([-#]?)m/e',
'/%([-#]?)M/e',
'/%n/',
'/%p/e',
'/%R/e',
'/%([-#]?)S/e',
'/%t/',
'/%T/e',
'/%x/e',
'/%X/e',
'/%y/e',
'/%Y/',
'/%%/'),
array('$this->strftime(Horde_Nls::getLangInfo(constant('ABMON_' .
(int)$this->month)))',
'$this->strftime(Horde_Nls::getLangInfo(constant('MON' .
(int)$this->_month)))',
'(int)($this->_year / 100)',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_mday)',
'$this->strftime('%m/%d/%y')',
'sprintf('%2d', $this->_mday)',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_hour)',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_hour == 0 ?
12 : ($this->_hour > 12 ? $this->_hour - 12 : $this->_hour))',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_month)',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_min)',
"\n",
'$this->strftime(Horde_Nls::getLangInfo($this->_hour < 12 ? AM_STR :
PM_STR))',
'$this->strftime('%H:%M')',
'sprintf('%' . ('$1' ? '' : '02') . 'd', $this->_sec)',
"\t",
'$this->strftime('%H:%M:%S')',
'$this->strftime(Horde_Nls::getLangInfo(D_FMT))',
'$this->strftime(Horde_Nls::getLangInfo(T_FMT))',
'substr(sprintf('%04d', $this->_year), -2)',
(int)$this->_year,
'%'),
$format);
}

Note, while I agree for this feature, must application / framework will
have to be fixed, but also kept compatible with 5.3, 5.4, 5.5.

Remi.