[pmwiki-users] Sanity check of first recipe

[Jan Erik Moström]

Thanks to the answers yesterday I've managed to cobble something 
together that seem to do what I want. But since this is my first 
PHP code and of course my first attempt of doing something 
pmwiki I've probably broken a lot of rules etc.

So I would be very grateful if someone could take a look at the 
code below and see if I've done something stupid (it seem to 
produce the correct HTML).

What I want to do is to be able to insert markup like this

     (:jpl src="path|45|10" pop="path|12|15" capt="bla bla bla":)

     (:jpr src="path|45|10" pop="path|12|15" capt="bla bla bla":)

     (:jpc src="path|45|10" pop="path|12|15" capt="bla bla bla":)

and it should generate some code for displaying photos (I'm 
going to use this markup in other places also, I will place the 
same text there so that's why I'm not using the standard image 
commands in pmwiki). The 'pop' and 'capt' arguments are optional.

Here is my recipe

<?php

Markup("jPhoto",
     'inline',
     "/\\(:jp(l|r|c) src=\"(.+?)\|(\d+)\|(\d+)\"(.+?):\\)/se",
     "Keep(jpFixMarkup('$1','$2','$3','$4',PSS('$5')))"
);

function jpFixMarkup( $type, $image, $iwidth, $iheight, 
$tagstring )
{
     $pops = array();
     $popfound = preg_match("/pop=\"(.+?)\\|(\\d+)\\|(\\d+)\"/",$tagstring,$pops);
     $capts = array();
     $captfound = preg_match("/capt=\"(.*?)\"/",$tagstring,$capts);
     if($type='l'){
         $imlink = "<div class=\"jPhotoLeft\">";
     } elseif ($type='r'){
         $imlink = "<div class=\"jPhotoRight\">";
     } else {
         $imlink = "<div class=\"jPhoto\">";
     }
     if($popfound){
         $imlink .= "<a href=\"{$pops[1]}\" width=\"{$pops[2]}\" height=\"{$pops[2]}\">";
     }

     $imlink .= "<img src=\"$image\" width=\"$iwidth\" 
height=\"$iheight\" />";

     if($popfound){
         $imlink .= "</a>";
     }
     if($captfound){
         $imlink .= "<p>{$capts[1]}</p>";
     }

     return $imlink . '</div>';
}
?>

What should I do differently?
-- 
Jan Erik Moström, www.mostrom.pp.se