[Catalyst-commits] r7966 - in Catalyst-Runtime/5.80/trunk: . lib
lib/Catalyst
groditi at dev.catalyst.perl.org
groditi at dev.catalyst.perl.org
Mon Jun 23 22:00:36 BST 2008
Author: groditi
Date: 2008-06-23 22:00:36 +0100 (Mon, 23 Jun 2008)
New Revision: 7966
Modified:
Catalyst-Runtime/5.80/trunk/
Catalyst-Runtime/5.80/trunk/TODO
Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm
Catalyst-Runtime/5.80/trunk/lib/Catalyst/Action.pm
Catalyst-Runtime/5.80/trunk/lib/Catalyst/Dispatcher.pm
Log:
r17012 at martha (orig r7524): groditi | 2008-03-25 20:45:31 -0400
some more comments and little changes as well as some notes. test output still the same. aplogizing if this breaks something, my SVK is acting really strange
Property changes on: Catalyst-Runtime/5.80/trunk
___________________________________________________________________
Name: svk:merge
- 1c72fc7c-9ce4-42af-bf25-3bfe470ff1e8:/local/Catalyst/trunk/Catalyst-Runtime:9763
4ad37cd2-5fec-0310-835f-b3785c72a374:/Catalyst-Runtime/5.70/trunk:7576
4ad37cd2-5fec-0310-835f-b3785c72a374:/Catalyst-Runtime/5.80/branches/moose:7523
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-ChildOf:4443
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-Runtime-jrockway:5857
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-component-setup:4320
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-docs:4325
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/current/Catalyst-Runtime:5142
4ad37cd2-5fec-0310-835f-b3785c72a374:/trunk/Catalyst:4483
4ad37cd2-5fec-0310-835f-b3785c72a374:/trunk/Catalyst-Runtime:6165
d7608cd0-831c-0410-93c0-e5b306c3c028:/local/Catalyst/Catalyst-Runtime:8339
d7608cd0-831c-0410-93c0-e5b306c3c028:/local/Catalyst/Catalyst-Runtime-jrockway:8342
e56d974f-7718-0410-8b1c-b347a71765b2:/local/Catalyst-Runtime:6511
e56d974f-7718-0410-8b1c-b347a71765b2:/local/Catalyst-Runtime-current:10442
+ 1c72fc7c-9ce4-42af-bf25-3bfe470ff1e8:/local/Catalyst/trunk/Catalyst-Runtime:9763
4ad37cd2-5fec-0310-835f-b3785c72a374:/Catalyst-Runtime/5.70/trunk:7576
4ad37cd2-5fec-0310-835f-b3785c72a374:/Catalyst-Runtime/5.80/branches/moose:7524
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-ChildOf:4443
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-Runtime-jrockway:5857
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-component-setup:4320
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/Catalyst-docs:4325
4ad37cd2-5fec-0310-835f-b3785c72a374:/branches/current/Catalyst-Runtime:5142
4ad37cd2-5fec-0310-835f-b3785c72a374:/trunk/Catalyst:4483
4ad37cd2-5fec-0310-835f-b3785c72a374:/trunk/Catalyst-Runtime:6165
d7608cd0-831c-0410-93c0-e5b306c3c028:/local/Catalyst/Catalyst-Runtime:8339
d7608cd0-831c-0410-93c0-e5b306c3c028:/local/Catalyst/Catalyst-Runtime-jrockway:8342
e56d974f-7718-0410-8b1c-b347a71765b2:/local/Catalyst-Runtime:6511
e56d974f-7718-0410-8b1c-b347a71765b2:/local/Catalyst-Runtime-current:10442
Modified: Catalyst-Runtime/5.80/trunk/TODO
===================================================================
--- Catalyst-Runtime/5.80/trunk/TODO 2008-06-23 21:00:25 UTC (rev 7965)
+++ Catalyst-Runtime/5.80/trunk/TODO 2008-06-23 21:00:36 UTC (rev 7966)
@@ -10,3 +10,9 @@
- Make classes immutable at setup() time
+
+ - GRODITI's list:
+ * comments marked /Moose TODO/i in the code
+ * Fix the CDI compat hack so we can start moving to immutable
+ * Profile before and after immutable.
+ * I think now owuld be a good time to lay ground for the App / Ctx split
\ No newline at end of file
Modified: Catalyst-Runtime/5.80/trunk/lib/Catalyst/Action.pm
===================================================================
--- Catalyst-Runtime/5.80/trunk/lib/Catalyst/Action.pm 2008-06-23 21:00:25 UTC (rev 7965)
+++ Catalyst-Runtime/5.80/trunk/lib/Catalyst/Action.pm 2008-06-23 21:00:36 UTC (rev 7966)
@@ -10,7 +10,7 @@
=head1 DESCRIPTION
-This class represents a Catalyst Action. You can access the object for the
+This class represents a Catalyst Action. You can access the object for the
currently dispatched action via $c->action. See the L<Catalyst::Dispatcher>
for more information on how actions are dispatched. Actions are defined in
L<Catalyst::Controller> subclasses.
@@ -47,8 +47,17 @@
sub dispatch { # Execute ourselves against a context
my ( $self, $c ) = @_;
+ #Moose todo: grrrrrr. this is no good. i don't know enough about it to
+ # debug it though. why can't we just call the accessor?
local $c->{namespace} = $self->namespace;
return $c->execute( $self->class, $self );
+
+ #believed to be equivalent:
+ #my $orig = $c->namespace;
+ #$c->namespace($self->namespace);
+ #my $ret = $c->execute( $self->class, $self );
+ #$c->namespace($orig);
+ #return $ret;
}
sub execute {
Modified: Catalyst-Runtime/5.80/trunk/lib/Catalyst/Dispatcher.pm
===================================================================
--- Catalyst-Runtime/5.80/trunk/lib/Catalyst/Dispatcher.pm 2008-06-23 21:00:25 UTC (rev 7965)
+++ Catalyst-Runtime/5.80/trunk/lib/Catalyst/Dispatcher.pm 2008-06-23 21:00:36 UTC (rev 7966)
@@ -108,8 +108,8 @@
sub dispatch {
my ( $self, $c ) = @_;
- if ( $c->action ) {
- $c->forward( join( '/', '', $c->action->namespace, '_DISPATCH' ) );
+ if ( my $action = $c->action ) {
+ $c->forward( join( '/', '', $action->namespace, '_DISPATCH' ) );
}
else {
@@ -254,9 +254,10 @@
sub prepare_action {
my ( $self, $c ) = @_;
- my $path = $c->req->path;
- my @path = split /\//, $c->req->path;
- $c->req->args( \my @args );
+ my $req = $c->req;
+ my $path = $req->path;
+ my @path = split /\//, $req->path;
+ $req->args( \my @args );
unshift( @path, '' ); # Root action
@@ -279,10 +280,11 @@
unshift @args, $arg;
}
- s/%([0-9A-Fa-f]{2})/chr(hex($1))/eg for grep { defined } @{$c->req->captures||[]};
+ #Moose todo: This seems illegible, even if efficient.
+ s/%([0-9A-Fa-f]{2})/chr(hex($1))/eg for grep { defined } @{$req->captures||[]};
- $c->log->debug( 'Path is "' . $c->req->match . '"' )
- if ( $c->debug && $c->req->match );
+ $c->log->debug( 'Path is "' . $req->match . '"' )
+ if ( $c->debug && $req->match );
$c->log->debug( 'Arguments are "' . join( '/', @args ) . '"' )
if ( $c->debug && @args );
@@ -300,7 +302,7 @@
$namespace = join( "/", grep { length } split '/', $namespace || "" );
- return $self->_action_hash->{"$namespace/$name"};
+ return $self->_action_hash->{"${namespace}/${name}"};
}
=head2 $self->get_action_by_path( $path );
@@ -352,6 +354,7 @@
return reverse grep { defined } @containers, $self->_container_hash->{''};
+ #return (split '/', $namespace); # isnt this more clear?
my @parts = split '/', $namespace;
}
@@ -390,12 +393,11 @@
my $registered = $self->_registered_dispatch_types;
- my $priv = 0;
+ #my $priv = 0; #seems to be unused
foreach my $key ( keys %{ $action->attributes } ) {
next if $key eq 'Private';
my $class = "Catalyst::DispatchType::$key";
unless ( $registered->{$class} ) {
- #eval "require $class";
#some error checking rethrowing here wouldn't hurt.
eval { Class::MOP::load_class($class) };
push( @{ $self->_dispatch_types }, $class->new ) unless $@;
Modified: Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm
===================================================================
--- Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm 2008-06-23 21:00:25 UTC (rev 7965)
+++ Catalyst-Runtime/5.80/trunk/lib/Catalyst.pm 2008-06-23 21:00:36 UTC (rev 7966)
@@ -30,10 +30,17 @@
BEGIN { require 5.008001; }
-__PACKAGE__->mk_accessors(
- qw/counter request response state action stack namespace stats stash/
-);
+has stack => (is => 'rw');
+has stash => (is => 'rw');
+has state => (is => 'rw');
+has stats => (is => 'rw');
+has action => (is => 'rw');
+has counter => (is => 'rw');
+has request => (is => 'rw');
+has response => (is => 'rw');
+has namespace => (is => 'rw');
+
attributes->import( __PACKAGE__, \&namespace, 'lvalue' );
sub depth { scalar @{ shift->stack || [] }; }
@@ -76,6 +83,7 @@
my $caller = caller(0);
+ #why does called have to ISA Catalyst and ISA Controller ?
unless ( $caller->isa('Catalyst') ) {
no strict 'refs';
if( $caller->can('meta') ){
@@ -1319,6 +1327,8 @@
=cut
+#Why does this exist? This is no longer safe and WILL NOT WORK.
+# it doesnt seem to be used anywhere. can we remove it?
sub _localize_fields {
my ( $c, $localized, $code ) = ( @_ );
@@ -1346,8 +1356,9 @@
}
# Allow engine to handle finalize flow (for POE)
- if ( $c->engine->can('finalize') ) {
- $c->engine->finalize($c);
+ my $engine = $c->engine;
+ if ( my $code = $engine->can('finalize') ) {
+ $engine->$code($c);
}
else {
@@ -1411,31 +1422,35 @@
sub finalize_headers {
my $c = shift;
+ my $response = $c->response; #accessor calls can add up?
+
+ # Moose TODO: Maybe this should be an attribute too?
# Check if we already finalized headers
- return if $c->response->{_finalized_headers};
+ return if $response->{_finalized_headers};
# Handle redirects
- if ( my $location = $c->response->redirect ) {
+ if ( my $location = $response->redirect ) {
$c->log->debug(qq/Redirecting to "$location"/) if $c->debug;
- $c->response->header( Location => $location );
+ $response->header( Location => $location );
- if ( !$c->response->body ) {
+ #Moose TODO: we should probably be using a predicate method here ?
+ if ( !$response->body ) {
# Add a default body if none is already present
- $c->response->body(
+ $response->body(
qq{<html><body><p>This item has moved <a href="$location">here</a>.</p></body></html>}
);
}
}
# Content-Length
- if ( $c->response->body && !$c->response->content_length ) {
+ if ( $response->body && !$response->content_length ) {
# get the length from a filehandle
- if ( blessed( $c->response->body ) && $c->response->body->can('read') )
+ if ( blessed( $response->body ) && $response->body->can('read') )
{
- my $stat = stat $c->response->body;
+ my $stat = stat $response->body;
if ( $stat && $stat->size > 0 ) {
- $c->response->content_length( $stat->size );
+ $response->content_length( $stat->size );
}
else {
$c->log->warn('Serving filehandle without a content-length');
@@ -1443,14 +1458,14 @@
}
else {
# everything should be bytes at this point, but just in case
- $c->response->content_length( bytes::length( $c->response->body ) );
+ $response->content_length( bytes::length( $response->body ) );
}
}
# Errors
- if ( $c->response->status =~ /^(1\d\d|[23]04)$/ ) {
- $c->response->headers->remove_header("Content-Length");
- $c->response->body('');
+ if ( $response->status =~ /^(1\d\d|[23]04)$/ ) {
+ $response->headers->remove_header("Content-Length");
+ $response->body('');
}
$c->finalize_cookies;
@@ -1458,7 +1473,7 @@
$c->engine->finalize_headers( $c, @_ );
# Done
- $c->response->{_finalized_headers} = 1;
+ $response->{_finalized_headers} = 1;
}
=head2 $c->finalize_output
@@ -1543,6 +1558,8 @@
my ( $class, @arguments ) = @_;
$class->context_class( ref $class || $class ) unless $class->context_class;
+ #Moose TODO: if we make empty containers the defaults then that can be
+ #handled by the context class itself instead of having this here
my $c = $class->context_class->new(
{
counter => {},
@@ -1582,6 +1599,7 @@
$c->request->_context($c);
$c->response->_context($c);
+ #XXX reuse coderef from can
# Allow engine to direct the prepare flow (for POE)
if ( $c->engine->can('prepare') ) {
$c->engine->prepare( $c, @arguments );
@@ -1633,6 +1651,7 @@
sub prepare_body {
my $c = shift;
+ #Moose TODO: what is _body ??
# Do we run for the first time?
return if defined $c->request->{_body};
@@ -1952,9 +1971,10 @@
$dispatcher = $class->dispatcher_class;
}
- unless (Class::Inspector->loaded($dispatcher)) {
- require Class::Inspector->filename($dispatcher);
- }
+ Class::MOP::load_class($dispatcher);
+ #unless (Class::Inspector->loaded($dispatcher)) {
+ # require Class::Inspector->filename($dispatcher);
+ #}
# dispatcher instance
$class->dispatcher( $dispatcher->new );
@@ -2040,9 +2060,10 @@
$engine = $class->engine_class;
}
- unless (Class::Inspector->loaded($engine)) {
- require Class::Inspector->filename($engine);
- }
+ Class::MOP::load_class($engine);
+ #unless (Class::Inspector->loaded($engine)) {
+ # require Class::Inspector->filename($engine);
+ #}
# check for old engines that are no longer compatible
my $old_engine;
@@ -2097,7 +2118,9 @@
$home = Catalyst::Utils::home($class);
}
+ #I remember recently being scolded for assigning config values like this
if ($home) {
+ #I remember recently being scolded for assigning config values like this
$class->config->{home} ||= $home;
$class->config->{root} ||= Path::Class::Dir->new($home)->subdir('root');
}
@@ -2119,6 +2142,7 @@
my $env_debug = Catalyst::Utils::env_value( $class, 'DEBUG' );
if ( defined($env_debug) ? $env_debug : $debug ) {
no strict 'refs';
+ #Moose todo: dying to be made a bool attribute
*{"$class\::debug"} = sub { 1 };
$class->log->debug('Debug messages enabled');
}
@@ -2144,6 +2168,7 @@
my $env = Catalyst::Utils::env_value( $class, 'STATS' );
if ( defined($env) ? $env : ($stats || $class->debug ) ) {
no strict 'refs';
+ #Moose todo: dying to be made a bool attribute
*{"$class\::use_stats"} = sub { 1 };
$class->log->debug('Statistics enabled');
}
More information about the Catalyst-commits
mailing list