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

Added UUID pattern support for the TextMatcher #130

Merged

Conversation

thomasbisignani
Copy link
Contributor

@thomasbisignani thomasbisignani commented Apr 25, 2018

This PR resolves the #112 issue. It enables to use the @uuid@ pattern with the TextMatcher.

This pattern is actually not supported : Type pattern "@uuid@" is not supported by TextMatcher.

After this patch, we can use :

if (!PHPMatcher::match("/user/ebd1fb0e-45ae-11e8-842f-0ed5f89f718b/profile", "/user/@uuid@/profile", $error)) {
    echo $error;
} else {
    echo "Matched.";
}

@thomasbisignani thomasbisignani changed the title Added Uuid matcher into the RegexConverter Added UUID matcher into the RegexConverter Apr 25, 2018
@thomasbisignani thomasbisignani force-pushed the feature/added-uuid-regexconverter branch from 90ea224 to 7ff9063 Compare April 25, 2018 09:14
@thomasbisignani thomasbisignani changed the title Added UUID matcher into the RegexConverter Added UUID pattern support for the TextMatcher Apr 25, 2018
@thomasbisignani thomasbisignani force-pushed the feature/added-uuid-regexconverter branch from 7ff9063 to 86df2d3 Compare April 25, 2018 09:19
@@ -21,6 +21,8 @@ public function toRegex(TypePattern $typePattern) : string
return '(\\-?[0-9]*)';
case 'double':
return '(\\-?[0-9]*[\\.|\\,][0-9]*)';
case 'uuid':
return "([\da-f]{8}-[\da-f]{4}-[1-5][\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12})";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we already have this pattern somewhere, maybe in UuidMatcher? Could you try to reuse it without duplicatuin?

Copy link
Contributor Author

@thomasbisignani thomasbisignani Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes of course, the pattern const in the UuidMatcher includes the start / end position :
'|^[\da-f]{8}-[\da-f]{4}-[1-5][\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12}$|';

So it's ok for this update in the UuidMatcher ?

const UUID_PATTERN = '[\da-f]{8}-[\da-f]{4}-[1-5][\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12}';
const MATCH_PATTERN = '/^'.self::UUID_PATTERN.'$/';

We will can reuse the UUID_PATTERN const in the RegexConverter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually to avoid BC break we would need to have 3 consts. Those 2 that you mentioned (with different names) above and 1 with not changed name builded from those 2.
However I don't believe anybody would use this pattern anywhere outside php-matcher, but still it's better to be safe then sorry

Copy link
Contributor Author

@thomasbisignani thomasbisignani Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a new commit with these two constants :

const UUID_PATTERN = '[\da-f]{8}-[\da-f]{4}-[1-5][\da-f]{3}-[89ab][\da-f]{3}-[\da-f]{12}';
const UUID_FORMAT_PATTERN = '|^'.self::UUID_PATTERN.'$|';

@norberttech norberttech merged commit 248184b into coduo:master Apr 25, 2018
@norberttech
Copy link
Member

Thanks 👍

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

Successfully merging this pull request may close these issues.

2 participants