Brain-WP / Cortex

Routing system for WordPress
MIT License
348 stars 20 forks source link

Defaults and host information in route groups #2

Closed Solinx closed 8 years ago

Solinx commented 10 years ago

Hello Giuseppe,

As told in the prior issue report there were other minor issues we've uncovered. Both other issues concern the grouping functionality.

  1. Setting a host in a route group does not appear to have any effect.
  2. Setting defaults for parameters that do not appear in the url do not get applied.

Because there are obvious workarounds we have not taken the time to look into the reason for these issues. It would be nice to have a fix for this, but it's not like we can't do without this functionality.

Cheers, Wouter van Dam

gmazzap commented 10 years ago

@Solinx

For first point:

Setting a host in a route group does not appear to have any effect

From Cortex docs (see 2nd point in the list)

Properties set in the group are merged with route properties when the route matched: it makes no sense put in the group properties that act on route matching process (like requirements, defaults…) because when the group is merged matching already happen.

Now 'host' is a property that act on route matching process and as stated in sentence above, can't be set via groups, sorry.

Second point:

Setting defaults for parameters that do not appear in the url do not get applied

There are some unit tests that test this feature and passes. I have also tested some random arguments in defaults and they are applied as expected.

Can you elaborate better this issue and possibly let me see your code?

gmazzap commented 10 years ago

@Solinx Again regarding 'host' param (and other params that can't be set via groups) I understand that may be annoing having to specify the host for every route, but to overcome this problem you can make use of Cortex API in a custom class, that will semplify the routes building task according to your needs.

See this Gist for a rough, but working example.

Solinx commented 10 years ago

Hello Giuseppe,

Thanks for the reminder about why it won't work. And we have indeed written a class to avoid having to repeat setting the host as well as the group.

Unfortunately I do not currently have the time to elaborate on the defaults issue. I will try to provide more details tomorrow or friday.

Regards, Wouter

Solinx commented 10 years ago

Hello Guiseppe,

Sorry for not getting back to you sooner. I have not forgotten about this issue, but I'm sorry to say I do not know when I'll be able to properly look into it again. The past week has been very busy and next week will be more of the same I'm afraid. My availability in the days thereafter unfortunately remains uncertain for now. I'll do my best to pick this up, but I can make no promises.

Regards, Wouter

gmazzap commented 10 years ago

Don't worry @Solinx comes when you can.

gmazzap commented 8 years ago

Closing to keep issue tracker clean...