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

Adds namedGroup method into MatchResult #11

Merged
merged 1 commit into from
Sep 15, 2016

Conversation

cv21
Copy link
Contributor

@cv21 cv21 commented Sep 15, 2016

Adds support for named groups and fulfill corresponding issue

*
* @param string $groupName
*
* @return mixed
Copy link
Member

Choose a reason for hiding this comment

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

Won't this always return a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, fix it now

@sebastiandedeyne
Copy link
Member

Nice!

Added a small comment, and could you get rid of the .idea direcotry that got committed. Will merge afterwards 👍

@cv21 cv21 force-pushed the feature/match-named-groups branch 2 times, most recently from fbded2a to 6100007 Compare September 15, 2016 13:41
@cv21
Copy link
Contributor Author

cv21 commented Sep 15, 2016

It seems to be ok now

@sebastiandedeyne sebastiandedeyne merged commit cb5099a into spatie:master Sep 15, 2016
@sebastiandedeyne
Copy link
Member

Awesome, thanks!

Sidenote, if I tag a 2.0 someday, I'd probably consolidate the group and namedGroup methods to group (can't remove the int typehint now without it being a breaking change)

@cv21
Copy link
Contributor Author

cv21 commented Sep 15, 2016

Yes, i understood. So, waiting for 2.0 :)

Nice project, @sebastiandedeyne!

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.

None yet

2 participants