Smarty

[rdalverny]

Hi there,

while working on a small project, I got several 'notice' errors that I wanted to get fixed, and here are my comments about it.

First, handling xml tags ; I had these headers in my header template :

[boots]

hi rdalverny,

1] I am using 2.6.0 RC2 and I could not reproduce your notices:
a) what version of PHP are you using?
b) what version of Smarty are you using?
c) not nitpicking, but in your example, shouldn't your xml tags be closed, ie: with ">"
d) what delimiters are you using for Smarty?
e) also, my understanding is that the special xml document tag (the first one) is only applicable at the top of a document -- analogous to http header(). Perhaps it is better to output that outside of the template?

2] most likely, those notices are a good thing as someone probably forgot to pass an important parameter. Otherwise I couldn't really understand the issue in your second comment--sorry!!

[rdalverny]

Hi boots,

1. XML tags.

a) and b) I am using Smarty 2.6.0. RC2 too, with PHP v4.2.3, Apache 1.3.26.

c) Actually, XML tags should be closed, but that is a forum bug (I did not notice, but when editing my post, closing tags are there...).

d) I use standard Smarty delimiters { and }.

e) You are quite right, but since XML documents tags may not appear at the top of all documents (think plain text templates, or other XML templates, with different encodings), I think it's better to put them inside the template. But that could be a matter of discussion.


2. Notices. They are a good thing, if someone forgot to pass an important parameter. Maybe I missed the point in using include_php, but when doing a simple include_php like this :

[rdalverny]

The code I wrote above for _compile_include_php_tag() is a patched version (which understands 'once' and 'assign' as optional params) of the same function in Smarty 2.6.0. RC2 I use in production, not the actual version.

[messju]

to me there is no "performance over correctness" here either, because we're in the compiler.

i removed the notices from _compile_include_php_tag().

the ones regarding the php_handling-stuff are not that trivial to fix but i'll look into it.

boot's argument that xml-tags should be closed is void to me. smarty does not know what kind of text the template contains and must not make any assumptions about the stuff that are not smarty-tags.

greetings
messju

[messju]

grmpf. i get rid of the notice in _compile_file() when i change line 295 (of current CVS's smarty_compiler.class.php)

from

[rdalverny]

The problem may be some lines below instead (depends on what the function actually does, I am not sure) : you have a for() loop over $sp_match[0] array (that is, over every matching tag), but in the if/elseif blocks, you use $curr_sp as an index for $sp_match[1] which may be smaller than $sp_match[0].

So what should be changed is $sp_match[1] into $sp_match[0] in the following lines.

[messju]

[rdalverny]

Yes, you are right ; I thought the regexp would have taken into account what was between PHP-like tags too : I have read the code a bit too fast. Actually, your correction is the correct one : replacing $sp_match[0] into $sp_match[1] in the for() statement.

Thanks a lot.

[messju]

okay. i committed the fix i proposed. i wasn't able to find template-code that may break it. thanks for pointing this out!

[mFeroz]

[quote="rdalverny"]Hi there,

while working on a small project, I got several 'notice' errors that I wanted to get fixed, and here are my comments about it.

[b]First[/b], handling xml tags ; I had these headers in my header template :

[code]<?xml version="1.0" encoding="iso-8859-1" ?>
<?xml-stylesheet type="text/css" media="screen" href="{$webRoot}/lib/style/camoastyle.css" ?>[/code]

which triggered this notice : Undefined offset: 2, in Smarty_Compiler.class.php line 298 ; actually, it is about $sp_match being set with <?php lookalike tags, the problem being that xml tags closely look like php tags.
The regexp responsible of that is there, line 287, same file :
[php]preg_match_all('!(<\?(?:\w+|=)?|\?>|language\s*=\s*[\"\']?php[\"\']?)!is', $text_blocks[$curr_tb], $sp_match)[/php]

I'm no expert in regexp actually, so I don't blame anyone, but if someone could fix this,
or point me to a solution (apart from leaving xml stuff away from my templates), that would be nice.

[b]Second remark[/b], about the _compile_include_php_tags() function, which does not handle correctly missing parameters.
If default attributes 'once' and 'assign' are not set in include_php call, notice errors still appear, because the function assumes they are set ; for instance, when doing [code]{include_php file='myFile.php'}[/code].

Moreover, $arg_list is not declared (as an empty array), which just make another notice if null. So here is a suggestion.

I know PHP is a weak-typed langage, but I think that is good practice to declare every variable you use ; for security and consistency purpose.
The "performance over correctness" issue does not seem to me to apply neither : we are
using a caching-capable system, and it is not sure that error raising (which internally occurs anyway) eats less cpu cycles than just a correct test or declaration.

So, in _compile_include_php_tags, around line 925 in Smart_Compiler.class.php :
[php]function _compile_include_php_tag($tag_args)
{
$attrs = $this->_parse_attrs($tag_args);

if (empty($attrs['file'])) {
this->_syntax_error("missing 'file' attribute in include_php tag", E_USER_ERROR, __FILE__, __LINE__);
}

if( !isset( $attrs['assign'] ) )
$assign_var = '';
else
$assign_var = $this->_dequote($attrs['assign']);

if( !isset( $attrs['once'] ) )
$once_var = '';
else
$once_var = ( $attrs['once'] == 'false' ) ? 'false' : 'true';

$arg_list = array();
foreach($attrs as $arg_name => $arg_value) {
...
}[/php][/quote]