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
>> >
>
>