[RFC|Merged] Template locator and path finder

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
User avatar
Arty
Registered User
Posts: 970
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

[RFC|Merged] Template locator and path finder

Post by Arty » Fri Mar 23, 2012 6:26 pm

Introduction

I think template locator and path finder require some changes.

Sample situation: 2 templates, 3 extensions

Templates: progreen, which inherits from prosilver.
Extensions: ext1, ext2 and ext3. ext1 has prosilver templates, ext2 has prosilver and progreen templates, ext3 has "all" templates (see events)

Currently locator stores paths to templates in one dimensional array. With template inheritance and extensions, order of search looks like this:
1. styles/progreen/template/
2. styles/prosilver/template/
3. ext/ext1/styles/prosilver/template
4. ext/ext2/styles/progreen/template
5. ext/ext2/styles/prosilver/template
6. ext/ext3/styles/all/template <-- currently missing, but I think it should be added

That works great when including templates.

However, it doesn't work for events because events must include templates from all extensions.

In current version of events PR, template filter is responsible for locating templates, it uses finder for it without accounting for template inheritance. For example above, it means templates will be searched for in locations #1, #4 and #6. So it isn't searching half of directories that it should search in.

If locator is used to locate template instead of finder, it doesn't distinguish between extension and style, so sample template located within several styles in same extension, like #4 and #5, both will be included.

If all of those styles in example have template for event, correct way would be to include only templates from #1, #3, #4 and #6. #2 should not be included because #1 overrides it with template inheritance. #5 should not be included because #4 overrides it with template inheritance.


Suggestion

I suggest to solve it by changing from one dimensional list of paths to 2 dimensions: extension and paths:
['style'][0] = styles/progreen/template/
['style'][1] = styles/prosilver/template/
['ext1'][0] ext/ext1/styles/prosilver/template
['ext2'][0] = ext/ext2/styles/progreen/template
['ext2'][1] = ext/ext2/styles/prosilver/template
['ext3'][0] = ext/ext3/styles/all/template

When searching for templates for events, locator should look for first entry in each top level array. As soon as entry in second level array is found, locator should go to next top level array. Example: searching in ['style'][0] => if found, go to ['ext1'][0]. if not found, check ['style'][1]

This way only one template per extension will be included in event.

Also with this change main template path is gone. It wasn't functioning properly anyway.

Patch

Ticket: http://tracker.phpbb.com/browse/PHPBB3-10735
Patch: https://github.com/phpbb/phpbb3/pull/677
Formerly known as CyberAlien.

Free phpBB styles | Premium responsive XenForo styles

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: Template locator and path finder

Post by MichaelC » Fri Mar 23, 2012 6:29 pm

This looks very good to me.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
brunoais
Registered User
Posts: 958
Joined: Fri Dec 18, 2009 3:55 pm

Re: Template locator and path finder

Post by brunoais » Sat Mar 24, 2012 10:05 am

Arty wrote: I suggest to solve it by changing from one dimensional list of paths to 2 dimensions: extension and paths:
[0] = styles/
[0][0] = styles/progreen/template/
[0][1] = styles/prosilver/template/
[1] = ext/ext1/styles/
[1][0] ext/ext1/styles/prosilver/template
[2] = ext/ext2/styles/
[2][0] = ext/ext2/styles/progreen/template
[2][1] = ext/ext2/styles/prosilver/template
[3] = ext/ext3/styles/
[3][0] = ext/ext3/styles/all/template

When searching for templates for events, locator should look for first entry in each top level array. As soon as entry in second level array is found, locator should go to next top level array. Example: searching in [0][0] => if found, go to [1][0]. if not found, check [0][1]

This way only one template per extension will be included in event. I'm not sure how much code changes it would require, but without it I don't see how template events could be implemented.
Hum... shouldn't it be something different?

Code: Select all

['phpbb_core'][0] = styles/ 
['phpbb_core']['progreen'] = styles/progreen/template/
['phpbb_core']['prosilver'] = styles/prosilver/template/
['ext1'][0] = ext/ext1/styles/
['ext1']['prosilver'] ext/ext1/styles/prosilver/template
['ext2'][0] = ext/ext2/styles/
['ext2']['progreen'] = ext/ext2/styles/progreen/template
['ext2']['prosilver'] = ext/ext2/styles/prosilver/template
['ext3'][0] = ext/ext3/styles/
['ext3']['all'] = ext/ext3/styles/all/template
?

If I understood correctly, what you have written is not even a valid php array.

User avatar
Arty
Registered User
Posts: 970
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: Template locator and path finder

Post by Arty » Sat Mar 24, 2012 10:11 am

That would work too, except for [whatever][0] - that item was for illustrative purposes only, it doesn't exist in array.
Formerly known as CyberAlien.

Free phpBB styles | Premium responsive XenForo styles

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: Template locator and path finder

Post by MichaelC » Sat Mar 24, 2012 2:44 pm

Does this need an RFC or as part of events/extensions?
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
Arty
Registered User
Posts: 970
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: Template locator and path finder

Post by Arty » Sat Mar 24, 2012 2:55 pm

I don't know. Without it or another similar solution events won't work properly, so I guess it should be a part of events.
Formerly known as CyberAlien.

Free phpBB styles | Premium responsive XenForo styles

User avatar
Arty
Registered User
Posts: 970
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Template locator and path finder

Post by Arty » Thu Mar 29, 2012 10:38 pm

I've changed topic from discussion to RFC and moved it to 3.2 RFCs forum.

After testing it, I've implemented these changes on my forum, using associative arrays that brunoais suggested.

Implementation of "all" directory requires changing template initialization function to add "all" as third fallback template path. Such change already exists in PR 625 because it can handle unlimited styles tree. Therefore, implementation of this RFC requires merge of PR #625
Formerly known as CyberAlien.

Free phpBB styles | Premium responsive XenForo styles

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: [RFC] Template locator and path finder

Post by MichaelC » Thu Mar 29, 2012 11:48 pm

This is required for 3.1 events so it shouldn't it be in the 3.1 RFC's & Patches Forum?
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

User avatar
Arty
Registered User
Posts: 970
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Template locator and path finder

Post by Arty » Sat Mar 31, 2012 6:23 pm

Formerly known as CyberAlien.

Free phpBB styles | Premium responsive XenForo styles

User avatar
igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Merged] Template locator and path finder

Post by igorw » Sat Mar 31, 2012 8:54 pm

Merged.

Post Reply