Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am creating a simple template engine that uses Regex to find special expressions, which are parsed and processed. They are enclosed in Ruby-style opening tags and have the format:

    <% label %>  OR:
    <% function(arg1, arg2, arg3) %>

However the problem is that I cannot have multiple entries on the same line, so I can't have this:

    <p><% name %> - <% age %> - <% gender %></p>

Instead I have to write it like this:

    <p><% name %> - 
       <% age %> -
       <% gender %><p>

How can I fix this? Below I included the code for the function that handles this. Basically what it does is read from a file, split everything line by line, then iterate through each line and do a regex match and return the results.

public function content()
{
    if (!file_exists($this->path))
        return;

    /* 0 = before, 1 = entry #1, 2 = entry #2, 3 = after */
    $content = file_get_contents($this->path); 
    $pattern = '/^(.*)\<\%\s*(.+)\s*\%\>(.*)$/'; 
    $lines = explode("\n", $content); 


    foreach ($lines as $line)
    {
        // Pattern match the line
        preg_match($pattern, $line, $matches); 

        $result = ''; 

        // If there are no matches, then don't parse it and add the raw string to output
        if (count($matches) == 0)
        {
            continue; 
        }   


        // Parse the matches and run the appropriate action         
        $before = $matches[1]; 
        $after = $matches[3]; 
        $entry = $matches[2]; 
        $return = ''; 


        // Process the entry
        $return = $this->processEntry($entry);

        // Add the processed line to the final result if not empty
        $result = $before . $return . $after; 
        if (!empty($result)) $final .= $result . "\n"; 
    }
    return $final; 
}
share|improve this question
add comment

2 Answers

up vote 2 down vote accepted

Your current regex is too greedy.

When running against your single line input, I get this result:

/^(.*)\<\%\s*(.+)\s*\%\>(.*)$/

[0] => <p><% name %> - <% age %> - <% gender %></p>
[1] => <p><% name %> - <% age %> - 
[2] => gender 
[3] => </p>

By upgrading the regex to be lazy and also using preg_match_all, we can get the following result:

/(.*?)\<\%\s*(.+?)\s*\%\>/

[0] =>  [0] => <p><% name %>
        [1] =>  - <% age %>
        [2] =>  - <% gender %>
[1] =>  [0] => <p>
        [1] =>  - 
        [2] =>  - 
[2] =>  [0] => name
        [1] => age
        [2] => gender

The only part missing from that match is the end, the last </p>. I would use another regex match to grab that portion:

/.*\%\>(.*)$/

[0] => <p><% name %> - <% age %> - <% gender %></p>
[1] => </p>

Recommended reading over greedy and lazy: http://www.regular-expressions.info/repeat.html

share|improve this answer
add comment

What to you thing about:

<?=label ?>  OR:
<?=function(arg1, arg2, arg3) ?>

PHP is already a template engine

Just write a wrapper around a include call and your template system is ready to use and more flexible than anything you could write.


That said, why to you match single lines? Please have a look at preg_replace_callback.

/\<\%\s*(.+?)\s*\%\>/

If you want to put the whitespace trimming in the callback and as you don't need to escape <>%, you could even use

/<%(.+?)%>/
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.