[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [cgiapp] Re: RFC: CAP::Plugin::PathInfo


On Date: Mon, 11 Jul 2005 20:57:11 +0000 (UTC), Mark Stosberg <suppressed> wrote:
> On 2005-07-11, David Emery <suppressed> wrote:
> Thanks for the contribution. Here's some feedback on the POD. 

Thanks for the feedback, Mark. 

> >  use CGI::Application::Plugin::PathInfo
>  And doesn't the user need to import
> path_info_to_query() here? The docs below says it's next exported. 

No, nothing is ex/imported. The plugin works invisibly, adding params
to the CGI Query at the pre_run stage. Does something in the docs seem
to say that a method is imported? - I don't see it.

> >    # set param names for the elements of path info
> >    $self->param('path_info_vars'=>
> > 		{
> > 		 'showdetail'=>['rm','category_id','item_id','color'],
> > 		 'showform'=>['rm','item_id'],
> > 		 'showlist'=>0, #ignore any path info (besides runmode)
> > 		}
> > 	       );
> >    }
> It would make more sense to me to have the path_info_vars be run mode
> names, not subroutine names, especially since run modes can point to
> anonymous subs. 

Oops. Actually it is the runmode names and not the subroutine names
that are used. It uses get_current_run_mode() to fetch the runmode name
in the callback. The example has it backwards. The lesson here: Don't
write your POD at the crack house, even if they do provide free
wireless broadband...

> Also, it would be nice if the 'showlist => 0' could simply be excluded
> above, if no special action is needed. 

It can currently be excluded, in which case the params will be set to
generic names (path_info_1, path_info_2) by default. I don't see any
harm in having those default params set, so I'm not sure there's a
practical use case for the 'myrunmode'=>0. I put it in just in case
there's some reason I haven't thought of to want them unset for a
particular run mode. I'm not especially attached to this behavior,
though. Personally, I'll likely set names wherever I want to use the
path info. 

Do you think it would be better to default to not setting the params
unless their names are set in setup()?

> This example could be clearer if it there was  CGI script present in the
> URL, instead of an implied index.cgi. 

Yes indeed, that example needs work.

> Finally, it could help the module name to include detail about what it
> does with PATH_INFO, since there is already some PATH_INFO processing
> built into CGI::App. 
> 
> Maybe ::PathInfo2Param or PathInfoToQuery, to match your method name.

Also, to avoid confusion with CGI::PathInfo, which does something a
bit different. CGI::Application::Plugin::PathInfoToQuery seems OK. 
Unless a better idea pops up, I'll probably go with that. Thanks.

> Overall, I like the idea. This style of URL is more user-friendly, and
> has traditionally been more work for the developer to parse. 

Groovy. I'll clean up the docs and write some tests (yeah, I know...) 
as I find time over the next couple of days.

Dave

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
              http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed


Mail converted by mhonarc 2.6.15
This archive provided courtesy of JSW4.NET, Internet Hosting Services for Small Business.