Cherry-picking Pearls in San Francisco

recent perl mistakes (or quite mistakes)

What to write about today? I don't have much time for iron man, so I decided to share two recent bummers or quite.

 Yesterday I got hit by the following code written by someone else:

if (! $blog->can('allow_html_comment') or ! $blog->allow_html_comment) {
   remove_html();
}

First, it's probably better to cleanly define the class/role hierarchy to avoid can() altogether, but sometimes, using can() is just the cheap hack that make it Just Work, so let's let it go.

What I was bitten by is actually in the underlaying allow_html_comment() implementation which I co-wrote. &allow_html_comment is actually installed using AUTOLOAD(), so can() always return false until the first call inside the process, making this conditional evaluation inconsistent depending on the execution order. Aweful.

I guess can() is a semi-bad idea, AUTOLOAD used this way is another semi-bad idea, but both combined make it a really-bad idea.

The second bummer is in this gist: http://gist.github.com/107815

It never occurred to me to use scalar() the way it is used in 'baz', but it occurred to a colleague, do you know what it does and why?

more review-board and git

I tried to read more of review-board documentation, and it looks like the post-review tool I was talking about in an earlier post doesn't do exactly what I want... Besides, it doesn't work when there is a relative path on the subversion server. I've dirty patched it (no guarantee at all that it doesn't break something else) to make it do something instead of failing (see gist below).

On the other hand, it looks like it still insists on using 'master' one way or another which doesn't really work for me. Ideally I would have to modify that too, or maybe add an option to the tool to make it accept a svn diff in input, because surprisingly it doesn't exist (post-review really wants to do the diff itself, which is... noble).

Posting a patch.diff based review right from the command line instead of using my browser would still be a total win, anyway.

 

post review and git

I found a nicer solution to my problem of making review-board work with Git. Indeed I've read the manual! so intead of my svn-diff.pl: you can also install the command-line post-review tool that also does tons of other useful things. With that tool do:

post-review --parent trunk -n

to generate a diff that you can upload in review board. I'm searching for the right way of actually submitting this patch directly to a svn review-board repository (review board can apparently be hooked into a git repo directly too), but so far I haven't read in the manual that it was possible.

Find::Lib to unclutter tools, tests and other stuff

In the past I've been typing over and over again the following kind of things:

use FindBin qw($Bin);
use lib "$Bin/../../../app/lib";
use lib "$Bin/../locallib/";
...

or,

use FindBin qw($Bin);
use lib "$Bin/../../../common/lib";
use Bootstrap; # smartely imports additional libraries for my app

Especially in test files, I use this all over the place. The nice concept with the exported $Bin variable is that it allows you to specify local libs path relative to the given script or tool, making the code transportable easily.

But now, what I do, is I install everywhere I can Find::Lib module, that provides some nice benefits:

• reduces the typing to one line in most cases
• no crufty $Bin thing which doesn't even indicate a binary
• since version 0.04 handles well the case where symlinks are used all over the place (which is not the case whith FindBin)

The second example is advantageously rewritten to:

use Find::Lib '../../../common/lib" => Bootstrap;

In conclusion, wherever you can install Find::Lib in system PERL5LIB, I recommend you use Find::Lib. Everywhere else you can use the buggy FindBin which is in Perl core, it is still better than using absolute paths in the source. Another solution is to use $ENV variables to indicate an application "base", but personnally I dislike it because I feel like I'll forget to update the env variable.

Find::Lib on CPAN

update: I just found out rlib on CPAN that has been around for 9 years. Find::Lib is more capable though