[Finder] Removed duplicated toRegex() code #14287
Conversation
| return new Regex('^'.$regex.'$'); | ||
| $regex = FinderGlob::toRegex($this->pattern, $strictLeadingDot, $strictWildcardSlash); | ||
|
|
||
| return new Regex(substr($regex, 1, -1)); |
There was a problem hiding this comment.
you can pass delimiter to Regex class:
return new Regex($regex, '', '#');There was a problem hiding this comment.
The delimiter is hardcoded in the Glob::toRegex(). I can't remove that without breaking BC, hence the reason for stripping it away before turning it into a regex.
There was a problem hiding this comment.
well it look like a hack, why not adding new argument in toRegex ?
public static function toRegex(..., $delimiter = '#')
{
....
}There was a problem hiding this comment.
If that doesnt break any bc rules, i'm happy to change that
There was a problem hiding this comment.
Adding a new optional argument is not a BC break according to our BC promise (http://symfony.com/doc/current/contributing/code/bc.html).
There was a problem hiding this comment.
Perfect, I'll change the PR with this update
|
@aitboudad @fabpot Added an optional argument. However, one thing that is missing here (but this seems to be the case everywhere considering regular expressions in the Finder, is that it always assumes the start and end delimiter to be the same. This is not the case in some occasions: the end-delimiter for |
|
Thank you @jaytaph. |
This PR was merged into the 2.7 branch. Discussion ---------- [Finder] Removed duplicated toRegex() code This patch removes duplicated `toRegex()` code by using the already existing `Glob` class. As this class wasn't unit-tested to begin with, this duplication also makes sure this class is tested properly. | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #14075 | License | MIT Commits ------- 6150c3a Removed duplicated toRegex() code
This patch removes duplicated
toRegex()code by using the already existingGlobclass. As this class wasn't unit-tested to begin with, this duplication also makes sure this class is tested properly.