On Wed, Sep 17, 2008 at 06:26:07PM -0400, BUCHMULLER Norbert via RT wrote:
Show quoted text> Queue: Catalyst-Plugin-RequireSSL
> Ticket <URL:
http://rt.cpan.org/Ticket/Display.html?id=38996 >
>
> On Wed, 10 Sep 2008 13:12:10 -0400 "Matt S Trout via RT"
> <bug-Catalyst-Plugin-RequireSSL@rt.cpan.org> wrote:
>
> > Your patch doesn't include captures and won't work for complex regexp
> > actions even if it did. However, if you were to supply a patch -with
> > tests- that simply did
> > $c->uri_for('/'.$c->req->path,$c->req->query_parameters) I think -that-
> > would work in all cases and work around your problem.
>
> You're right, I did not take the captures into account. OTOH
> $c->req->path does not necessarily correspond to the path argument of
> $c->uri_for() (that's exactly the reason why we have $c->uri_for(),
> isn't it?:-). What do you think of this solution:
>
> $c->uri_for(
> $c->action, $c->req->captures,
> @{$c->req->arguments},
> $c->req->params
> );
>
> According the documentation of Catalyst::uri_for(), if $path is an action
> object and the first argument after it is an array ref, it is considered
> as the list of captures, and is passed to
> $c->dispatcher->uri_for_action() along with the action object. So, if the
> dispatcher can reconstruct the path from the action object and the
> captures, it works. Now the question is how much can we depend on it (ie.
> that uri_for_action() works well).
My note about "complex regexp actions" was specifically because
uri_for_action can't work universally.
How about reusing $c->req->uri to get the query string?
I honestly don't think uri_for_action is safe enough for this.
Hmm. You still want to go via uri_for of course.
What about
my $uri = $c->uri_for('/'.$c->req->path);
$uri->query($c->req->uri->query);
?
That seems to fix the query param problem you have and still not rely
on uri_for_action. If $c->req->path isn't the right thing then that's a
separate problem and probably belongs on catalyst-dev@lists.scsys.co.uk
Show quoted text> For Catalyst::DispatchType::Regex I just filed a bug report (rt.cpan.org
> #39369), as its uri_for_action() is totally broken. But even if my patch
> is accepted, the implementation is still admittedly weak (eg. it cannot
> cope with nested parentheses according to the documentation), and it's no
> surprise, as the task is inherently very tough (to reconstruct the
> original string that was matched from the regex and captures).
Yes, I know. I wrote the original code for that (and all of Chained) and
basically implemented uri_for_action for regexp to "mostly work" - I didn't
try and cover many edge cases because once you go into edge cases it gets
harder and harder, and was frankly pleasantly surprised I managed to make
it work at all :) - but please note that the correct place to discuss
changes to things is the catalyst-dev@ list, most of the relevant people
aren't on rt so you'll get long ping times like we've had here.
Once we get capture matching for Chained so you can do something like
CaptureArgs(qr/\d+/)
or similar, then Regexp basically becomes obsolete, which was always my
intention, and then it becomes a much simpler thing.
Show quoted text> > A better solution for -you- IMO would be if you captured the language
> > via a
> >
> > sub lang :Chained('/') :CaptureArgs(1) {
> >
> > action - then this would all work already; mangling $c->req->path is
> > dangerous and highly inadvisable.
>
> Thanks for pointing it out. In fact I perused the manual of
> Catalyst::DispatchType::Chained twice before we settled with the
> prepare_path() + uri_for() hack, as I felt that it's something very
> close, but somehow it did not lend itself as the solution we looked for.
> It did not get through for me that there need not be one predetermined
> endpoint for each chain. I'll convert our app to use Chained soon.
Well, there is a predetermined end point, but you can chain lots of
things off one start point :)
Anyway, that conversation should continue on the mailing list if at all.
--
Matt S Trout Need help with your Catalyst or DBIx::Class project?
Technical Director
http://www.shadowcat.co.uk/catalyst/
Shadowcat Systems Ltd. Want a managed development or deployment platform?
http://chainsawblues.vox.com/ http://www.shadowcat.co.uk/servers/
