[dokuwiki] Re: Please Review

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sebastian Menge wrote:
> Hi again
> 
> I just committed my first syntax plugin (for the music
> typesetter "lilypond") to google code. It's problably not of wide
> interest here, but anyways: I would be very thankful if someone could
> review the plugin, especially for security and general php and
> dokuwiki issues.

Well, I'm interested in the plugin. I'm a happy user of ABCmidi
(dw-plugin) for simple things but lilypond offers much better typesetting.

I found some problems with rev.2: It only works in the root-namespace.
FIX is simple: use $filename=mediaFN($ID);
<explanation>
  Filenames in subnamespace get expanded with a ':' and you'd need to
  escape those in a file-path; besides ALL lilypond files end up
  in DOKU_INC/data/media/ and not it's subfolders.
</explanation>

Furthermore exec( "/usr/bin/pdfinfo $file |.." should read
            exec( 'pdfinfo'.escapeshellarg($file)." | .."
You should check if the file_exists($file) before doing that and may
want to see if(is_array($output)) before trying to
split(':',$output[0]); it.

I also had the problem that lilypong PNG's were cached by my browser
after I've changed the lilypond source and saved it.

FIX: In render() you can use sth. alike.
    global $conf;
    $hash = md5(serialize($data));
    $filename = $conf['mediadir'] . '/lilypond/'.$hash;
    $url = ml('lilypond:'.$hash.'.png');

and make use of caching the images: call lilypond() and writely() in
render() not in handle(); and only once for each hash. see for example
the graphviz or latex plugin. You may also want to cache the number of
pages instead of calling `pdfinfo` for each page request. fi.
  p_set_metadata($ID,'lilypages', $this->pages(..));
  p_get_metadata($ID,'lilypages');
  p_get_metadata($ID,'lilyfilename');


It may sound like much work, but it's not. You're thru the worst
already! - lilypond is a great plugin; keep up the work. For later
versions I'd suggest to also make the PNG size configurable. (either
global $conf or parameter eg. <lilypond&resolution=20> ) - Have a look
at the http://www.dokuwiki.org/plugin:abc source for configuration options.


> It's very small and with google-code you can double click on the lines
> you want to comment, and fill out a textarea:

I prefer svn/vim to double-clicking; and hope you don't mind to take
this by email.

> Heres the link: 
> http://code.google.com/p/lilypond-plugin/source/browse/trunk/syntax.php
> 
> Thanks a lot, Sebastian.

cheers,
robin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkj8p5AACgkQeVUk8U+VK0K2pACfTmc3KAuQlLSTI6plKsehD2ri
PkoAoJey8Hh32GCQVOwChk0CZ4H9xUAf
=nX8M
-----END PGP SIGNATURE-----
-- 
DokuWiki mailing list - more info at
http://wiki.splitbrain.org/wiki:mailinglist

Other related posts: