[Catalyst] Re: PathPart("") v PathPart compared with Path("") v Path

[J. Shirley]

On Sun, Mar 1, 2009 at 6:52 AM, Aristotle Pagaltzis <pagaltzis at gmx.de>wrote:

> * Tomas Doran <bobtfish at bobtfish.net> [2009-02-28 23:40]:
> > We should either allow "PathPart" to work like "Path", or, if
> > there is good reason why it needs to be "PathPart('')" (which I
> > can't think of, given we have : Local), then "Path" should be
> > deprecated to maintain consistency.
>
> It seems to me that `Path` and `PathPart` declare fundamentally
> different things, namely the former is a dispatch type while the
> latter is just a configuration setting for `Chained`.
>
> It doesn=92t make any sense to me that a parameter-less `PathPart`
> is accepted at all; it should be an error, IMO.
>
> Also IMO, bare `Path` should mean the same as repeating the
> action sub name, eg. for `sub foo` it would mean the same as
> `Path('foo')`. This is just what `Chained` without a `PathPart`
> does. But of course that ship has long sailed and we=92d have to
> break backcompat to change it now, so it=92s not feasible.
>
> However, is there any reason why the `PathPart` has to be spelled
> out so verbosely for chained actions? In `Path` dispatch, you
> declare the local path right there as an attribute parameter; why
> not make `Chained` like that as well? Ie. the `Chained` attribute
> could simply take an optional second parameter, so that this=85
>
>    sub view : Chained('item') PathPath('') { ... }
>
> =85 could instead just be this:
>
>    sub view : Chained('item', '') { ... }
>
> And if you write this=85
>
>    sub view : Chained('item') { ... }
>
> =85 then you get the same as now, ie implicitly this:
>
>    sub view : Chained('item', 'view') { ... }
>
> This change would be completely backwards compatible, would cost
> nothing in terms of expressivity, and would make it much easier
> to write and a lot less noisy to read chained actions.
>
> Regards,
> --
> Aristotle Pagaltzis // <http://plasmasturm.org/>
>

I can appreciate brevity, but I don't like that syntax at all.  If
Chained(...) starts accepting additional arguments, I'd prefer it to bind to
multiple upstream points.  Binding the internal chain path to the external
PathPart seems like a coupling that would be too restrictive in the future.
I really like the decoupling.

Additionally, in the case of overriding PathPart via config, I wouldn't want
to have to do:

__PACKAGE__->config(
   actions =3D> {
       view =3D> { Chained =3D> [ 'item', 'view' ] }
   }
);

Because that doesn't make any sense to fresh eyes looking at it.

Also, I may not know what 'item' is if it is inheriting from a base class,
and someone just wants to modify the path.  There isn't an easy way to do
that unless PathPart can also be used in conjunction with Chained('.',
'pathpart') and have precedence.

Thanks,
-J
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.scsys.co.uk/pipermail/catalyst/attachments/20090301/ca3c2=
ded/attachment.htm