blmage / mage-enhanced-admin-grids

[ARCHIVED] Enhanced Admin Grids extension for Magento 1. WIP version available for testing.
248 stars 115 forks source link

Compatibility with HHVM #101

Open paales opened 10 years ago

paales commented 10 years ago

I can't push to te repository and didn't want to maintain a fork. Can you apply the patch below?

From ebc487f125d2a31b9239a1510fca1a0921c0481d Mon Sep 17 00:00:00 2001
From: Paul Hachmang <paul@h-o.nl>
Date: Wed, 27 Aug 2014 12:36:55 +0200
Subject: [PATCH] [Fix] Compatibility with HHVM

---
 app/code/community/BL/CustomGrid/Helper/Config.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/code/community/BL/CustomGrid/Helper/Config.php b/app/code/community/BL/CustomGrid/Helper/Config.php
index 328b636..3a9bfcc 100644
--- a/app/code/community/BL/CustomGrid/Helper/Config.php
+++ b/app/code/community/BL/CustomGrid/Helper/Config.php
@@ -73,8 +73,8 @@ class BL_CustomGrid_Helper_Config extends Mage_Core_Helper_Abstract

     protected function _matchGridAgainstException($exception, $blockType, $rewritingClassName)
     {
-        return (preg_match($exception['block_type'], $blockType)
-            && preg_match($exception['rewriting_class_name'], $rewritingClassName));
+        return (preg_match('~'.$exception['block_type'].'~', $blockType)
+            && preg_match('~'.$exception['rewriting_class_name'].'~', $rewritingClassName));
     }

     public function addGridToExclusionsList($blockType, $rewritingClassName, $reinit=false)
-- 
1.8.4.2
mage-eag commented 10 years ago

Should all the # regex delimiters be replaced with ~ ?

paales commented 10 years ago

I'm not sure what the reason was I replaced the errors, I'll have to dig up the error. There was a related stackoverflow question that suggested this solution, but I can't find it anymore.

mage-eag commented 10 years ago

We have been using HHVM on one of our websites for a week, and we got some warnings that seem to correspond to the same problem, coming from this part of the code :

protected function _getExceptionsListConfig($key)
{
    $exceptions = $this->_getSerializedArrayConfig($key);

    foreach ($exceptions as $key => $exception) {
        // Prepare exceptions patterns
        $exceptions[$key] = array(
            'block_type' => $this->_prepareExceptionPattern($exception['block_type']),
            'rewriting_class_name' => $this->_prepareExceptionPattern($exception['rewriting_class_name']),
        );
    }

    return $exceptions;
}

I don't have read much about HHVM internals yet so I don't really understand what's going on there, but it seems that when using it, the $exceptions array initially use numeric string keys such as "1". The problem here is that it also seems that those keys are turned into integer values when the array is iterated, making all the values be duplicated, and only half of them be valid regexes, later leading to a delimiter error.

This is the only problem (which should easily be fixed) that we got with an everyday use of the extension , but I guess that it will require much more testing to ensure that the extension is fully compatible with HHVM.

paales commented 9 years ago

More information about preg match what is happening: http://stackoverflow.com/questions/7660545/delimiter-must-not-be-alphanumeric-or-backslash-and-preg-match

We just need another delimiter like ~ as mentioned in the stack overflow answer.