From: Fabien Potencier <fabien.potencier@gmail.com>
Date: Mon, 21 Nov 2011 07:53:51 +0000 (+0100)
Subject: merged branch shvchk/patch-2 (PR #522)
X-Git-Url: http://git.silmor.de/gitweb/?a=commitdiff_plain;h=b76a5749447eecc4c5b635451d09eb04de9eac81;p=web%2Fkonrad%2Ftwig.git

merged branch shvchk/patch-2 (PR #522)

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.
---

b76a5749447eecc4c5b635451d09eb04de9eac81