Skip to content

fix for template_dir is array without key of zero defined #14

Merged
glensc merged 6 commits intosmarty-gettext:masterfrom
timmit-nl:master
Oct 30, 2016
Merged

fix for template_dir is array without key of zero defined #14
glensc merged 6 commits intosmarty-gettext:masterfrom
timmit-nl:master

Conversation

@timmit-nl
Copy link
Copy Markdown
Contributor

Hi There,

I have written a fix for if the array contains diffrent keys then 0,1,2,3,4....
Before the locale function only pickes $template_dir[0]. Now the function will pick the first directory in order of the array if the template_dir is a directory including the params['path'].

Also have a fallback if the path is not defined, for example with stack="pop" (then the path is not necessary)

Please add it upstream so we can use the packagist packages.

Thanks,

Tim

Tim Schoondergang added 2 commits September 16, 2016 16:04
@glensc
Copy link
Copy Markdown
Member

glensc commented Sep 16, 2016

care to drop in some testcase?

@timmit-nl
Copy link
Copy Markdown
Contributor Author

Hi Elan,

Ehmmm good question, I never have done one propperly. But I will give it a try.

@timmit-nl
Copy link
Copy Markdown
Contributor Author

Hi Elan,

I have tried to update the testcases. Can you review it and if correct intergrate the pull request?

Thanks,

Tim

@glensc
Copy link
Copy Markdown
Member

glensc commented Sep 20, 2016

looks like your last commit broke tests:
https://travis-ci.org/smarty-gettext/smarty-gettext/builds/161287106

i haven't looked closely in what you are doing there, but template_dir being string (not array) support must be retained.

also to get first item from array regardless what it is, you can use current() php function

@timmit-nl
Copy link
Copy Markdown
Contributor Author

timmit-nl commented Sep 27, 2016

Hi Elan,

I have restored the
$smarty->template_dir = $template_dir;
in the test case, so you can test the template_dir as string.

The current() is not what I am looking for, because I want the first existing path, not the first template_dir.

Hopefully you can see what I am doing in the TestCase.php. The test will now run OK

@timmit-nl
Copy link
Copy Markdown
Contributor Author

Hi Elan,

Sorry to bother you again. But did you have time to review this merge request?

Thanks,

Tim

@glensc
Copy link
Copy Markdown
Member

glensc commented Oct 13, 2016

sorry, not yet

@glensc glensc merged commit 9c8e82d into smarty-gettext:master Oct 30, 2016
@timmit-nl
Copy link
Copy Markdown
Contributor Author

Thanks! Can you also make a new release?

@glensc
Copy link
Copy Markdown
Member

glensc commented Nov 3, 2016

yes. planned it 4 days ago, but got lost in sea of information again :D

i got distracted when waiting for travis job to succeed.

ayway. released now!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants