PHP 7.2 has deprecated the each() function, and this module uses it twice in DiffEngine.php. It can usually be replaced with a foreach().
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | diff-php7-compat-2946139-4.patch | 836 bytes | bkosborne |
| #3 | remove_deprecrated_each-2946139-3.patch | 788 bytes | malaynayak |
Comments
Comment #2
malaynayak commentedComment #3
malaynayak commentedHi @bkosborne,
Please review the patch.
Comment #4
bkosborneNo, I don't think that's right. I think we need to preserve the location of the internal array pointer between these two loops. Foreach won't do that.
Comment #5
mrgoodfellow commentedLooking at this issue: https://www.drupal.org/project/drupal/issues/2885309
it recommends the code in this patch.
however when patching I get the following error:
Fatal error: Undefined class constant 'USE_ASSERTS' in \sites\all\modules\contrib\diff\DiffEngine.php on line 318
Comment #6
mrgoodfellow commentedBased on this patch: https://www.drupal.org/files/issues/2885309-43.patch
and this issue: https://www.drupal.org/project/drupal/issues/2885309
However it does not appear to function and throws a fatal error:
Fatal error: Undefined class constant 'USE_ASSERTS' in \sites\all\modules\contrib\diff\DiffEngine.php on line 318
Comment #7
chris burge commentedComment #9
bkosborne@mrgoodfellow, it seems like you just copied and pasted code from Drupal 8 as a replacement? That's not necessary. Did you try the patch I posted in #4? It should work fine.
Comment #10
joseph.olstadI just got this too running the diff module with php 7.2.
Patch #4 works perfectly, thanks!
Comment #11
joseph.olstadGreat work as usual bkosborne,
FYI: php 7.2 support for D7 core is comming in the next release window, (confirmed).
Lets get this one in for this win.
Comment #13
joseph.olstadpatch #4 works great
Please commit this and tag a release , php 7.2.x support is for now. php 7.3.x is already out.
Comment #15
alan d. commentedTa.
bkosborne was totally right about the internal array pointer, foreach resets this each time, but the horrible old code uses one loop to find and break, and the second loop to progress from the last read element after the break.
e.g.
each() == [key(), current()] + next().@joseph.olstad
Sorry, was meaning to give cred in the commit
@mrgoodfellow
Best to check what your doing when posting patches. D8 version of Diff is completely different to D7 and clearly that patch wouldn't have worked at all.
Comment #16
joseph.olstad@Alan D , thank you very much!
The only thing more that I could ask for is a tagged release! :) been two years.
php 7.2.x compatibility is worth tagging and publishing a release for.
Comment #17
alan d. commentedNot really that much in dev, one minor change, these two PHP 7.2 bugs really
Changes since 7.x-3.3
#2946139: Remove usage of deprecrated each() function for PHP 7.2+ compatability by bkosborne, malaynayak, joseph.olstad (for PHP 7.2)
#3002285: Remove usage of deprecated assert() on string for PHP 7.2+ compatibility by Chris Burge (for PHP 7.2)
#2872108: Split field label and machine name in field table by hass
#2845895: diff_update_7307 assumes the module is enabled. by torotil, Alan D.
#2860650: Assertion failed in _HWLDF_WordAccumulator->addWords() by joelpittet, Alan D.
Maybe around the time D7.61 comes out
Comment #18
joseph.olstadPlease release diff, 7.61 is comming (very soon)
I have spoken with the core maintainers, this is why I've published a release for features, for php 7.2.x compatiblity, I've already published several other contrib releases that include php 7.2.x compatiblity. It would be nice if this was too.
Comment #19
alan d. commentedJust for Joseph ;)
https://www.drupal.org/project/diff/releases/7.x-3.4
Give it a few minutes to publish.
Comment #20
joseph.olstadThanks!