Support for Dancer2 #2

Open
wants to merge 2 commits into
from

Projects

None yet

2 participants

@sukria
sukria commented Mar 25, 2012

These are the needed patches to make it pass against the last version of Dancer2 (master branch on GitHub).

Important note: it can't pass on Dancer 1 and Dancer 2 at the same time because of a change in the arguments passed to registered coderefs in Dancer2 :

In Dancer 1 we did :

register something => sub {
my ($arg1, $arg2) = @_;
};

In Dancer 2 we have :

register something => sub {
my $dsl = shift;
my ($arg1, $arg2) = @_;
};

If we want the plugin to support both versions, we'll need to have some extra code to support both syntax.

@sukria sukria commented on the diff Mar 25, 2012
lib/Dancer/Plugin/FlashMessage.pm
@@ -16,29 +16,45 @@ my $session_hash_key = $conf->{session_hash_key} || '_flash';
my $session_engine;
-register flash => sub ($;$) {
+set template => 'template_toolkit';
@sukria
sukria Mar 25, 2012

Yes, by the way, the way the plugin works depends on TT : it uses coderefs in the tokens hash, for instance Template::Tiny which is the default one in Dancer 2 does not support that.

Maybe the plugin wants to document that?

@sukria sukria commented on the diff Mar 25, 2012
lib/Dancer/Plugin/FlashMessage.pm
@@ -16,29 +16,45 @@ my $session_hash_key = $conf->{session_hash_key} || '_flash';
my $session_engine;
-register flash => sub ($;$) {
+set template => 'template_toolkit';
+
+register flash => sub {
+ my $dsl = shift;
@sukria
sukria Mar 25, 2012

The main change, shifting $dsl before processing arguments. This allows a cleaner syntax in the plugin code ($dsl->keyword)

@dams
dams Apr 2, 2012 Owner

as discussed, I think Dancer2 should ship Dancer::Template::Tiny or something

@sukria
sukria Apr 3, 2012

Well, this won't change the fact that under Dancer 2, a sub passed to "register" must shift first the $dsl object ;)

@dams
dams Apr 3, 2012 Owner

Sorry I wrote my note at the wrong place :) I intended to comment the set template => 'template_toolkit'. But yes, I agree :)

@sukria sukria commented on the diff Mar 25, 2012
lib/Dancer/Plugin/FlashMessage.pm
+
+ $tokens->{$token_name} = {
+ map {
+ my $key = $_;
+ my $value;
+ ( $key,
+ sub {
+ defined $value and return $value;
+ my $flash = session($session_hash_key) || {};
+ $value = delete $flash->{$key};
+ session $session_hash_key, $flash;
+ return $value;
+ }
+ );
+ } @keys_in_session
+ };
@sukria
sukria Mar 25, 2012

Oh, Perl::Tidy went over this apparently, sorry ;)

@sukria sukria commented on the diff Mar 25, 2012
lib/Dancer/Plugin/FlashMessage.pm
my ($key, $value) = @_;
- $session_engine ||= engine 'session'
- or croak __PACKAGE__ . " error2 : there is no session engine configured in the configuration. You need a session engine to be able to use this plugin";
+ if (! defined $session_engine) {
+ eval { $session_engine = $dsl->engine( 'session' ) };
@sukria
sukria Mar 25, 2012

In Dancer 2 $dsl->engine triggers an exception if the engine is not set. Don't know if you really want to wrap that?

Also, why not doing that in init time?

@dams
dams Apr 2, 2012 Owner

I don't want to do that at init time, because I initially did, but realized that people actually change their session engine on the fly. Yes they do :)
So need to do the check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment