plack/Plack — pull request #364 by avar (2013-01-17 18:07:12+00:00)
Title: Plack::Middleware::Static: add ability to pass $env to callbacks

Change the callback interface for static middleware to pass $env to
the callbacks in addition to setting $_ and passing $_ as the first
argument.

Some middleware might want to look at other things in $env, or consult
some object we previously stuck in $env instead of just looking at
$env->{PATH_INFO}. This adds the ability to easily do that.

Chat about an earlier version of this patch on #plack which instead of
just adding $env to the arguments passed to the callaback. See
https://github.com/plack/Plack/pull/363.

17:29:43 < miyagawa> avar: quite honestly i don't like the pattern you often come up with, that changes callback signature with other options
17:33:20 < avar> miyagawa: How would you add that to the API while preserving compatibility? Add another option with a sub that has a different signature?
17:36:55 < miyagawa> avar: yes, or in this case pass $env as a second arg
17:37:08 < miyagawa> OR, you write a completely new middleware as a subclass
17:37:35 < miyagawa> i don't know what you actually want to achieve
17:37:39 < miyagawa> "some middleware might" isn't a use case
17:38:35 < avar> If you think breaking "my $path = pop" is fine that's fine by me. I was going for something that wouldn't ever break existing stuff without explicitly declaring what you wanted.
17:40:45 < miyagawa> this is not POE nobody would ever use pop
17:41:08 < miyagawa> but then you can also use other option like you said
17:41:48 < miyagawa> and again i don't see a use case
17:42:36 < avar> I'm setting up a Plack::Request object early in another middleware that everything else has access to, also an object in $env you can log against.
17:42:51 < avar> Not having to set that up again + being able to log with that object you pass around is handy
17:48:30 < miyagawa> nobody would use $path = shift either because the interface is $_
17:48:50 < miyagawa> but don't take that as a suggestion to completely change that
17:49:22 < avar> Why is it passing $_ in addition to setting it in the first place?
17:50:05 < miyagawa> i guess no app will break if you don't pass it
17:50:31 < avar> well some might be doing: my ($path) = @_;
17:50:51 < doy> which still wouldn't break
17:51:28 < miyagawa> yeah.
17:51:41 < miyagawa> the documentation says nothing about the argument anyway
17:51:42 < miyagawa> BUT
17:51:56 < miyagawa> i think your use case is pretty odd
17:52:04 < doy> yeah, the documentation explicitly says "The callback should operate on $_ and return a true or false value."
17:52:11 < miyagawa> and that it has to be handled separately than adding a new argument to "path:"
17:52:41 < miyagawa> right
17:53:04 < miyagawa> I think the easiest compromise would be to pass $path, $env to the callback :)
17:54:41 < avar> Sure, works for me. For what it's worth I don't usually write APIs with odd semantics like this, I was just going for something that didn't break the documented or implicit API