Vheissu / Ci-Smarty

Smarty templating engine integration for Codeigniter 3.0+ with inbuilt theming and asset management.
http://ilikekillnerds.com
179 stars 43 forks source link

Not working at all #5

Closed nakp closed 13 years ago

nakp commented 13 years ago

Hi, I've installed and its supposed to work out of the box right? well I've tried with the test controller and loads nothing but a blank page :/ not even debug info about what could be wrong, It seems there's an issue with MY_Parser class as it's not loading even normal views or normal templates, I remove MY_Parser and everthing works fine.. I'll check the code :)

Vheissu commented 13 years ago

This could be related to the bug with cached vars due to a change in Codeigniter Reactor and the cached vars variable being changed to protected along with a few other variables.

nakp commented 13 years ago

so is this a CodeIgniter bug? I still can't get it working :/

Vheissu commented 13 years ago

Not even with the latest code I pushed?

nakp commented 13 years ago

nope, maybe you didnt push MY_Parser fixed? it's the same it was 2 days ago :/ and still not working

nakp commented 13 years ago

Ok I've noticed many things in your code, I would request push but I'm new with git ñ_ñ

first, shouldn't you be calling the instance by reference? both classes aren't and I remember it's supposed to be $CI =& get_instance();

and this directory doesn't exist (taken from config/smarty.php) I dunno what's for

$config['config_directory']     = APPPATH."third_party/Smarty/configs";

well... while I was posting this I changed error_reporting to see what was going on, my cache and compiled folders were 775 instead of 777 :) (isn't it supposed to work with 771 on folders and 664 on files? )

anyway ;) thank you for this great lib!

Vheissu commented 13 years ago

That's fine :) I'm not entirely that great or seasoned in Git myself, I'm
still sort of new to it. As for the & get instance, that is a PHP4 thing.
Because Codeigniter dropped PHP4 support sometime ago, PHP 5 actually by
default assigns by reference so you don't actually need the ampersand any
more. As for the configs directory, I'll check that out, could be something
someone else added in for a reason. Thanks for your feedback, I'll push a
note in the instructions to check permissions on your cache and compiled
folders.

On , nakp
reply@reply.github.com
wrote:

Ok I've noticed many things in your code, I would request push but I'm
new with git _

first, shouldn't you be calling the instance by reference? both classes
aren't and I remember it's supposed to be $CI =& get_instance();

and this directory doesn't exist (taken from config/smarty.php) I dunno
what's for


$config['config_directory'] = APPPATH."third_party/Smarty/configs";

well... while I was posting this I changed error_reporting to see what
was going on, my cache and compiled folders were 775 instead of 777 :)
(isn't it supposed to work with 771 on folders and 664 on files? )

anyway ;) thank you for this great lib!

Reply to this email directly or view it on GitHub:

https://github.com/Vheissu/Ci-Smarty/issues/5#comment_1144660

nakp commented 13 years ago

Ok, so maybe the cached vars never were buggy right?? btw I found something I would do different :) on MY_Parser class, checking if $template is empty, actually I would use empty function instead of comparing to '' (empty string) because you could pass 0, false, null or '' and in the first 3 cases it would continue.

Thank you for such wonderful library and for opening my eyes :) this is a great example of how a third_party library are used, I was thinking about several libraries (actually only 2 and their alternatives) and about integrating them to my project (into the parser) and now I know how it's done ;)

Vheissu commented 13 years ago

Awesome dude, glad you found it useful. Will take your suggestions about the empty checking on board and implement something tonight. If you're interested, I've also got a library called Plenty Parser which is a driver based template library that has support for Smarty and Twig. https://github.com/Vheissu/Plenty-Parser

nakp commented 13 years ago

Ok here I am again, with more suggestions haha

I've found you can use _cached_vars and there's no bug with it, I just found I needed them and you deleted 'em :( So here are the changes I suggest, in MY_Parser library

Declare the default $data as an array, as it is on view loader

<?php
public function parse($template, $data = array(), $return = FALSE, $use_theme = FALSE)

Secondly, again, merge _cached_vars as supposed :)

<?php
// Merge in any cached variables with our supplied variables
$data = array_merge($data, $this->CI->load->_ci_cached_vars);

as $data is declared as an array() there's no need to check if it's an array again

and lastly but not less important the way you check if template is empty :) (so nobody can pass false, null, 0 or sumshit else)

<?php
// Make sure we have a template, yo.
if (empty($template))
{
    return FALSE;
}

thank you for suggesting plenty-parser but mergin smarty with the default parser just fits my needs :) I might use it in other projects