[Dancer-users] Flash Message
damien krotkine
dkrotkine at gmail.com
Sun Jan 30 11:49:43 CET 2011
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 at 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 at gmail.com> wrote:
>>
>>
>> On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine <dkrotkine at gmail.com>
>> wrote:
>>>
>>> On 28 January 2011 18:40, Flavio Poletti <polettix at 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 at 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 at 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 at 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 at 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 at 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 at 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 at 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 at perldancer.org
>>> >> >>> >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>>> >> >>> >> >
>>> >> >>> >> _______________________________________________
>>> >> >>> >> Dancer-users mailing list
>>> >> >>> >> Dancer-users at perldancer.org
>>> >> >>> >> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>>> >> >>> >
>>> >> >>> >
>>> >> >>> > _______________________________________________
>>> >> >>> > Dancer-users mailing list
>>> >> >>> > Dancer-users at perldancer.org
>>> >> >>> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>>> >> >>> >
>>> >> >>> >
>>> >> >>> _______________________________________________
>>> >> >>> Dancer-users mailing list
>>> >> >>> Dancer-users at perldancer.org
>>> >> >>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>>> >> >>
>>> >> >>
>>> >> > _______________________________________________
>>> >> > Dancer-users mailing list
>>> >> > Dancer-users at perldancer.org
>>> >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
>>> >> >
>>> >
>>> >
>>
>
>
More information about the Dancer-users
mailing list