Fixed flat URLs for app hooks. #1347

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

mitar commented Jul 15, 2012

Even if flat URLs will be deprecated in 2.4, they should still work in 2.3. This little change fixes them to again work correctly. As changes to support CMS_FLAT_URLS are really small, I would argue that they could be kept in also in future versions.

This pull request passes (merged c50c492 into dc00224).

Collaborator

ojii commented Jul 15, 2012

exactly code like this is why i want to deprecate it. CMS_FLAT_URLS has (in my opinion) little actual benefit, but makes the code uselessly more complicated and a lot harder to test.

Contributor

mitar commented Jul 15, 2012

But code is already in place and it works. And it has quite nice use case: complex menu structure but keeping flat URL structure.

Collaborator

ojii commented Jul 15, 2012

It is in place, but as you might have noticed, our test suite doesn't agree with your (correct) claim that this is broken. CMS_FLAT_URLS basically means we have to test everything twice, and for what? Just so the URLs on a handful of sites are shorter. If your menu is small enough to work with flat-urls, it's small enough to either manually use url-overwrite or to write a pre-save hook that sets it automatically.

Contributor

mitar commented Jul 15, 2012

No, menu is large, but URL can still not overlap.

Contributor

mitar commented Jul 15, 2012

Maybe the solution would be to make path attribute of the page dynamic, so that it can be just a slug or whole path, or even dynamically computed value, depending on what is configured.

Member

digi604 commented Aug 13, 2012

Needs tests

Member

digi604 commented Mar 28, 2013

closed pull request for stallement in spring cleaning

digi604 closed this Mar 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment