Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
As a first small result of my effort to replace as much as i can handle occurences of ereg* functions by preg_match in Drupal ( http://groups.drupal.org/node/544 ), this is the first patch i am creating. Please let me know if i am doing things well as i am new to Drupal development :)
This patch removes ereg() in includes/file.inc at:
elseif ($depth >= $min_depth && ereg($mask, $file)) {
At my point of view, everything is working with preg_match().
Attaching patch (is this correct? should i put it elsewhere?)
Comment | File | Size | Author |
---|---|---|---|
#24 | preg_64967.patch | 12.07 KB | drewish |
#23 | preg_64967.patch | 14.56 KB | drewish |
#15 | drupal-preg-64967-15.patch | 4.64 KB | cburschka |
#7 | eregpreg_0.patch | 2.04 KB | meba |
#5 | eregpreg.patch | 2.04 KB | meba |
Comments
Comment #1
meba CreditAttribution: meba commentedWell, i have a typo in the patch, attaching a new one
Comment #2
chx CreditAttribution: chx commentedAwesome idea. But the code needs work. Please fix the caller places to pass pregs.
image.inc:10: $toolkits = file_scan_directory('includes', 'image\..*\.inc$');
system.module:591: foreach (file_scan_directory(dirname($theme->filename), 'style.css$') as $style) {
system.module:689: * The key to be passed to file_scan_directory().
system.module:707: $files = array_merge($files, file_scan_directory($dir, $mask, array('.', '..', 'CVS'), 0, TRUE, $key, $min_depth));
system.module:547: $themes = system_listing('\.theme$', 'themes');
system.module:550: $engines = system_listing('\.engine$', 'themes/engines');
system.module:696:function system_listing($mask, $directory, $key = 'name', $min_depth = 1) {
system.module:893: $files = system_listing('\.module$', 'modules', 'name', 0);
In the future someone might change them and then it'll be crucial for them to be full pregs. Also, I really do not advice using a slash for a delimiter, because we are dealing with files, paths and such. Someone might write
[^/]
so I'd recommend you ^lt; > as delimiters. Or #.Comment #3
chx CreditAttribution: chx commentedI wanted to say < and > but an obvious typo stopped this.
Comment #4
meba CreditAttribution: meba commentedok, will do next week, now i am working on my exams at university .)
Comment #5
meba CreditAttribution: meba commentedRerolling against 5.x. Searched for all eregs using file_scan_directory and should be fixed.
Test please
Comment #6
RobRoy CreditAttribution: RobRoy commentedComment #7
meba CreditAttribution: meba commentedComment #8
RobRoy CreditAttribution: RobRoy commentedNot related to your patch specifically, but should we prefix the style line with a '^' to not match somestyle.css also?
Comment #9
drummComment #10
Gábor HojtsyI bet this does not apply to Drupal 6 at all. But a nice undertaking to start with :)
Comment #11
Gábor HojtsyI mean the patch does not apply, not the idea :) So the patch probably needs to be rerolled.
Comment #12
catchComment #13
PanchoGood start, indeed! We need to get away from ereg soon, as POSIX style regex are to be removed from PHP6.
However I think we should tackle this once and for all as a big task, therefore D7.
As this will break Drupal in PHP6, it is definitely a critical task.
Comment #14
catchhttp://drupal.org/node/286893 was duplicate, but has a new patch. http://drupal.org/files/issues/drupal-preg-286893-4.patch
Comment #15
cburschkaIt's easier to follow an issue when the patch is attached directly.
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedSubscribing
Comment #17
lilou CreditAttribution: lilou commentedStill apply.
Comment #18
Susurrus CreditAttribution: Susurrus commentedIf we do this then this additional issue should be addressed: #270824: Require preg_* functions for regular expressions.
Comment #19
Robin Monks CreditAttribution: Robin Monks commentedPlease note, this issue will be affected by this as well: http://drupal.org/node/74645
+1 for replacing ereg with preg!
Patch still applies, and appears to work perfectly. RTBC!
Robin
Comment #20
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #21
drewish CreditAttribution: drewish commenteddid we intentionally skip the ereg() in file_scan_directory()?
Comment #22
Dave ReidNot sure if it was intentional, but it's definitely the last instance left in HEAD. I'll see if I can patch that to preg_match today.
Comment #23
drewish CreditAttribution: drewish commentedhere's a first wack at this. i ran through the installer and enabled all the modules with no problems. running the tests now.
Comment #24
drewish CreditAttribution: drewish commentedfinished the tests and found one regex i'd missed in simpletest.
Comment #25
drewish CreditAttribution: drewish commentedsince it's only affecting the file_scan_dir now i'll bump the component.
Comment #26
webchickHm. Could we not keep the regexes in the calling function the same, and surround the pattern with "/ ... /" in file_scan_directory()?
Comment #27
drewish CreditAttribution: drewish commentedwebchick, we could do that but we'd loose the ability to use case insensitive regular expressions. And this is a more honest way of doing it that won't lead to subtle bugs where ereg expressions fail with preg.
Comment #28
webchickWell, that's true. And also, people could also use a different delimiter like # if they wanted to not have to escape /s or whatnot.
Reading this code reminds me that #255551: DX: Array-itize file_scan_directory()'s parameters and #74645: modify file_scan_directory to include a regex for the nomask. would both be nice improvements to file_scan_directory(). Those 120+ character function calls are nasty. ;)
Re-ran the file tests and everything looks good, so committed this. Thanks!
The change to file_scan_directory()'s parameters needs to be noted in the upgrade docs, so marking this code needs work until then.
Comment #29
drewish CreditAttribution: drewish commentedjust submitted: http://drupal.org/node/224333#preg_match
Comment #30
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.
Comment #31
donquixote CreditAttribution: donquixote commentedWhat can we do for 6.x ?
I see this issue being closed since October 2008, but in 6.x we still have ereg() instead of preg_match(). With all the nasty errors in PHP 5.3.
Should we make a new issue for 6.x, or switch this one from 7.x to 6.x ?
I would like to point out that the issue was reported for x.y.z (?), then switched to 5.x, then to 6.x. Now it's fixed in D7, but then was forgotten with no backport for almost one year! Is there something wrong with the issue workflow?
EDIT:
Would "@ereg" instead of "ereg" be ok as a temporary fix?
Comment #32
Dave ReidUsing preg() instead of ereg() in Drupal 6 would break existing APIs, so it will never happen. Your best bet is to set your PHP settings to not show E_DEPRECATED errors. See http://us2.php.net/manual/en/function.error-reporting.php
Comment #33
sinasquax CreditAttribution: sinasquax commentedWe can keep the compatibility and adds this feature by adding an extra parameter to the function :
With this, the old functions which use file_scan_directory will not break and the new functions which need preg can set the last parameter to TRUE
Comment #34
sinasquax CreditAttribution: sinasquax commentedComment #36
Druid CreditAttribution: Druid commentedIt has to happen sooner or later. The "ereg" family of functions (as well as many other "deprecated" functions) are going away in PHP 6.0. Gone. Period. I suppose you could suppress the warning messages until then, and add emulation wrappers for PHP 6+ users at the time they upgrade to 6, but why not bite the bullet and do it right, by changing all the "ereg" family calls?
Comment #37
Damien Tournoud CreditAttribution: Damien Tournoud commentedereg() is still marked as deprecated, but it's not going away.
Comment #38
Druid CreditAttribution: Druid commentedHas PHP changed its plans? ereg is still marked as "deprecated", which means they plan(ned) to remove it at some point. Last I heard, that was PHP 6.0 (which isn't that far off).
Comment #39
Damien Tournoud CreditAttribution: Damien Tournoud commentedPHP 6 doesn't exist (and will probably never exists), and ereg is not scheduled to be removed anymore because the full Unicode support is not on the table anymore.
Anyway, Drupal 6 will probably be end-of-life before PHP vNext makes its way to major LAMP distributions.
Comment #40
Druid CreditAttribution: Druid commentedAh, I had not heard that PHP 6 development has been suspended. Is there any official word on what they plan to do in future releases? Are "ereg" family functions, among other things, still scheduled to be removed at some point in the near future? I would assume, until otherwise officially informed, that the PHP 6 effort is not dead, but merely in stand-down for some period while they regroup and come up with a new roadmap.
In the meantime, barring any official PHP word to the contrary, it would be safe to assume that "ereg" is going away and that "preg_*" should be substituted for it. If this has not been done before word comes down from PHP that "ereg" is here to stay, then no big deal. If it's done, and "ereg" is declared to stay, no great loss. However, if "ereg" is removed some time soon, it could make a mess if Drupal isn't ready for it! I'm not sure I'm ready to play the odds and risk that "ereg" is permanently staying in PHP, absent official word that it is.
Comment #41
Damien Tournoud CreditAttribution: Damien Tournoud commentedDruid: to be very clear: