Commits
-------
542e9e7 added getKeyValuePairs() on Twig_Node_Expression_Array
3f64f1d added test for hash keys
4bdb3a9 fixed tests
3b1f269 support any expression as hash key
Discussion
----------
Allow hash keys to be any expression
This allows hash keys to be any expression
---------------------------------------------------------------------------
by fabpot at 2011/12/07 02:08:19 -0800
I had not implemented this feature back then because I'm not sure it makes sense to support it in the context of a templating system. I'm still not convinced that this is useful.
Commits
-------
d41d10c simplification
2dbb420 moved hard-coded regexes
61986c1 shortcut for non-interpolated double quoted strings
062ed5c regex optimization
0e63abb integration tests for interpolated strings
773fff5 parser tests for interpolated strings
098634e parser support for interpolated strings
c8bba24 lexer tests for interpolated strings
a203686 lexer support for interpolated strings
0bd63dd stack lexer states
Discussion
----------
String interpolation syntax
This adds support for string interpolation:
```
{{ "foo #{ any expression here } bar" }}
```
I find string interpolation to be often more readable than concatenation, and doesn't have the precedence ambiguity of the concatenation operator:
```
Interpolation:
{{ "foo #{bar} baz" }}
{{ "foo #{1+2} baz" }}
{{ "foo #{1+2} baz"|escape }}
Concatenation:
{{ "foo "~bar~" baz" }}
{{ "foo "~(1+2)~" baz" }} extra parentheses
{{ ("foo "~(1+2)~" baz")|escape }} more extra parentheses
```
---------------------------------------------------------------------------
by nikic at 2011/11/12 12:41:54 -0800
Really nice idea. We should discuss the exact syntax though :) Dart for example uses `${expr}` for interpolation or `$name` if the expression is just a name. I think it makes sense to have a special short syntax for the name case, because it is the most common one.
---------------------------------------------------------------------------
by stof at 2011/11/12 13:06:51 -0800
@nikic could you comment on the PR instead of commenting on each commit individually ? it would make it easier to see what is concerned in the PR.
---------------------------------------------------------------------------
by arnaud-lb at 2011/11/12 14:22:14 -0800
@nikic Thanks for your review, I'll take care of your remarks.
For the syntax, I have no preference. I've taken this one because it has the same semantics. In Ruby and Coffeescript the `#{...}` allows any expression between the brackets. In PHP the expression in the `${...}` must start with a variable name. As you mentioned Dart's `${...}` seems to allow any expression.
---------------------------------------------------------------------------
by nikic at 2011/11/12 15:03:48 -0800
@arnaud-lb If Ruby and Coffeescript use this, the #{...} syntax should be okay. Especially as ${...} might be confusing for PHP developers as it has the semantics of variable variables in PHP.
@stof Thanks, will do so next time.
---------------------------------------------------------------------------
by damianb at 2011/11/12 15:38:31 -0800
@stof he doesn't need to - this is something github handles already. in the PR view, beside the "@nikic commented" bits, there's a set of links that shows where exactly the comment was in the format of commit sha, filename, and line inside the diff.
You can see for yourself right here, upper left:
---------------------------------------------------------------------------
by stof at 2011/11/13 01:20:38 -0800
@damianb The comment appears, but not the context in which the comment was. We have to follow each link to see it whereas comments done on the PR appear directly with their context. This is why I said it is easier when comments are on the PR.
---------------------------------------------------------------------------
by nikic at 2011/12/02 15:13:30 -0800
So what about this now?
Btw, `REGEX_STRING` and `REGEX_DQ_STRING_PART` still are hardcoded and do not depend on the specified `interpolation` tags. But this looks hard to change, at least the regexes are quite complicated.
---------------------------------------------------------------------------
by arnaud-lb at 2011/12/02 15:22:45 -0800
I think I've addressed all issues (except hardcoding of `#{` and `}` in `REGEX_STRING` and `REGEX_DQ_STRING_PART`.)
Can this be merged after v1.4 is release if the syntax is accepted ?
---------------------------------------------------------------------------
by nikic at 2011/12/03 01:53:41 -0800
This would be my go for `STRING` and `DQ_STRING_PART` (isn't tested):
```php
<?php
$interpolChar = preg_quote(substr($options['interpolation'][0], 0, 1), '/');
$interpolPart = '[^' . $interpolChar . '"\\\\]*';
if (strlen($options['interpolation'][0]) > 1) {
$interpolRest = preg_quote(substr($options['interpolation'][0], 1), '/');
$interpolRegex = $interpolPart . '(?:(?:\\\\.|' . $interpolChar . '(?!' . $interpolRest . '))' . $interpolPart . ')*';
} else {
$interpolRegex = $interpolPart . '(?:\\\\.' . $interpolPart . ')*';
}
$this->options['dq_string_part_regex'] = '/' . $interpolRegex . '/As';
$this->options['string_regex'] = '/"' . $interpolRegex . '"|\'([^\'\\\\]*(?:\\\\.[^\'\\\\]*)*)\'/As';
```
It should allow both a single char interpolation syntax (like `{`) and multichar ones (like `#{`).
By the way, could we maybe move the compiled regexes out of `$this->options` into `$this->regexes`?
Commits
-------
989bb22 Removed trailing white spaces
f880bab Help the developer when they specify an invalid filter by providing some alternatives
Discussion
----------
Help the developer when they specify an invalid filter by providing some...
... alternatives.
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Uses a simple `strpos` search (no fancy diff to determine what the filter could be). There is no impact to runtime performance because this is all happening during template compilation.
Examples:
```jinja
{{ my_entity|json }}
```
Shows exception:
`The filter "json" does not exist. Did you mean "json_encode"?`
```jinja
{{ my_date|dat }}
```
Shows:
`The filter "dat" does not exist. Did you mean "date"?`
```jinja
{{ my_var|f }}
```
Shows:
`The filter "f" does not exist. Did you mean "format", "default", "_default", "format_args", "format_args_as_text", "file_excerpt", "format_file", "format_file_from_text", "file_link"?`
Also good for custom filters created in your own domain `st_`:
```jinja
{{ my_var|st_ }}
```
Shows:
`The filter "st_" does not exist. Did you mean "st_friendly_date", "st_site_test_url"?`
---------------------------------------------------------------------------
by stof at 2011/12/04 16:34:27 -0800
I like the idea to provide help to the developer in case of error. You should probably do the same for functions
---------------------------------------------------------------------------
by Tobion at 2011/12/05 05:56:29 -0800
+1 for a combination of string starts with (suggesting domain specific filters, e.g. st_*) and levenshtein for typos
---------------------------------------------------------------------------
by chucktrukk at 2011/12/05 08:13:10 -0800
+1. This would be really helpful.
Commits
-------
11b8689 Refactoring: using && instead of nested if's
7aa6757 Bug correction: Parsing integers large than PHP_INT_MAX was generating trucated token values.
046e4ff Bug correction: Parsing integers large than PHP_INT_MAX was generating trucated token values.
Discussion
----------
Fixed bug that was trucating integers large then PHP_INT_MAX
The following code
{% set sizes = [7077888, 452984832, 28991029248, 1855425871872, 9223372036854775807] %}
{% for i in sizes %}
{{ i }}
{% endfor %}
was generating the following output
7077888
452984832
2147483647
2147483647
2147483647
With the fix, the output looks like the following on Ubuntu 11.04
7077888
452984832
28991029248
1855425871872
9.2233720368548E+18
uses a static variable to avoid initializing the charsets array each
time the function is called; and does a hash lookup instead of array
search (saves a function call too).
Commits
-------
5f30db0 !empty($date[0]) → !empty($date)
0182dbc indentation fix
c7a06cb php warning fix
a4a2d6f Support of negative timestamps
Discussion
----------
Support of negative timestamps
Date could be checked against this condition, as proposed in my patch:
```php
ctype_digit((string) $date)
|| (('-' === $date[0])
&& (ctype_digit(substr($date, 1))))
```
or this:
```php
preg_match('/^\-?\d+/', $date)
```
Second is maybe more readable, but 20% slower.
---------------------------------------------------------------------------
by nikic at 2011/11/20 01:59:16 -0800
Couldn't we do an int validation here? I.e. `if (false !== filter_var($date, FILTER_VALIDATE_INT)) { ... }`. That will allow both positive and negative integers in the allowed integer range. Note though that it will no longer allow ints starting with a zero (apart from zero itself). To support that one would need to pass the ALLOW_OCTAL flag.
Some tests: http://codepad.viper-7.com/w0MDTH
---------------------------------------------------------------------------
by damianb at 2011/11/20 07:50:39 -0800
@nikic why not `ctype_digit`?
---------------------------------------------------------------------------
by nikic at 2011/11/20 07:53:26 -0800
@damianb ctype_digit is what this PR wants to replace, because it does not allow negative numbers. Replacing it with `ctype_digit((string) $date) || (!empty($date[0]) && ('-' === $date[0]) && ctype_digit(substr($date, 1)))` seems like overkill to me though, that's why I proposed to use `false !== filter_var($date, FILTER_VALIDATE_INT)` instead, which apart from being shorter is also semantically more correct.
---------------------------------------------------------------------------
by damianb at 2011/11/20 08:03:09 -0800
@nikic But does filter_var with FILTER_VALIDATE_INT perform better than what's proposed?
Also, I would think that the `empty($date[0])` should just be replaced with an empty check on the `$date` var itself - seems quite pointless to check if the first character is empty, that'd imply that either a) the string itself is empty b) `date[0] == 0`, which could result in a nasty gotcha for someone down the road.
---------------------------------------------------------------------------
by shvchk at 2011/11/20 09:36:17 -0800
@damianb, you are right, I will update the code.
@nikic, on 32-bit systems ```PHP_INT_MAX``` is ```2147483647```, so your code will fail on such systems for 01.01.1900, for example, which is ```-2208988800``` and is not that rarely used.
Eugene Serkin
Fabien Potencier
nikic
JEDIBC
Jonathan Ingram
Paulo Roberto Ribeiro
Martin Hasoň
Arnaud Le Blanc
Tobias Schultze
Pierre Joye
