Fix handling of NULLs when replacing query params #169

Merged
merged 1 commit into from Oct 12, 2013

Conversation

Projects
None yet
3 participants
Contributor

lazyhammer commented Mar 13, 2013

No description provided.

@stof stof and 1 other commented on an outdated diff Mar 14, 2013

Twig/DoctrineExtension.php
@@ -269,6 +269,10 @@ static public function escapeFunction($parameter)
case is_object($result) :
$result = addslashes((string) $result);
break;
+
+ case is_null($result) :
@stof

stof Mar 14, 2013

Member

Please use ``null === $result` to be consistent with our coding standards

@lazyhammer

lazyhammer Mar 14, 2013

Contributor

Fixed

@Baachi Baachi commented on the diff Mar 14, 2013

Twig/DoctrineExtension.php
@@ -290,11 +294,11 @@ public function replaceQueryParameters($query, $parameters)
'/\?|(:[a-z0-9_]+)/i',
function ($matches) use ($parameters, &$i) {
$key = substr($matches[0], 1);
- if (!isset($parameters[$i]) && !isset($parameters[$key])) {
+ if (!array_key_exists($i, $parameters) && !array_key_exists($key, $parameters)) {
@Baachi

Baachi Mar 14, 2013

Is this change really necessary?

@lazyhammer

lazyhammer Mar 14, 2013

Contributor

Try something like replaceQueryParameters('?, ?', array(1, null)) or replaceQueryParameters('?, ?', array(null, 1)) and you'll see :)

@stof stof added a commit that referenced this pull request Oct 12, 2013

@stof stof Merge pull request #169 from lazyhammer/runnable-query-fix-null
Fix handling of NULLs when replacing query params
f61a9b2

@stof stof merged commit f61a9b2 into doctrine:master Oct 12, 2013

1 check failed

default The Travis build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment