[Dancer-users] Flash Message
Brian E. Lozier
brian at massassi.com
Sun Jan 30 20:28:00 CET 2011
Where on github can I see comments?
On Sun, Jan 30, 2011 at 2:49 AM, damien krotkine <dkrotkine at 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 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