Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Finder] Glob::toRegex is duplicated #14075

Closed
jaytaph opened this issue Mar 26, 2015 · 1 comment
Closed

[Finder] Glob::toRegex is duplicated #14075

jaytaph opened this issue Mar 26, 2015 · 1 comment

Comments

@jaytaph
Copy link
Contributor

jaytaph commented Mar 26, 2015

It seems that the toRegex method in Finder\Glob.php is duplicated in the Finder\Expression\Glob.php. There are some small differences in the code though:

   <     public static function toRegex($glob, $strictLeadingDot = true, $strictWildcardSlash = true)
   ---
   >     public function toRegex($strictLeadingDot = true, $strictWildcardSlash = true)
   7c7
   <         $sizeGlob = strlen($glob);
   ---
   >         $sizeGlob = strlen($this->pattern);
   9c9
   <             $car = $glob[$i];
   ---
   >             $car = $this->pattern[$i];
   55c55
   <         return '#^'.$regex.'$#';
   ---
   >         return new Regex('^'.$regex.'$');

None which seems strange. It might be a good idea to actually remove the toRegex from the Expression/Glob.php method, and instead use the Glob::toRegex in its place (this file is not unit-tested neither).

Any suggestions / comments on this? I'm happy to create a PR for this.

@jaytaph jaytaph changed the title [Finder] Glob::toRegex is duplicate [Finder] Glob::toRegex is duplicated Mar 26, 2015
@fabpot
Copy link
Member

fabpot commented Mar 27, 2015

Happy to review a PR on that :)

fabpot added a commit that referenced this issue Apr 15, 2015
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
@fabpot fabpot closed this as completed Apr 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants