[Catalyst-dev] Annoying undef warnings from CP::Static::Simple

[Jonathan Rockway]

* On Sat, May 31 2008, Dave Rolsky wrote:
> On Sat, 31 May 2008, Matt S Trout wrote:
>
>> On Fri, May 30, 2008 at 06:35:11PM -0500, Dave Rolsky wrote:
>>> I'm getting a bunch of warnings from requests for static content that look
>>> like this:
>>>
>>>  Use of uninitialized value in length at
>>>  /usr/local/share/perl/5.8.8/Catalyst/Dispatcher.pm line 287.
>>>
>>> The problem is that Catalyst::Dispatcher expects $c->req->match to be
>>> defined, but it isn't when CPSS does the dispatching.
>>>
>>> I'm not sure where this should be fixed. Should $c->req->match always be
>>> set? Or should CD just not assume it is defined?
>>
>> There's no $c->action at all when CPSS fires so I don't think I can really
>> see $c->req->match as making a lot of sense in that case.
>>
>> This has already been reported via rt.cpan but sans test; if you can whip one
>> up there's no reason we can't add a defined() check.
>
> I've attached a new test and a patch to make it pass.
>
> As an aside, I realized that part of the reason this was never fixed
> is that the core Catalyst code does not enable "use warnings" in the
> modules. However, if you run under the -w flag like the standalone
> server does, then you get all the warnings from everywhere.
>
> I think it'd be a good idea to turn on warnings in the core modules
> and then shut them up. Are such patches welcome?

Yes.  (This is a good time to try git-svn, btw.  Make a branch, make one
commit for each fix, then tell git to dump the whole set onto
the mailing list.  Very convenient.)

Just for the record, sometimes I think turning off warnings is better
than "fixing" them, for example, I think:

  { no warnings;
    if($foo eq 'bar'){ ... }
  }

is easier to read than:

  if($foo && $foo eq 'bar'){ ... }

The second adds code that serves absolutely no algorithmic purpose, and
I'm against that.

Regards,
Jonathan Rockway

-- 
print just => another => perl => hacker => if $,=$"