[Catalyst-commits] r7524 - in Catalyst-Runtime/5.80/branches/moose:
. lib lib/Catalyst
groditi at dev.catalyst.perl.org
groditi at dev.catalyst.perl.org
Wed Mar 26 00:45:31 GMT 2008
Author: groditi
Date: 2008-03-26 00:45:31 +0000 (Wed, 26 Mar 2008)
New Revision: 7524
Modified:
Catalyst-Runtime/5.80/branches/moose/TODO
Catalyst-Runtime/5.80/branches/moose/lib/Catalyst.pm
Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Action.pm
Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Dispatcher.pm
Log:
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
Modified: Catalyst-Runtime/5.80/branches/moose/TODO
===================================================================
--- Catalyst-Runtime/5.80/branches/moose/TODO 2008-03-25 23:06:21 UTC (rev 7523)
+++ Catalyst-Runtime/5.80/branches/moose/TODO 2008-03-26 00:45:31 UTC (rev 7524)
@@ -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/branches/moose/lib/Catalyst/Action.pm
===================================================================
--- Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Action.pm 2008-03-25 23:06:21 UTC (rev 7523)
+++ Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Action.pm 2008-03-26 00:45:31 UTC (rev 7524)
@@ -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/branches/moose/lib/Catalyst/Dispatcher.pm
===================================================================
--- Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Dispatcher.pm 2008-03-25 23:06:21 UTC (rev 7523)
+++ Catalyst-Runtime/5.80/branches/moose/lib/Catalyst/Dispatcher.pm 2008-03-26 00:45:31 UTC (rev 7524)
@@ -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/branches/moose/lib/Catalyst.pm
===================================================================
--- Catalyst-Runtime/5.80/branches/moose/lib/Catalyst.pm 2008-03-25 23:06:21 UTC (rev 7523)
+++ Catalyst-Runtime/5.80/branches/moose/lib/Catalyst.pm 2008-03-26 00:45:31 UTC (rev 7524)
@@ -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