PHP - preg_replace not matching multiple occurrences

Question

Trying to replace a string, but it seems to only match the first occurrence, and if I have another occurrence it doesn't match anything, so I think I need to add some sort of end delimiter?

My code:

$mappings = array(
    'fname'     => $prospect->forename,
    'lname'     => $prospect->surname,
    'cname'     => $prospect->company,
);

foreach($mappings as $key => $mapping) if(empty($mapping)) $mappings[$key] = '$2';

$match  = '~{(.*)}(.*?){/.*}$~ise';
$source     = 'Hello {fname}Default{/fname} {lname}Last{/lname}';
// $source  = 'Hello {fname}Default{/fname}';

$text = preg_replace($match, '$mappings["$1"]', $source);

So if I use the $source that's commented, it matches fine, but if I use the one currently in the code above where there's 2 matches, it doesn't match anything and I get an error:

Message: Undefined index: fname}Default{/fname} {lname

Filename: schedule.php(62) : regexp code

So am I right in saying I need to provide an end delimiter or something?

Thanks, Christian

You are using .*? in between, but left the other matches .* greedy. (Tip: always constrain the pattern, if there are obvious characters to allow or disallow.) — mario
– mario, Commented Oct 10, 2012 at 20:24
Thanks mario - Missed that one out. Any ideas on the main problem? :) — christian.thomas
– christian.thomas, Commented Oct 10, 2012 at 20:29

Community · Accepted Answer · 2017-05-23 12:18:50Z

1

Apparently your regexp matches fname}Default{/fname} {lname instead of Default.

As I mentioned here use {(.*?)} instead of {(.*)}.

{ has special meaning in regexps so you should escape it \\{.

And I recommend using preg_replace_callback instead of e modifier (you have more flow control and syntax higlighting and it's impossible to force your program to execute malicious code).

Last mistake you're making is not checking whether the requested index exists. :)

My solution would be:

<?php

class A { // Of course with better class name :)
    public $mappings = array(
        'fname' => 'Tested'
    );

    public function callback( $match)
    {
        if( isset( $this->mappings[$match[1]])){
            return $this->mappings[$match[1]];
        }

        return $match[2];
    }   
}

$a = new A();
$match  = '~\\{([^}]+)\\}(.*?)\\{/\\1\\}~is';
$source     = 'Hello {fname}Default{/fname} {lname}Last{/lname}';

echo preg_replace_callback( $match, array($a, 'callback'), $source);

This results into:

[vyktor@grepfruit tmp]$ php stack.php
Hello Tested Last

edited May 23, 2017 at 12:18

CommunityBot

11 silver badge

answered Oct 10, 2012 at 20:30

Vyktor

21.1k6 gold badges69 silver badges98 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

NullUserException Over a year ago

You'd also have to make the last .* not greedy and remove the end of string anchor $ to make it work. Also, there's no need to escape { here.

christian.thomas Over a year ago

Brilliant - Thanks for your help guys, working :) - I've gone with @NullUserException's expression, but your implementation. Thanks again!

NullUserException Over a year ago

@christian.thomas It's the same expression, except this one has escapes everywhere. Some people like to play it safe and escape everything; I prefer escaping as little as possible for better readability.

christian.thomas Over a year ago

Yep - Noted. Just looked a lot cleaner as well.

Vyktor Over a year ago

@NullUserException I'm escaping like a hamster on coffin just in PHP... Somehow I don't believe they won't change meaning of certain characters in time :-/

|

Sean McSomething · Accepted Answer · 2012-10-10 20:51:21Z

1

Your regular expression is anchored to the end of the string so you closing {/whatever} must be the last thing in your string. Also, since your open and closing tags are simply .*, there's nothing in there to make sure they match up. What you want is to make sure that your closing tag matches your opening one - using a backreference like {(.+)}(.*?){/\1} will make sure they're the same.

I'm sure there's other gotchas in there - if you have control over the format of strings you're working with (IE - you're rolling your own templating language), I'd seriously consider moving to a simpler, easier to match format. Since you're not 'saving' the default values, having enclosing tags provides you with no added value but makes the parsing more complicated. Just using $VARNAME would work just as well and be easier to match (\$[A-Z]+), without involving back-references or having to explicitly state you're using non-greedy matching.

answered Oct 10, 2012 at 20:51

Sean McSomething

6,5372 gold badges26 silver badges28 bronze badges

1 Comment

Sean McSomething Over a year ago

@christian.thomas Regular expressions are an amazingly powerful tool. The most important thing to learn about them is when not to use them - using a simpler tag syntax would allow you to just user str_replace() and not even bother with regular expressions.

Collectives™ on Stack Overflow

PHP - preg_replace not matching multiple occurrences

2 Answers 2

6 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related