What am I doing wrong with preg_replace?

Question

I have fought with this for multiple days now. Im trying to get my preg_replaces working. I'm getting the data to use the preg-replace on from a database and binding them to variables. $post_preview is one of them.

<?php
    $body_sub = substr($post_preview, 0) . "<br><br><a href='post.php?id=$post_id'> Continue Reading →</a>";
    echo nl2br($body_sub); ?></p>
            <?php
    //bold
        $post_preview = preg_replace('/(\[bold\])/', '<strong>', $post_preview);
        $post_preview = preg_replace('/(\[\/bold\])/', '</strong>', $post_preview);
        echo '[bold]Test[/bold]';
    //italic
        $post_preview = preg_replace('/(\[i\])/', '<i>', $post_preview);
        $post_preview = preg_replace('/(\[\/i\])/', '</i>', $post_preview);

This isn't replacing the [i] and [bold] nor the [/i] and the [/bold]. No errors even with E_ALL. What have I done wrong and how do I fix it?

Are you sure you output the correct data? `echo '[bold]Test[/bold]';` seems misplaced. — kero, Sep 22 '13 at 17:29
Its there just for testing purposes. The correct data is the `echo nl2br($body_sub); ?>` — tehX, Sep 22 '13 at 17:35

score 1 · Answer 1 · answered Sep 22 '13 at 17:34

<?php
//bold
$post_preview = preg_replace('/\[bold\]/', '<strong>', $post_preview);
$post_preview = preg_replace('/\[\/bold\]/', '</strong>', $post_preview);
//italic
$post_preview = preg_replace('/\[i\]/', '<i>', $post_preview);
$post_preview = preg_replace('/\[\/i\]/', '</i>', $post_preview);

echo $post_preview;
?>

This should do it. I'm wondering why you'd want to use a regular expression for these though. Wouldn't it be far easier if you'd use str_replace in this case?

score 0 · Accepted Answer · edited May 23 '17 at 11:56

Your regular expressions have too much brackets (see n0tiz' post) . However this can be much simplified. First of all, I'd suggest to use one regular expression for one tag, not two. This can easily be done via a wildcard like (.*)? meaning:

( begin of new group
. every character
* 0 or more times
) end of group
? don't be greedy (What do lazy and greedy mean in the context of regular expressions?)

Now since your operations don't have any arguments, in the next part since both operations are the same they should get into one loop, so the code needs to be written only once.

$text = "[bold]hello this [i]is a test[/bold][bold]i[/i]";

$html_tags = array(
    "bold" => "strong",
    "i" => "em",
);

foreach ($html_tags as $code=>$tag)
    $text = preg_replace("/\[$code\](.*)?\[\/$code]/i", "<$tag>$1</$tag>", $text);

Using one regular expression has the advantage that wrong code won't create wrong HTML.

What am I doing wrong with preg_replace?

2 Answers2