Re: [Dancer-users] Flash Message
Hi again, So I juste released Dancer-Plugin-FlashMessage-0.308 After discussing with sukria, sawyer, franck, I decided to go with the minimalistic implementation of the Flash Message Plugin : Messages are always associated to a name, and stored in a hashref in the session There is no persistence option, flash messages are deleted either when used in the template, or when retrieved using flash($key) I think this solution makes it very easy to use the plugin. Also, it's similar to what exists in other web framework. And so it leaves the room for a Dancer::Plugin::FlashMessage::Advanced, or however you want to call it,for anyone (Flavio ?) that needs a more advanced set of features. I hope that's OK with you guys. Thanks again for the feedback :) dams On 30 January 2011 21:17, Flavio Poletti <polettix@gmail.com> wrote:
https://github.com/polettix/Dancer-Plugin-FlashMessage/commit/b36a20d2b8d355...
On Sun, Jan 30, 2011 at 8:28 PM, Brian E. Lozier <brian@massassi.com> wrote:
Where on github can I see comments?
On Sun, Jan 30, 2011 at 2:49 AM, damien krotkine <dkrotkine@gmail.com> wrote:
Hi !
I've commented on github. Let's swithc to that for the following discussion indeed. Brian, you are more than welcome to participate :)
dams
On 29 January 2011 14:59, Flavio Poletti <polettix@gmail.com> wrote:
Hi all, I don't know if this mailing list is actually the right place to continue this discussion about this module implementation, let us know if you're bored! I implemented a generic approach and you can find it
here: https://github.com/polettix/Dancer-Plugin-FlashMessage/tree/alternatives It is based on the configurability of three traits: * how messages are registered ("queue" option) * how additional parameters are handled ("arguments" option) * how messages are flushed away ("dequeue" option). There is a lot of new meat so I also added the full documentation with some examples. This implementation is kinda back-compatible, with two outstanding exceptions: * persistence has been dropped, even though its functionality has been absorbed by "dequeue" * the 'flash' method now only *sets* messages, and it never deletes them. IMHO, the case for deletion in the code is quite obscure, so I tried to move this functionality inside the new "flash_flush" registered keyword. It is anyway possible to revert to the old behaviour if it is considered better. For any other aspects, the current default values replicate the previous behaviour. If it makes sense to you we can add a plethora of tests. Thank you all for your patience, we'll probably better continue this discussion on GitHub if you agree. Flavio.
On Fri, Jan 28, 2011 at 7:05 PM, Flavio Poletti <polettix@gmail.com> wrote:
On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine <dkrotkine@gmail.com> wrote:
On 28 January 2011 18:40, Flavio Poletti <polettix@gmail.com> wrote: > Hi, > > the implementation is sound, even though I'd prefer to be able > to > set > multiple messages per single channel. If I validate a form, for > example, I'd > like to flash a warning for each field I don't like, not just the > last > one.
Hm, my brain being what it is on friday night, could you elaborate a bit more ? can you give an example of such channels ?
I'm thinking you could use an array of string as flash.warnings, instead of a simple string, but I'm not sure I understand what you're saying
Sorry, you got the "channel" idea. I'd like to use flash like this:
flash warning => 'you made error X';
and then, possibly later, say:
flash warning => 'you made error Y';
In the current implementation, only the last gets to the template. I'm suggesting to always push the incoming $value into an array. Otherwise, with the current implementation that lets you delete stuff, I'd be forced to do it like this:
my $array = flash('warning') || []; push @$array, 'you made error X'; flash warning => $array;
which is a bit ugly...
> On the other hand, deleting flash messages from the controller does > seem to > be a corner case, which I'd handle with an ad-hoc function or > ignore > completely. > > I'll try to propose something on GitHub that lets the end user > decide > the > preferred semantics through a configuration; I would personally > like to > avoid having leftovers in the flash when my templates are not > bug-free > and I > definitely don't like the idea of remembering to call > flash->flush()
I agree. After all, maybe it would make sense to have 3 behaviors : behaviour 1 : delete all the keys as soon as one is used (to be implemented ) behaviour 2 : delete only the key which is used ( current implementation ) behaviour 3 : never delete the keys ( currently persistence = 1 )
And maybe behaviour 1 should be the default, I don't know. As long as behaviour 2 is available, I'm happy (and brian too)
maybe the behaviour could be chosen with the persistence config keyword ? It could be renamed to flush_behaviour
flush_behaviour = key_level flush_behaviour = hash_level flush_behaviour = never
What do you think ? OK I'm terrible at finding good names, so any help welcome :)
I'd leave "_behaviour", add an 'always' (your previous implementation) and put something like:
flush = 'key' flush = 'all' flush = 'always' flush = 'never'
Anyway, as long as it's documented and it's reasonably connected to the behaviour everything is fine :-)
> (even > though it sounds so nice!)
aha, yes, I like flash->flush :) Let's add it for fun, even if it's not necessary :)
> > Cheers, > > Flavio. > > On Fri, Jan 28, 2011 at 6:33 PM, damien krotkine > <dkrotkine@gmail.com> > wrote: >> >> I agree with Brian, for a slightly different reason : >> >> Let's imagine this: >> >> you have a Template1, that does this : <% flash.keys() %>, to be >> able >> to know that they are errors. If that's the case, it doesn't >> display >> the errors, but display a link, saying that they are erros. This >> link >> does a new request against the server, serving Template2 >> >> Template2 does use flash.error, and effectively consumes the >> message. >> >> You don't want the flash hash structure to be emptied just after >> having displayed Template1, you want the messages to survive until >> Template2 (after all, you didn't consume them in Template1) >> >> If I understand correctly, In the original implementation, the >> simple >> fact of getting the keys of the hash would delete all the >> messages, so >> the next request, Template2 won't be able to retrieve <% >> flash.error >> %> >> >> In my new implementation, it would work. >> >> Granted, this is a bit of a corner case, however, I don't see any >> drawback. >> >> I could also add a flash->flush() method. >> >> >> On 28 January 2011 18:09, Brian E. Lozier <brian@massassi.com> >> wrote: >> > I can't think of any real world use case where you'd only want >> > to >> > show >> > certain messages. That said, it still seems better to only >> > clear >> > the >> > messages you use, not all of them. >> > >> > On Fri, Jan 28, 2011 at 9:02 AM, Flavio Poletti >> > <polettix@gmail.com> >> > wrote: >> >> Hi, >> >> >> >> of course we can try to adapt to it (it seems a bit >> >> difficult >> >> although >> >> probably not impossible), but I fail to see the point. >> >> >> >> IMHO, if you get the chance to show the messages, you should >> >> show >> >> them >> >> all: >> >> showing warnings in one page and errors in the following one >> >> would >> >> puzzle >> >> the end user, wouldn't it? >> >> >> >> Unless there is some use case that point in the opposite >> >> direction >> >> I'd >> >> stick >> >> to the "wipe them all when you get the chance to show them" >> >> semantics. >> >> >> >> Cheers, >> >> >> >> Flavio. >> >> >> >> On Fri, Jan 28, 2011 at 5:32 PM, Brian E. Lozier >> >> <brian@massassi.com> >> >> wrote: >> >>> >> >>> It looks like a pretty good solution. The only thing I can >> >>> see >> >>> would >> >>> be that if you are in a template and check for flash.error but >> >>> not >> >>> flash.warning, it looks like it will clear both flash.error >> >>> and >> >>> flash.warning. It seems like it should only clear the one >> >>> you're >> >>> checking for. >> >>> >> >>> >> >>> >> >>> On Fri, Jan 28, 2011 at 12:02 AM, Flavio Poletti >> >>> <polettix@gmail.com> >> >>> wrote: >> >>> > Hi, >> >>> > I thought a bit about it and I think that the current >> >>> > implementation >> >>> > is a >> >>> > bit too limited (this seems also what brian thinks in the >> >>> > bug >> >>> > report http://rt.cpan.org/Public/Bug/Display.html?id=65009 - >> >>> > at >> >>> > least >> >>> > from >> >>> > what I understand). >> >>> > The problem is that the flash message might have to be >> >>> > displayed >> >>> > in >> >>> > the >> >>> > next >> >>> > request, which happens when redirections kick in. In this >> >>> > case, >> >>> > the >> >>> > flash >> >>> > message should be set, kept in the session and removed only >> >>> > when >> >>> > it >> >>> > is >> >>> > actually displayed; this should happen automatically, i.e. >> >>> > without >> >>> > playing >> >>> > with the $persistent variable - which is global - or with >> >>> > further >> >>> > overhead >> >>> > on the controller side. >> >>> > Following brian's feedback, I tried to re-code the plugin >> >>> > with >> >>> > the >> >>> > following >> >>> > criteria: >> >>> > * the "flash" method only adds messages (and handles many of >> >>> > them >> >>> > for >> >>> > every >> >>> > category - you might have more than one error in a page). I >> >>> > see >> >>> > no >> >>> > point >> >>> > in >> >>> > removing messages while inside the controller; >> >>> > * the message removal logic is shifted in the template via >> >>> > an >> >>> > anonymous >> >>> > sub. >> >>> > If the sub is called then the flash cache is cleared, >> >>> > otherwise >> >>> > it >> >>> > remains >> >>> > there. This should guarantee that the messages are not >> >>> > removed >> >>> > until >> >>> > they >> >>> > get the chance to be displayed. >> >>> > This is the core of the recoding - not tested but should >> >>> > suffice >> >>> > to >> >>> > give >> >>> > you >> >>> > the idea: >> >>> > register flash => sub { >> >>> > my ($key, $value) = @_; >> >>> > my $flash = session($session_hash_key); >> >>> > if (! $flash) { # initialise the container for flash >> >>> > messages >> >>> > $flash = {}; >> >>> > session($session_hash_key, $flash); >> >>> > } >> >>> > push @{$flash->{$key}}, $value; >> >>> > return; >> >>> > }; >> >>> > before_template sub { >> >>> > my $obj = shift; >> >>> > my $flashes; >> >>> > $obj->{$token_name} = sub { >> >>> > if (! $flashes) { # first call, get messages and clear >> >>> > $flashes = session($session_hash_key) || {}; >> >>> > session($session_hash_key, {}); >> >>> > } >> >>> > return $flashes; >> >>> > }; >> >>> > return; >> >>> > }; >> >>> > >> >>> > IMHO there is no need for an "exists", in the template you >> >>> > can >> >>> > do >> >>> > like >> >>> > this: >> >>> > <% IF flash.errors %>...<% END %> >> >>> > The important thing to remember is that even checking for >> >>> > the >> >>> > presence >> >>> > of >> >>> > something inside "flash" clears the messages, which is >> >>> > consistent >> >>> > with >> >>> > the >> >>> > semantics that "they get cleared as soon as they have a >> >>> > chance >> >>> > to be >> >>> > displayed". The $flashes variable guarantees that they >> >>> > survive >> >>> > at >> >>> > least >> >>> > until the end of the request, i.e. you are able to call >> >>> > "flash" >> >>> > multiple >> >>> > times while handling a request and always get all messages. >> >>> > If you think that this makes sense, I can propose a pull >> >>> > request >> >>> > on >> >>> > GitHub >> >>> > or you can make the changes directly. >> >>> > Cheers, >> >>> > Flavio. >> >>> > >> >>> > >> >>> > On Tue, Jan 11, 2011 at 5:48 PM, damien krotkine >> >>> > <dkrotkine@gmail.com> >> >>> > wrote: >> >>> >> >> >>> >> I've re-implemented it to be more Rails-like, as sukria >> >>> >> said. >> >>> >> >> >>> >> https://github.com/dams/Dancer-Plugin-FlashMessage >> >>> >> >> >>> >> and on CPAN, pending mirrors refresh. >> >>> >> >> >>> >> The funny part of the story ? the effective code is only 30 >> >>> >> lines >> >>> >> long. Talking about Perl and Dancer expressiveness... >> >>> >> >> >>> >> dams. >> >>> >> >> >>> >> On 11 January 2011 14:40, Alexis Sukrieh >> >>> >> <sukria@sukria.net> >> >>> >> wrote: >> >>> >> > Hi list! >> >>> >> > >> >>> >> > Le 11/01/2011 14:29, damien krotkine a écrit : >> >>> >> >> >> >>> >> >> Hi, >> >>> >> >> >> >>> >> >> following previous thread, I've done a first >> >>> >> >> implementation >> >>> >> >> of >> >>> >> >> Dancer::Plugin::FlashMessage : >> >>> >> >> >> >>> >> >> https://github.com/dams/Dancer-Plugin-FlashMessage >> >>> >> > >> >>> >> > Great! Thanks a lot for your time dams, the myth is still >> >>> >> > alived! >> >>> >> > (Dancer's >> >>> >> > community)++ >> >>> >> > >> >>> >> > >> >>> >> > >> >>> >> >> Some parts need to be improved, for instance : >> >>> >> >> >> >>> >> >> - it supports only one flash message >> >>> >> >> - the keywords are not short enough. >> >>> >> >> >> >>> >> >> So I think I'll change the implementation so that the >> >>> >> >> template >> >>> >> >> token >> >>> >> >> is >> >>> >> >> simply called 'flash', and it'll be a hash, like in >> >>> >> >> Rails. >> >>> >> >> I'll >> >>> >> >> also >> >>> >> >> change the registered method so that it's just flash() >> >>> >> >> instead >> >>> >> >> of >> >>> >> >> get_flash() >> >>> >> > >> >>> >> > I agree. I'd like to behave just like Rails' flash >> >>> >> > feature. >> >>> >> > The >> >>> >> > idea >> >>> >> > is >> >>> >> > pretty straight forward: >> >>> >> > >> >>> >> > "flash" is an accessor to a particular session hash table >> >>> >> > whose >> >>> >> > values >> >>> >> > can >> >>> >> > only be accessed once. Nothing more complicated than >> >>> >> > that. >> >>> >> > >> >>> >> > So to conclude, IMO, flash should be a wrapper like the >> >>> >> > following: >> >>> >> > >> >>> >> > sub flash { >> >>> >> > my ($key, $value) = @_; >> >>> >> > my $flash = session('_flash'); >> >>> >> > >> >>> >> > # write >> >>> >> > if (@_ == 2) { >> >>> >> > $flash->{$key} = $value; >> >>> >> > session('_flash' => $flash); >> >>> >> > } >> >>> >> > >> >>> >> > # read (+ delete) >> >>> >> > else { >> >>> >> > my $value = $flash->{$key}; >> >>> >> > delete $flash->{$key} if defined $value; >> >>> >> > session('_flash' => $flash); >> >>> >> > } >> >>> >> > >> >>> >> > return $value; >> >>> >> > } >> >>> >> > >> >>> >> > This is it, I think. This allows for the following code >> >>> >> > in a >> >>> >> > Dancer >> >>> >> > app: >> >>> >> > >> >>> >> > >> >>> >> > get '/' => sub { >> >>> >> > flash welcome => "This is a welcome message, only >> >>> >> > shown >> >>> >> > once"; >> >>> >> > } >> >>> >> > >> >>> >> > Then, as soon as the key 'welcome' is accesed via >> >>> >> > flash('welcome'), >> >>> >> > the >> >>> >> > entry will be purged. >> >>> >> > >> >>> >> > This will be very helpful for authentication stuff in >> >>> >> > before >> >>> >> > filters, >> >>> >> > error >> >>> >> > messages, notifications, .... >> >>> >> > >> >>> >> > >> >>> >> > Kudos to dams! >> >>> >> > >> >>> >> > (BTW I haven't read the code yet) >> >>> >> > >> >>> >> > -- >> >>> >> > Alexis Sukrieh >> >>> >> > _______________________________________________ >> >>> >> > Dancer-users mailing list >> >>> >> > Dancer-users@perldancer.org >> >>> >> > >> >>> >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >> >>> >> > >> >>> >> _______________________________________________ >> >>> >> Dancer-users mailing list >> >>> >> Dancer-users@perldancer.org >> >>> >> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >> >>> > >> >>> > >> >>> > _______________________________________________ >> >>> > Dancer-users mailing list >> >>> > Dancer-users@perldancer.org >> >>> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >> >>> > >> >>> > >> >>> _______________________________________________ >> >>> Dancer-users mailing list >> >>> Dancer-users@perldancer.org >> >>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >> >> >> >> >> > _______________________________________________ >> > Dancer-users mailing list >> > Dancer-users@perldancer.org >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >> > > >
Great. Will use it, for sure. Needing something like that :) Alberto On 05/02/2011 12:18, damien krotkine wrote:
Hi again,
So I juste released Dancer-Plugin-FlashMessage-0.308
After discussing with sukria, sawyer, franck, I decided to go with the minimalistic implementation of the Flash Message Plugin :
Messages are always associated to a name, and stored in a hashref in the session
There is no persistence option, flash messages are deleted either when used in the template, or when retrieved using flash($key)
I think this solution makes it very easy to use the plugin. Also, it's similar to what exists in other web framework.
And so it leaves the room for a Dancer::Plugin::FlashMessage::Advanced, or however you want to call it,for anyone (Flavio ?) that needs a more advanced set of features.
I hope that's OK with you guys. Thanks again for the feedback :)
dams
On 30 January 2011 21:17, Flavio Poletti<polettix@gmail.com> wrote:
https://github.com/polettix/Dancer-Plugin-FlashMessage/commit/b36a20d2b8d355...
On Sun, Jan 30, 2011 at 8:28 PM, Brian E. Lozier<brian@massassi.com> wrote:
Where on github can I see comments?
On Sun, Jan 30, 2011 at 2:49 AM, damien krotkine<dkrotkine@gmail.com> wrote:
Hi !
I've commented on github. Let's swithc to that for the following discussion indeed. Brian, you are more than welcome to participate :)
dams
On 29 January 2011 14:59, Flavio Poletti<polettix@gmail.com> wrote:
Hi all, I don't know if this mailing list is actually the right place to continue this discussion about this module implementation, let us know if you're bored! I implemented a generic approach and you can find it
here: https://github.com/polettix/Dancer-Plugin-FlashMessage/tree/alternatives It is based on the configurability of three traits: * how messages are registered ("queue" option) * how additional parameters are handled ("arguments" option) * how messages are flushed away ("dequeue" option). There is a lot of new meat so I also added the full documentation with some examples. This implementation is kinda back-compatible, with two outstanding exceptions: * persistence has been dropped, even though its functionality has been absorbed by "dequeue" * the 'flash' method now only *sets* messages, and it never deletes them. IMHO, the case for deletion in the code is quite obscure, so I tried to move this functionality inside the new "flash_flush" registered keyword. It is anyway possible to revert to the old behaviour if it is considered better. For any other aspects, the current default values replicate the previous behaviour. If it makes sense to you we can add a plethora of tests. Thank you all for your patience, we'll probably better continue this discussion on GitHub if you agree. Flavio.
On Fri, Jan 28, 2011 at 7:05 PM, Flavio Poletti<polettix@gmail.com> wrote:
On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine<dkrotkine@gmail.com> wrote: > > On 28 January 2011 18:40, Flavio Poletti<polettix@gmail.com> wrote: >> Hi, >> >> the implementation is sound, even though I'd prefer to be able >> to >> set >> multiple messages per single channel. If I validate a form, for >> example, I'd >> like to flash a warning for each field I don't like, not just the >> last >> one. > > Hm, my brain being what it is on friday night, could you elaborate a > bit more ? can you give an example of such channels ? > > I'm thinking you could use an array of string as flash.warnings, > instead of a simple string, but I'm not sure I understand what you're > saying
Sorry, you got the "channel" idea. I'd like to use flash like this:
flash warning => 'you made error X';
and then, possibly later, say:
flash warning => 'you made error Y';
In the current implementation, only the last gets to the template. I'm suggesting to always push the incoming $value into an array. Otherwise, with the current implementation that lets you delete stuff, I'd be forced to do it like this:
my $array = flash('warning') || []; push @$array, 'you made error X'; flash warning => $array;
which is a bit ugly...
> >> On the other hand, deleting flash messages from the controller does >> seem to >> be a corner case, which I'd handle with an ad-hoc function or >> ignore >> completely. >> >> I'll try to propose something on GitHub that lets the end user >> decide >> the >> preferred semantics through a configuration; I would personally >> like to >> avoid having leftovers in the flash when my templates are not >> bug-free >> and I >> definitely don't like the idea of remembering to call >> flash->flush() > > I agree. After all, maybe it would make sense to have 3 behaviors : > behaviour 1 : delete all the keys as soon as one is used (to be > implemented ) > behaviour 2 : delete only the key which is used ( current > implementation > ) > behaviour 3 : never delete the keys ( currently persistence = 1 ) > > And maybe behaviour 1 should be the default, I don't know. As long as > behaviour 2 is available, I'm happy (and brian too) > > maybe the behaviour could be chosen with the persistence config > keyword ? It could be renamed to flush_behaviour > > flush_behaviour = key_level > flush_behaviour = hash_level > flush_behaviour = never > > What do you think ? OK I'm terrible at finding good names, so any > help > welcome :)
I'd leave "_behaviour", add an 'always' (your previous implementation) and put something like:
flush = 'key' flush = 'all' flush = 'always' flush = 'never'
Anyway, as long as it's documented and it's reasonably connected to the behaviour everything is fine :-)
> >> (even >> though it sounds so nice!) > > aha, yes, I like flash->flush :) Let's add it for fun, even if it's > not necessary :) > > >> >> Cheers, >> >> Flavio. >> >> On Fri, Jan 28, 2011 at 6:33 PM, damien krotkine >> <dkrotkine@gmail.com> >> wrote: >>> >>> I agree with Brian, for a slightly different reason : >>> >>> Let's imagine this: >>> >>> you have a Template1, that does this :<% flash.keys() %>, to be >>> able >>> to know that they are errors. If that's the case, it doesn't >>> display >>> the errors, but display a link, saying that they are erros. This >>> link >>> does a new request against the server, serving Template2 >>> >>> Template2 does use flash.error, and effectively consumes the >>> message. >>> >>> You don't want the flash hash structure to be emptied just after >>> having displayed Template1, you want the messages to survive until >>> Template2 (after all, you didn't consume them in Template1) >>> >>> If I understand correctly, In the original implementation, the >>> simple >>> fact of getting the keys of the hash would delete all the >>> messages, so >>> the next request, Template2 won't be able to retrieve<% >>> flash.error >>> %> >>> >>> In my new implementation, it would work. >>> >>> Granted, this is a bit of a corner case, however, I don't see any >>> drawback. >>> >>> I could also add a flash->flush() method. >>> >>> >>> On 28 January 2011 18:09, Brian E. Lozier<brian@massassi.com> >>> wrote: >>>> I can't think of any real world use case where you'd only want >>>> to >>>> show >>>> certain messages. That said, it still seems better to only >>>> clear >>>> the >>>> messages you use, not all of them. >>>> >>>> On Fri, Jan 28, 2011 at 9:02 AM, Flavio Poletti >>>> <polettix@gmail.com> >>>> wrote: >>>>> Hi, >>>>> >>>>> of course we can try to adapt to it (it seems a bit >>>>> difficult >>>>> although >>>>> probably not impossible), but I fail to see the point. >>>>> >>>>> IMHO, if you get the chance to show the messages, you should >>>>> show >>>>> them >>>>> all: >>>>> showing warnings in one page and errors in the following one >>>>> would >>>>> puzzle >>>>> the end user, wouldn't it? >>>>> >>>>> Unless there is some use case that point in the opposite >>>>> direction >>>>> I'd >>>>> stick >>>>> to the "wipe them all when you get the chance to show them" >>>>> semantics. >>>>> >>>>> Cheers, >>>>> >>>>> Flavio. >>>>> >>>>> On Fri, Jan 28, 2011 at 5:32 PM, Brian E. Lozier >>>>> <brian@massassi.com> >>>>> wrote: >>>>>> >>>>>> It looks like a pretty good solution. The only thing I can >>>>>> see >>>>>> would >>>>>> be that if you are in a template and check for flash.error but >>>>>> not >>>>>> flash.warning, it looks like it will clear both flash.error >>>>>> and >>>>>> flash.warning. It seems like it should only clear the one >>>>>> you're >>>>>> checking for. >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Jan 28, 2011 at 12:02 AM, Flavio Poletti >>>>>> <polettix@gmail.com> >>>>>> wrote: >>>>>>> Hi, >>>>>>> I thought a bit about it and I think that the current >>>>>>> implementation >>>>>>> is a >>>>>>> bit too limited (this seems also what brian thinks in the >>>>>>> bug >>>>>>> report http://rt.cpan.org/Public/Bug/Display.html?id=65009 - >>>>>>> at >>>>>>> least >>>>>>> from >>>>>>> what I understand). >>>>>>> The problem is that the flash message might have to be >>>>>>> displayed >>>>>>> in >>>>>>> the >>>>>>> next >>>>>>> request, which happens when redirections kick in. In this >>>>>>> case, >>>>>>> the >>>>>>> flash >>>>>>> message should be set, kept in the session and removed only >>>>>>> when >>>>>>> it >>>>>>> is >>>>>>> actually displayed; this should happen automatically, i.e. >>>>>>> without >>>>>>> playing >>>>>>> with the $persistent variable - which is global - or with >>>>>>> further >>>>>>> overhead >>>>>>> on the controller side. >>>>>>> Following brian's feedback, I tried to re-code the plugin >>>>>>> with >>>>>>> the >>>>>>> following >>>>>>> criteria: >>>>>>> * the "flash" method only adds messages (and handles many of >>>>>>> them >>>>>>> for >>>>>>> every >>>>>>> category - you might have more than one error in a page). I >>>>>>> see >>>>>>> no >>>>>>> point >>>>>>> in >>>>>>> removing messages while inside the controller; >>>>>>> * the message removal logic is shifted in the template via >>>>>>> an >>>>>>> anonymous >>>>>>> sub. >>>>>>> If the sub is called then the flash cache is cleared, >>>>>>> otherwise >>>>>>> it >>>>>>> remains >>>>>>> there. This should guarantee that the messages are not >>>>>>> removed >>>>>>> until >>>>>>> they >>>>>>> get the chance to be displayed. >>>>>>> This is the core of the recoding - not tested but should >>>>>>> suffice >>>>>>> to >>>>>>> give >>>>>>> you >>>>>>> the idea: >>>>>>> register flash => sub { >>>>>>> my ($key, $value) = @_; >>>>>>> my $flash = session($session_hash_key); >>>>>>> if (! $flash) { # initialise the container for flash >>>>>>> messages >>>>>>> $flash = {}; >>>>>>> session($session_hash_key, $flash); >>>>>>> } >>>>>>> push @{$flash->{$key}}, $value; >>>>>>> return; >>>>>>> }; >>>>>>> before_template sub { >>>>>>> my $obj = shift; >>>>>>> my $flashes; >>>>>>> $obj->{$token_name} = sub { >>>>>>> if (! $flashes) { # first call, get messages and clear >>>>>>> $flashes = session($session_hash_key) || {}; >>>>>>> session($session_hash_key, {}); >>>>>>> } >>>>>>> return $flashes; >>>>>>> }; >>>>>>> return; >>>>>>> }; >>>>>>> >>>>>>> IMHO there is no need for an "exists", in the template you >>>>>>> can >>>>>>> do >>>>>>> like >>>>>>> this: >>>>>>> <% IF flash.errors %>...<% END %> >>>>>>> The important thing to remember is that even checking for >>>>>>> the >>>>>>> presence >>>>>>> of >>>>>>> something inside "flash" clears the messages, which is >>>>>>> consistent >>>>>>> with >>>>>>> the >>>>>>> semantics that "they get cleared as soon as they have a >>>>>>> chance >>>>>>> to be >>>>>>> displayed". The $flashes variable guarantees that they >>>>>>> survive >>>>>>> at >>>>>>> least >>>>>>> until the end of the request, i.e. you are able to call >>>>>>> "flash" >>>>>>> multiple >>>>>>> times while handling a request and always get all messages. >>>>>>> If you think that this makes sense, I can propose a pull >>>>>>> request >>>>>>> on >>>>>>> GitHub >>>>>>> or you can make the changes directly. >>>>>>> Cheers, >>>>>>> Flavio. >>>>>>> >>>>>>> >>>>>>> On Tue, Jan 11, 2011 at 5:48 PM, damien krotkine >>>>>>> <dkrotkine@gmail.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> I've re-implemented it to be more Rails-like, as sukria >>>>>>>> said. >>>>>>>> >>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>> >>>>>>>> and on CPAN, pending mirrors refresh. >>>>>>>> >>>>>>>> The funny part of the story ? the effective code is only 30 >>>>>>>> lines >>>>>>>> long. Talking about Perl and Dancer expressiveness... >>>>>>>> >>>>>>>> dams. >>>>>>>> >>>>>>>> On 11 January 2011 14:40, Alexis Sukrieh >>>>>>>> <sukria@sukria.net> >>>>>>>> wrote: >>>>>>>>> Hi list! >>>>>>>>> >>>>>>>>> Le 11/01/2011 14:29, damien krotkine a écrit : >>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> following previous thread, I've done a first >>>>>>>>>> implementation >>>>>>>>>> of >>>>>>>>>> Dancer::Plugin::FlashMessage : >>>>>>>>>> >>>>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>>> >>>>>>>>> Great! Thanks a lot for your time dams, the myth is still >>>>>>>>> alived! >>>>>>>>> (Dancer's >>>>>>>>> community)++ >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> Some parts need to be improved, for instance : >>>>>>>>>> >>>>>>>>>> - it supports only one flash message >>>>>>>>>> - the keywords are not short enough. >>>>>>>>>> >>>>>>>>>> So I think I'll change the implementation so that the >>>>>>>>>> template >>>>>>>>>> token >>>>>>>>>> is >>>>>>>>>> simply called 'flash', and it'll be a hash, like in >>>>>>>>>> Rails. >>>>>>>>>> I'll >>>>>>>>>> also >>>>>>>>>> change the registered method so that it's just flash() >>>>>>>>>> instead >>>>>>>>>> of >>>>>>>>>> get_flash() >>>>>>>>> >>>>>>>>> I agree. I'd like to behave just like Rails' flash >>>>>>>>> feature. >>>>>>>>> The >>>>>>>>> idea >>>>>>>>> is >>>>>>>>> pretty straight forward: >>>>>>>>> >>>>>>>>> "flash" is an accessor to a particular session hash table >>>>>>>>> whose >>>>>>>>> values >>>>>>>>> can >>>>>>>>> only be accessed once. Nothing more complicated than >>>>>>>>> that. >>>>>>>>> >>>>>>>>> So to conclude, IMO, flash should be a wrapper like the >>>>>>>>> following: >>>>>>>>> >>>>>>>>> sub flash { >>>>>>>>> my ($key, $value) = @_; >>>>>>>>> my $flash = session('_flash'); >>>>>>>>> >>>>>>>>> # write >>>>>>>>> if (@_ == 2) { >>>>>>>>> $flash->{$key} = $value; >>>>>>>>> session('_flash' => $flash); >>>>>>>>> } >>>>>>>>> >>>>>>>>> # read (+ delete) >>>>>>>>> else { >>>>>>>>> my $value = $flash->{$key}; >>>>>>>>> delete $flash->{$key} if defined $value; >>>>>>>>> session('_flash' => $flash); >>>>>>>>> } >>>>>>>>> >>>>>>>>> return $value; >>>>>>>>> } >>>>>>>>> >>>>>>>>> This is it, I think. This allows for the following code >>>>>>>>> in a >>>>>>>>> Dancer >>>>>>>>> app: >>>>>>>>> >>>>>>>>> >>>>>>>>> get '/' => sub { >>>>>>>>> flash welcome => "This is a welcome message, only >>>>>>>>> shown >>>>>>>>> once"; >>>>>>>>> } >>>>>>>>> >>>>>>>>> Then, as soon as the key 'welcome' is accesed via >>>>>>>>> flash('welcome'), >>>>>>>>> the >>>>>>>>> entry will be purged. >>>>>>>>> >>>>>>>>> This will be very helpful for authentication stuff in >>>>>>>>> before >>>>>>>>> filters, >>>>>>>>> error >>>>>>>>> messages, notifications, .... >>>>>>>>> >>>>>>>>> >>>>>>>>> Kudos to dams! >>>>>>>>> >>>>>>>>> (BTW I haven't read the code yet) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Alexis Sukrieh >>>>>>>>> _______________________________________________ >>>>>>>>> Dancer-users mailing list >>>>>>>>> Dancer-users@perldancer.org >>>>>>>>> >>>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Dancer-users mailing list >>>>>>>> Dancer-users@perldancer.org >>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Dancer-users mailing list >>>>>>> Dancer-users@perldancer.org >>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>> >>>>>>> >>>>>> _______________________________________________ >>>>>> Dancer-users mailing list >>>>>> Dancer-users@perldancer.org >>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>> >>>>> >>>> _______________________________________________ >>>> Dancer-users mailing list >>>> Dancer-users@perldancer.org >>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>> >> >>
_______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
Thank you! 0.310 working like a charm :) On 05/02/2011 12:18, damien krotkine wrote:
Hi again,
So I juste released Dancer-Plugin-FlashMessage-0.308
After discussing with sukria, sawyer, franck, I decided to go with the minimalistic implementation of the Flash Message Plugin :
Messages are always associated to a name, and stored in a hashref in the session
There is no persistence option, flash messages are deleted either when used in the template, or when retrieved using flash($key)
I think this solution makes it very easy to use the plugin. Also, it's similar to what exists in other web framework.
And so it leaves the room for a Dancer::Plugin::FlashMessage::Advanced, or however you want to call it,for anyone (Flavio ?) that needs a more advanced set of features.
I hope that's OK with you guys. Thanks again for the feedback :)
dams
On 30 January 2011 21:17, Flavio Poletti<polettix@gmail.com> wrote:
https://github.com/polettix/Dancer-Plugin-FlashMessage/commit/b36a20d2b8d355...
On Sun, Jan 30, 2011 at 8:28 PM, Brian E. Lozier<brian@massassi.com> wrote:
Where on github can I see comments?
On Sun, Jan 30, 2011 at 2:49 AM, damien krotkine<dkrotkine@gmail.com> wrote:
Hi !
I've commented on github. Let's swithc to that for the following discussion indeed. Brian, you are more than welcome to participate :)
dams
On 29 January 2011 14:59, Flavio Poletti<polettix@gmail.com> wrote:
Hi all, I don't know if this mailing list is actually the right place to continue this discussion about this module implementation, let us know if you're bored! I implemented a generic approach and you can find it
here: https://github.com/polettix/Dancer-Plugin-FlashMessage/tree/alternatives It is based on the configurability of three traits: * how messages are registered ("queue" option) * how additional parameters are handled ("arguments" option) * how messages are flushed away ("dequeue" option). There is a lot of new meat so I also added the full documentation with some examples. This implementation is kinda back-compatible, with two outstanding exceptions: * persistence has been dropped, even though its functionality has been absorbed by "dequeue" * the 'flash' method now only *sets* messages, and it never deletes them. IMHO, the case for deletion in the code is quite obscure, so I tried to move this functionality inside the new "flash_flush" registered keyword. It is anyway possible to revert to the old behaviour if it is considered better. For any other aspects, the current default values replicate the previous behaviour. If it makes sense to you we can add a plethora of tests. Thank you all for your patience, we'll probably better continue this discussion on GitHub if you agree. Flavio.
On Fri, Jan 28, 2011 at 7:05 PM, Flavio Poletti<polettix@gmail.com> wrote:
On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine<dkrotkine@gmail.com> wrote: > > On 28 January 2011 18:40, Flavio Poletti<polettix@gmail.com> wrote: >> Hi, >> >> the implementation is sound, even though I'd prefer to be able >> to >> set >> multiple messages per single channel. If I validate a form, for >> example, I'd >> like to flash a warning for each field I don't like, not just the >> last >> one. > > Hm, my brain being what it is on friday night, could you elaborate a > bit more ? can you give an example of such channels ? > > I'm thinking you could use an array of string as flash.warnings, > instead of a simple string, but I'm not sure I understand what you're > saying
Sorry, you got the "channel" idea. I'd like to use flash like this:
flash warning => 'you made error X';
and then, possibly later, say:
flash warning => 'you made error Y';
In the current implementation, only the last gets to the template. I'm suggesting to always push the incoming $value into an array. Otherwise, with the current implementation that lets you delete stuff, I'd be forced to do it like this:
my $array = flash('warning') || []; push @$array, 'you made error X'; flash warning => $array;
which is a bit ugly...
> >> On the other hand, deleting flash messages from the controller does >> seem to >> be a corner case, which I'd handle with an ad-hoc function or >> ignore >> completely. >> >> I'll try to propose something on GitHub that lets the end user >> decide >> the >> preferred semantics through a configuration; I would personally >> like to >> avoid having leftovers in the flash when my templates are not >> bug-free >> and I >> definitely don't like the idea of remembering to call >> flash->flush() > > I agree. After all, maybe it would make sense to have 3 behaviors : > behaviour 1 : delete all the keys as soon as one is used (to be > implemented ) > behaviour 2 : delete only the key which is used ( current > implementation > ) > behaviour 3 : never delete the keys ( currently persistence = 1 ) > > And maybe behaviour 1 should be the default, I don't know. As long as > behaviour 2 is available, I'm happy (and brian too) > > maybe the behaviour could be chosen with the persistence config > keyword ? It could be renamed to flush_behaviour > > flush_behaviour = key_level > flush_behaviour = hash_level > flush_behaviour = never > > What do you think ? OK I'm terrible at finding good names, so any > help > welcome :)
I'd leave "_behaviour", add an 'always' (your previous implementation) and put something like:
flush = 'key' flush = 'all' flush = 'always' flush = 'never'
Anyway, as long as it's documented and it's reasonably connected to the behaviour everything is fine :-)
> >> (even >> though it sounds so nice!) > > aha, yes, I like flash->flush :) Let's add it for fun, even if it's > not necessary :) > > >> >> Cheers, >> >> Flavio. >> >> On Fri, Jan 28, 2011 at 6:33 PM, damien krotkine >> <dkrotkine@gmail.com> >> wrote: >>> >>> I agree with Brian, for a slightly different reason : >>> >>> Let's imagine this: >>> >>> you have a Template1, that does this :<% flash.keys() %>, to be >>> able >>> to know that they are errors. If that's the case, it doesn't >>> display >>> the errors, but display a link, saying that they are erros. This >>> link >>> does a new request against the server, serving Template2 >>> >>> Template2 does use flash.error, and effectively consumes the >>> message. >>> >>> You don't want the flash hash structure to be emptied just after >>> having displayed Template1, you want the messages to survive until >>> Template2 (after all, you didn't consume them in Template1) >>> >>> If I understand correctly, In the original implementation, the >>> simple >>> fact of getting the keys of the hash would delete all the >>> messages, so >>> the next request, Template2 won't be able to retrieve<% >>> flash.error >>> %> >>> >>> In my new implementation, it would work. >>> >>> Granted, this is a bit of a corner case, however, I don't see any >>> drawback. >>> >>> I could also add a flash->flush() method. >>> >>> >>> On 28 January 2011 18:09, Brian E. Lozier<brian@massassi.com> >>> wrote: >>>> I can't think of any real world use case where you'd only want >>>> to >>>> show >>>> certain messages. That said, it still seems better to only >>>> clear >>>> the >>>> messages you use, not all of them. >>>> >>>> On Fri, Jan 28, 2011 at 9:02 AM, Flavio Poletti >>>> <polettix@gmail.com> >>>> wrote: >>>>> Hi, >>>>> >>>>> of course we can try to adapt to it (it seems a bit >>>>> difficult >>>>> although >>>>> probably not impossible), but I fail to see the point. >>>>> >>>>> IMHO, if you get the chance to show the messages, you should >>>>> show >>>>> them >>>>> all: >>>>> showing warnings in one page and errors in the following one >>>>> would >>>>> puzzle >>>>> the end user, wouldn't it? >>>>> >>>>> Unless there is some use case that point in the opposite >>>>> direction >>>>> I'd >>>>> stick >>>>> to the "wipe them all when you get the chance to show them" >>>>> semantics. >>>>> >>>>> Cheers, >>>>> >>>>> Flavio. >>>>> >>>>> On Fri, Jan 28, 2011 at 5:32 PM, Brian E. Lozier >>>>> <brian@massassi.com> >>>>> wrote: >>>>>> >>>>>> It looks like a pretty good solution. The only thing I can >>>>>> see >>>>>> would >>>>>> be that if you are in a template and check for flash.error but >>>>>> not >>>>>> flash.warning, it looks like it will clear both flash.error >>>>>> and >>>>>> flash.warning. It seems like it should only clear the one >>>>>> you're >>>>>> checking for. >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Jan 28, 2011 at 12:02 AM, Flavio Poletti >>>>>> <polettix@gmail.com> >>>>>> wrote: >>>>>>> Hi, >>>>>>> I thought a bit about it and I think that the current >>>>>>> implementation >>>>>>> is a >>>>>>> bit too limited (this seems also what brian thinks in the >>>>>>> bug >>>>>>> report http://rt.cpan.org/Public/Bug/Display.html?id=65009 - >>>>>>> at >>>>>>> least >>>>>>> from >>>>>>> what I understand). >>>>>>> The problem is that the flash message might have to be >>>>>>> displayed >>>>>>> in >>>>>>> the >>>>>>> next >>>>>>> request, which happens when redirections kick in. In this >>>>>>> case, >>>>>>> the >>>>>>> flash >>>>>>> message should be set, kept in the session and removed only >>>>>>> when >>>>>>> it >>>>>>> is >>>>>>> actually displayed; this should happen automatically, i.e. >>>>>>> without >>>>>>> playing >>>>>>> with the $persistent variable - which is global - or with >>>>>>> further >>>>>>> overhead >>>>>>> on the controller side. >>>>>>> Following brian's feedback, I tried to re-code the plugin >>>>>>> with >>>>>>> the >>>>>>> following >>>>>>> criteria: >>>>>>> * the "flash" method only adds messages (and handles many of >>>>>>> them >>>>>>> for >>>>>>> every >>>>>>> category - you might have more than one error in a page). I >>>>>>> see >>>>>>> no >>>>>>> point >>>>>>> in >>>>>>> removing messages while inside the controller; >>>>>>> * the message removal logic is shifted in the template via >>>>>>> an >>>>>>> anonymous >>>>>>> sub. >>>>>>> If the sub is called then the flash cache is cleared, >>>>>>> otherwise >>>>>>> it >>>>>>> remains >>>>>>> there. This should guarantee that the messages are not >>>>>>> removed >>>>>>> until >>>>>>> they >>>>>>> get the chance to be displayed. >>>>>>> This is the core of the recoding - not tested but should >>>>>>> suffice >>>>>>> to >>>>>>> give >>>>>>> you >>>>>>> the idea: >>>>>>> register flash => sub { >>>>>>> my ($key, $value) = @_; >>>>>>> my $flash = session($session_hash_key); >>>>>>> if (! $flash) { # initialise the container for flash >>>>>>> messages >>>>>>> $flash = {}; >>>>>>> session($session_hash_key, $flash); >>>>>>> } >>>>>>> push @{$flash->{$key}}, $value; >>>>>>> return; >>>>>>> }; >>>>>>> before_template sub { >>>>>>> my $obj = shift; >>>>>>> my $flashes; >>>>>>> $obj->{$token_name} = sub { >>>>>>> if (! $flashes) { # first call, get messages and clear >>>>>>> $flashes = session($session_hash_key) || {}; >>>>>>> session($session_hash_key, {}); >>>>>>> } >>>>>>> return $flashes; >>>>>>> }; >>>>>>> return; >>>>>>> }; >>>>>>> >>>>>>> IMHO there is no need for an "exists", in the template you >>>>>>> can >>>>>>> do >>>>>>> like >>>>>>> this: >>>>>>> <% IF flash.errors %>...<% END %> >>>>>>> The important thing to remember is that even checking for >>>>>>> the >>>>>>> presence >>>>>>> of >>>>>>> something inside "flash" clears the messages, which is >>>>>>> consistent >>>>>>> with >>>>>>> the >>>>>>> semantics that "they get cleared as soon as they have a >>>>>>> chance >>>>>>> to be >>>>>>> displayed". The $flashes variable guarantees that they >>>>>>> survive >>>>>>> at >>>>>>> least >>>>>>> until the end of the request, i.e. you are able to call >>>>>>> "flash" >>>>>>> multiple >>>>>>> times while handling a request and always get all messages. >>>>>>> If you think that this makes sense, I can propose a pull >>>>>>> request >>>>>>> on >>>>>>> GitHub >>>>>>> or you can make the changes directly. >>>>>>> Cheers, >>>>>>> Flavio. >>>>>>> >>>>>>> >>>>>>> On Tue, Jan 11, 2011 at 5:48 PM, damien krotkine >>>>>>> <dkrotkine@gmail.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> I've re-implemented it to be more Rails-like, as sukria >>>>>>>> said. >>>>>>>> >>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>> >>>>>>>> and on CPAN, pending mirrors refresh. >>>>>>>> >>>>>>>> The funny part of the story ? the effective code is only 30 >>>>>>>> lines >>>>>>>> long. Talking about Perl and Dancer expressiveness... >>>>>>>> >>>>>>>> dams. >>>>>>>> >>>>>>>> On 11 January 2011 14:40, Alexis Sukrieh >>>>>>>> <sukria@sukria.net> >>>>>>>> wrote: >>>>>>>>> Hi list! >>>>>>>>> >>>>>>>>> Le 11/01/2011 14:29, damien krotkine a écrit : >>>>>>>>>> >>>>>>>>>> Hi, >>>>>>>>>> >>>>>>>>>> following previous thread, I've done a first >>>>>>>>>> implementation >>>>>>>>>> of >>>>>>>>>> Dancer::Plugin::FlashMessage : >>>>>>>>>> >>>>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>>> >>>>>>>>> Great! Thanks a lot for your time dams, the myth is still >>>>>>>>> alived! >>>>>>>>> (Dancer's >>>>>>>>> community)++ >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> Some parts need to be improved, for instance : >>>>>>>>>> >>>>>>>>>> - it supports only one flash message >>>>>>>>>> - the keywords are not short enough. >>>>>>>>>> >>>>>>>>>> So I think I'll change the implementation so that the >>>>>>>>>> template >>>>>>>>>> token >>>>>>>>>> is >>>>>>>>>> simply called 'flash', and it'll be a hash, like in >>>>>>>>>> Rails. >>>>>>>>>> I'll >>>>>>>>>> also >>>>>>>>>> change the registered method so that it's just flash() >>>>>>>>>> instead >>>>>>>>>> of >>>>>>>>>> get_flash() >>>>>>>>> >>>>>>>>> I agree. I'd like to behave just like Rails' flash >>>>>>>>> feature. >>>>>>>>> The >>>>>>>>> idea >>>>>>>>> is >>>>>>>>> pretty straight forward: >>>>>>>>> >>>>>>>>> "flash" is an accessor to a particular session hash table >>>>>>>>> whose >>>>>>>>> values >>>>>>>>> can >>>>>>>>> only be accessed once. Nothing more complicated than >>>>>>>>> that. >>>>>>>>> >>>>>>>>> So to conclude, IMO, flash should be a wrapper like the >>>>>>>>> following: >>>>>>>>> >>>>>>>>> sub flash { >>>>>>>>> my ($key, $value) = @_; >>>>>>>>> my $flash = session('_flash'); >>>>>>>>> >>>>>>>>> # write >>>>>>>>> if (@_ == 2) { >>>>>>>>> $flash->{$key} = $value; >>>>>>>>> session('_flash' => $flash); >>>>>>>>> } >>>>>>>>> >>>>>>>>> # read (+ delete) >>>>>>>>> else { >>>>>>>>> my $value = $flash->{$key}; >>>>>>>>> delete $flash->{$key} if defined $value; >>>>>>>>> session('_flash' => $flash); >>>>>>>>> } >>>>>>>>> >>>>>>>>> return $value; >>>>>>>>> } >>>>>>>>> >>>>>>>>> This is it, I think. This allows for the following code >>>>>>>>> in a >>>>>>>>> Dancer >>>>>>>>> app: >>>>>>>>> >>>>>>>>> >>>>>>>>> get '/' => sub { >>>>>>>>> flash welcome => "This is a welcome message, only >>>>>>>>> shown >>>>>>>>> once"; >>>>>>>>> } >>>>>>>>> >>>>>>>>> Then, as soon as the key 'welcome' is accesed via >>>>>>>>> flash('welcome'), >>>>>>>>> the >>>>>>>>> entry will be purged. >>>>>>>>> >>>>>>>>> This will be very helpful for authentication stuff in >>>>>>>>> before >>>>>>>>> filters, >>>>>>>>> error >>>>>>>>> messages, notifications, .... >>>>>>>>> >>>>>>>>> >>>>>>>>> Kudos to dams! >>>>>>>>> >>>>>>>>> (BTW I haven't read the code yet) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Alexis Sukrieh >>>>>>>>> _______________________________________________ >>>>>>>>> Dancer-users mailing list >>>>>>>>> Dancer-users@perldancer.org >>>>>>>>> >>>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Dancer-users mailing list >>>>>>>> Dancer-users@perldancer.org >>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Dancer-users mailing list >>>>>>> Dancer-users@perldancer.org >>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>> >>>>>>> >>>>>> _______________________________________________ >>>>>> Dancer-users mailing list >>>>>> Dancer-users@perldancer.org >>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>> >>>>> >>>> _______________________________________________ >>>> Dancer-users mailing list >>>> Dancer-users@perldancer.org >>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>> >> >>
_______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
cheers :) 0.311 checks that you have a session engine, otherwise it's the same code dams On 5 February 2011 23:05, Alberto Simoes <ambs@perl-hackers.net> wrote:
Thank you! 0.310 working like a charm :)
On 05/02/2011 12:18, damien krotkine wrote:
Hi again,
So I juste released Dancer-Plugin-FlashMessage-0.308
After discussing with sukria, sawyer, franck, I decided to go with the minimalistic implementation of the Flash Message Plugin :
Messages are always associated to a name, and stored in a hashref in the session
There is no persistence option, flash messages are deleted either when used in the template, or when retrieved using flash($key)
I think this solution makes it very easy to use the plugin. Also, it's similar to what exists in other web framework.
And so it leaves the room for a Dancer::Plugin::FlashMessage::Advanced, or however you want to call it,for anyone (Flavio ?) that needs a more advanced set of features.
I hope that's OK with you guys. Thanks again for the feedback :)
dams
On 30 January 2011 21:17, Flavio Poletti<polettix@gmail.com> wrote:
https://github.com/polettix/Dancer-Plugin-FlashMessage/commit/b36a20d2b8d355...
On Sun, Jan 30, 2011 at 8:28 PM, Brian E. Lozier<brian@massassi.com> wrote:
Where on github can I see comments?
On Sun, Jan 30, 2011 at 2:49 AM, damien krotkine<dkrotkine@gmail.com> wrote:
Hi !
I've commented on github. Let's swithc to that for the following discussion indeed. Brian, you are more than welcome to participate :)
dams
On 29 January 2011 14:59, Flavio Poletti<polettix@gmail.com> wrote:
Hi all, I don't know if this mailing list is actually the right place to continue this discussion about this module implementation, let us know if you're bored! I implemented a generic approach and you can find it
here: https://github.com/polettix/Dancer-Plugin-FlashMessage/tree/alternatives It is based on the configurability of three traits: * how messages are registered ("queue" option) * how additional parameters are handled ("arguments" option) * how messages are flushed away ("dequeue" option). There is a lot of new meat so I also added the full documentation with some examples. This implementation is kinda back-compatible, with two outstanding exceptions: * persistence has been dropped, even though its functionality has been absorbed by "dequeue" * the 'flash' method now only *sets* messages, and it never deletes them. IMHO, the case for deletion in the code is quite obscure, so I tried to move this functionality inside the new "flash_flush" registered keyword. It is anyway possible to revert to the old behaviour if it is considered better. For any other aspects, the current default values replicate the previous behaviour. If it makes sense to you we can add a plethora of tests. Thank you all for your patience, we'll probably better continue this discussion on GitHub if you agree. Flavio.
On Fri, Jan 28, 2011 at 7:05 PM, Flavio Poletti<polettix@gmail.com> wrote: > > > On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine<dkrotkine@gmail.com> > wrote: >> >> On 28 January 2011 18:40, Flavio Poletti<polettix@gmail.com> wrote: >>> >>> Hi, >>> >>> the implementation is sound, even though I'd prefer to be able >>> to >>> set >>> multiple messages per single channel. If I validate a form, for >>> example, I'd >>> like to flash a warning for each field I don't like, not just the >>> last >>> one. >> >> Hm, my brain being what it is on friday night, could you elaborate a >> bit more ? can you give an example of such channels ? >> >> I'm thinking you could use an array of string as flash.warnings, >> instead of a simple string, but I'm not sure I understand what >> you're >> saying > > > > Sorry, you got the "channel" idea. I'd like to use flash like this: > > flash warning => 'you made error X'; > > and then, possibly later, say: > > flash warning => 'you made error Y'; > > In the current implementation, only the last gets to the template. > I'm > suggesting to always push the incoming $value into an array. > Otherwise, with > the current implementation that lets you delete stuff, I'd be forced > to do > it like this: > > my $array = flash('warning') || []; > push @$array, 'you made error X'; > flash warning => $array; > > which is a bit ugly... > > >> >>> On the other hand, deleting flash messages from the controller does >>> seem to >>> be a corner case, which I'd handle with an ad-hoc function or >>> ignore >>> completely. >>> >>> I'll try to propose something on GitHub that lets the end user >>> decide >>> the >>> preferred semantics through a configuration; I would personally >>> like to >>> avoid having leftovers in the flash when my templates are not >>> bug-free >>> and I >>> definitely don't like the idea of remembering to call >>> flash->flush() >> >> I agree. After all, maybe it would make sense to have 3 behaviors : >> behaviour 1 : delete all the keys as soon as one is used (to be >> implemented ) >> behaviour 2 : delete only the key which is used ( current >> implementation >> ) >> behaviour 3 : never delete the keys ( currently persistence = 1 ) >> >> And maybe behaviour 1 should be the default, I don't know. As long >> as >> behaviour 2 is available, I'm happy (and brian too) >> >> maybe the behaviour could be chosen with the persistence config >> keyword ? It could be renamed to flush_behaviour >> >> flush_behaviour = key_level >> flush_behaviour = hash_level >> flush_behaviour = never >> >> What do you think ? OK I'm terrible at finding good names, so any >> help >> welcome :) > > > I'd leave "_behaviour", add an 'always' (your previous > implementation) > and > put something like: > > flush = 'key' > flush = 'all' > flush = 'always' > flush = 'never' > > Anyway, as long as it's documented and it's reasonably connected to > the > behaviour everything is fine :-) > > >> >>> (even >>> though it sounds so nice!) >> >> aha, yes, I like flash->flush :) Let's add it for fun, even if it's >> not necessary :) >> >> >>> >>> Cheers, >>> >>> Flavio. >>> >>> On Fri, Jan 28, 2011 at 6:33 PM, damien krotkine >>> <dkrotkine@gmail.com> >>> wrote: >>>> >>>> I agree with Brian, for a slightly different reason : >>>> >>>> Let's imagine this: >>>> >>>> you have a Template1, that does this :<% flash.keys() %>, to be >>>> able >>>> to know that they are errors. If that's the case, it doesn't >>>> display >>>> the errors, but display a link, saying that they are erros. This >>>> link >>>> does a new request against the server, serving Template2 >>>> >>>> Template2 does use flash.error, and effectively consumes the >>>> message. >>>> >>>> You don't want the flash hash structure to be emptied just after >>>> having displayed Template1, you want the messages to survive until >>>> Template2 (after all, you didn't consume them in Template1) >>>> >>>> If I understand correctly, In the original implementation, the >>>> simple >>>> fact of getting the keys of the hash would delete all the >>>> messages, so >>>> the next request, Template2 won't be able to retrieve<% >>>> flash.error >>>> %> >>>> >>>> In my new implementation, it would work. >>>> >>>> Granted, this is a bit of a corner case, however, I don't see any >>>> drawback. >>>> >>>> I could also add a flash->flush() method. >>>> >>>> >>>> On 28 January 2011 18:09, Brian E. Lozier<brian@massassi.com> >>>> wrote: >>>>> >>>>> I can't think of any real world use case where you'd only want >>>>> to >>>>> show >>>>> certain messages. That said, it still seems better to only >>>>> clear >>>>> the >>>>> messages you use, not all of them. >>>>> >>>>> On Fri, Jan 28, 2011 at 9:02 AM, Flavio Poletti >>>>> <polettix@gmail.com> >>>>> wrote: >>>>>> >>>>>> Hi, >>>>>> >>>>>> of course we can try to adapt to it (it seems a bit >>>>>> difficult >>>>>> although >>>>>> probably not impossible), but I fail to see the point. >>>>>> >>>>>> IMHO, if you get the chance to show the messages, you should >>>>>> show >>>>>> them >>>>>> all: >>>>>> showing warnings in one page and errors in the following one >>>>>> would >>>>>> puzzle >>>>>> the end user, wouldn't it? >>>>>> >>>>>> Unless there is some use case that point in the opposite >>>>>> direction >>>>>> I'd >>>>>> stick >>>>>> to the "wipe them all when you get the chance to show them" >>>>>> semantics. >>>>>> >>>>>> Cheers, >>>>>> >>>>>> Flavio. >>>>>> >>>>>> On Fri, Jan 28, 2011 at 5:32 PM, Brian E. Lozier >>>>>> <brian@massassi.com> >>>>>> wrote: >>>>>>> >>>>>>> It looks like a pretty good solution. The only thing I can >>>>>>> see >>>>>>> would >>>>>>> be that if you are in a template and check for flash.error but >>>>>>> not >>>>>>> flash.warning, it looks like it will clear both flash.error >>>>>>> and >>>>>>> flash.warning. It seems like it should only clear the one >>>>>>> you're >>>>>>> checking for. >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Fri, Jan 28, 2011 at 12:02 AM, Flavio Poletti >>>>>>> <polettix@gmail.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> I thought a bit about it and I think that the current >>>>>>>> implementation >>>>>>>> is a >>>>>>>> bit too limited (this seems also what brian thinks in the >>>>>>>> bug >>>>>>>> report http://rt.cpan.org/Public/Bug/Display.html?id=65009 - >>>>>>>> at >>>>>>>> least >>>>>>>> from >>>>>>>> what I understand). >>>>>>>> The problem is that the flash message might have to be >>>>>>>> displayed >>>>>>>> in >>>>>>>> the >>>>>>>> next >>>>>>>> request, which happens when redirections kick in. In this >>>>>>>> case, >>>>>>>> the >>>>>>>> flash >>>>>>>> message should be set, kept in the session and removed only >>>>>>>> when >>>>>>>> it >>>>>>>> is >>>>>>>> actually displayed; this should happen automatically, i.e. >>>>>>>> without >>>>>>>> playing >>>>>>>> with the $persistent variable - which is global - or with >>>>>>>> further >>>>>>>> overhead >>>>>>>> on the controller side. >>>>>>>> Following brian's feedback, I tried to re-code the plugin >>>>>>>> with >>>>>>>> the >>>>>>>> following >>>>>>>> criteria: >>>>>>>> * the "flash" method only adds messages (and handles many of >>>>>>>> them >>>>>>>> for >>>>>>>> every >>>>>>>> category - you might have more than one error in a page). I >>>>>>>> see >>>>>>>> no >>>>>>>> point >>>>>>>> in >>>>>>>> removing messages while inside the controller; >>>>>>>> * the message removal logic is shifted in the template via >>>>>>>> an >>>>>>>> anonymous >>>>>>>> sub. >>>>>>>> If the sub is called then the flash cache is cleared, >>>>>>>> otherwise >>>>>>>> it >>>>>>>> remains >>>>>>>> there. This should guarantee that the messages are not >>>>>>>> removed >>>>>>>> until >>>>>>>> they >>>>>>>> get the chance to be displayed. >>>>>>>> This is the core of the recoding - not tested but should >>>>>>>> suffice >>>>>>>> to >>>>>>>> give >>>>>>>> you >>>>>>>> the idea: >>>>>>>> register flash => sub { >>>>>>>> my ($key, $value) = @_; >>>>>>>> my $flash = session($session_hash_key); >>>>>>>> if (! $flash) { # initialise the container for flash >>>>>>>> messages >>>>>>>> $flash = {}; >>>>>>>> session($session_hash_key, $flash); >>>>>>>> } >>>>>>>> push @{$flash->{$key}}, $value; >>>>>>>> return; >>>>>>>> }; >>>>>>>> before_template sub { >>>>>>>> my $obj = shift; >>>>>>>> my $flashes; >>>>>>>> $obj->{$token_name} = sub { >>>>>>>> if (! $flashes) { # first call, get messages and clear >>>>>>>> $flashes = session($session_hash_key) || {}; >>>>>>>> session($session_hash_key, {}); >>>>>>>> } >>>>>>>> return $flashes; >>>>>>>> }; >>>>>>>> return; >>>>>>>> }; >>>>>>>> >>>>>>>> IMHO there is no need for an "exists", in the template you >>>>>>>> can >>>>>>>> do >>>>>>>> like >>>>>>>> this: >>>>>>>> <% IF flash.errors %>...<% END %> >>>>>>>> The important thing to remember is that even checking for >>>>>>>> the >>>>>>>> presence >>>>>>>> of >>>>>>>> something inside "flash" clears the messages, which is >>>>>>>> consistent >>>>>>>> with >>>>>>>> the >>>>>>>> semantics that "they get cleared as soon as they have a >>>>>>>> chance >>>>>>>> to be >>>>>>>> displayed". The $flashes variable guarantees that they >>>>>>>> survive >>>>>>>> at >>>>>>>> least >>>>>>>> until the end of the request, i.e. you are able to call >>>>>>>> "flash" >>>>>>>> multiple >>>>>>>> times while handling a request and always get all messages. >>>>>>>> If you think that this makes sense, I can propose a pull >>>>>>>> request >>>>>>>> on >>>>>>>> GitHub >>>>>>>> or you can make the changes directly. >>>>>>>> Cheers, >>>>>>>> Flavio. >>>>>>>> >>>>>>>> >>>>>>>> On Tue, Jan 11, 2011 at 5:48 PM, damien krotkine >>>>>>>> <dkrotkine@gmail.com> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> I've re-implemented it to be more Rails-like, as sukria >>>>>>>>> said. >>>>>>>>> >>>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>>> >>>>>>>>> and on CPAN, pending mirrors refresh. >>>>>>>>> >>>>>>>>> The funny part of the story ? the effective code is only 30 >>>>>>>>> lines >>>>>>>>> long. Talking about Perl and Dancer expressiveness... >>>>>>>>> >>>>>>>>> dams. >>>>>>>>> >>>>>>>>> On 11 January 2011 14:40, Alexis Sukrieh >>>>>>>>> <sukria@sukria.net> >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> Hi list! >>>>>>>>>> >>>>>>>>>> Le 11/01/2011 14:29, damien krotkine a écrit : >>>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> following previous thread, I've done a first >>>>>>>>>>> implementation >>>>>>>>>>> of >>>>>>>>>>> Dancer::Plugin::FlashMessage : >>>>>>>>>>> >>>>>>>>>>> https://github.com/dams/Dancer-Plugin-FlashMessage >>>>>>>>>> >>>>>>>>>> Great! Thanks a lot for your time dams, the myth is still >>>>>>>>>> alived! >>>>>>>>>> (Dancer's >>>>>>>>>> community)++ >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> Some parts need to be improved, for instance : >>>>>>>>>>> >>>>>>>>>>> - it supports only one flash message >>>>>>>>>>> - the keywords are not short enough. >>>>>>>>>>> >>>>>>>>>>> So I think I'll change the implementation so that the >>>>>>>>>>> template >>>>>>>>>>> token >>>>>>>>>>> is >>>>>>>>>>> simply called 'flash', and it'll be a hash, like in >>>>>>>>>>> Rails. >>>>>>>>>>> I'll >>>>>>>>>>> also >>>>>>>>>>> change the registered method so that it's just flash() >>>>>>>>>>> instead >>>>>>>>>>> of >>>>>>>>>>> get_flash() >>>>>>>>>> >>>>>>>>>> I agree. I'd like to behave just like Rails' flash >>>>>>>>>> feature. >>>>>>>>>> The >>>>>>>>>> idea >>>>>>>>>> is >>>>>>>>>> pretty straight forward: >>>>>>>>>> >>>>>>>>>> "flash" is an accessor to a particular session hash table >>>>>>>>>> whose >>>>>>>>>> values >>>>>>>>>> can >>>>>>>>>> only be accessed once. Nothing more complicated than >>>>>>>>>> that. >>>>>>>>>> >>>>>>>>>> So to conclude, IMO, flash should be a wrapper like the >>>>>>>>>> following: >>>>>>>>>> >>>>>>>>>> sub flash { >>>>>>>>>> my ($key, $value) = @_; >>>>>>>>>> my $flash = session('_flash'); >>>>>>>>>> >>>>>>>>>> # write >>>>>>>>>> if (@_ == 2) { >>>>>>>>>> $flash->{$key} = $value; >>>>>>>>>> session('_flash' => $flash); >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> # read (+ delete) >>>>>>>>>> else { >>>>>>>>>> my $value = $flash->{$key}; >>>>>>>>>> delete $flash->{$key} if defined $value; >>>>>>>>>> session('_flash' => $flash); >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> return $value; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> This is it, I think. This allows for the following code >>>>>>>>>> in a >>>>>>>>>> Dancer >>>>>>>>>> app: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> get '/' => sub { >>>>>>>>>> flash welcome => "This is a welcome message, only >>>>>>>>>> shown >>>>>>>>>> once"; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> Then, as soon as the key 'welcome' is accesed via >>>>>>>>>> flash('welcome'), >>>>>>>>>> the >>>>>>>>>> entry will be purged. >>>>>>>>>> >>>>>>>>>> This will be very helpful for authentication stuff in >>>>>>>>>> before >>>>>>>>>> filters, >>>>>>>>>> error >>>>>>>>>> messages, notifications, .... >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Kudos to dams! >>>>>>>>>> >>>>>>>>>> (BTW I haven't read the code yet) >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Alexis Sukrieh >>>>>>>>>> _______________________________________________ >>>>>>>>>> Dancer-users mailing list >>>>>>>>>> Dancer-users@perldancer.org >>>>>>>>>> >>>>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> Dancer-users mailing list >>>>>>>>> Dancer-users@perldancer.org >>>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> Dancer-users mailing list >>>>>>>> Dancer-users@perldancer.org >>>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>>>> >>>>>>>> >>>>>>> _______________________________________________ >>>>>>> Dancer-users mailing list >>>>>>> Dancer-users@perldancer.org >>>>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> Dancer-users mailing list >>>>> Dancer-users@perldancer.org >>>>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users >>>>> >>> >>> >
_______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
_______________________________________________ Dancer-users mailing list Dancer-users@perldancer.org http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
participants (2)
-
Alberto Simoes -
damien krotkine