[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
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

[RFC|Merged] Template locator and path finder

Post by Arty »

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

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 »

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: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: Template locator and path finder

Post by brunoais »

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
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: Template locator and path finder

Post by Arty »

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

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 »

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
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: Template locator and path finder

Post by Arty »

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.

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Template locator and path finder

Post by Arty »

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

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 »

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.


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

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

Post by igorw »

Merged.

Post Reply