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

[Regex] Introduce *_match() functions to fetch captured data groups #151

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

veewee
Copy link
Collaborator

@veewee veewee commented Mar 5, 2021

This PR introduces a first_match() and every_match() function in which you can specify the shape of the result:

/** @var array{0: string, name: string}|null $matches */
$matches = Regex\first_match(
    'Hello world',
    '/(hello) (?<name>world)/i',
    Regex\capture_groups(['name'])
);
/** @var list<array{0: string, name: string, digit: string}>|null $matches */
$matches = Regex\every_match(
    <<<EODATA
    a: 1
    b: 2
    c: 3
    EODATA,
    '@(?P<name>\w+): (?P<digit>\d+)@i',
    Regex\capture_groups(['name', 'digit'])
);

src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Mar 5, 2021

Pull Request Test Coverage Report for Build 733281690

  • 33 of 33 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 725317265: 0.0%
Covered Lines: 2982
Relevant Lines: 2982

💛 - Coveralls

src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
@azjezz azjezz added Priority: High After critical issues are fixed, these should be dealt with before any further issues. Type: Enhancement Most issues will probably ask for additions or changes. labels Mar 5, 2021
@azjezz azjezz added this to the 1.5.0 milestone Mar 5, 2021
src/Psl/Regex/matching.php Outdated Show resolved Hide resolved
@veewee veewee force-pushed the regex-matching branch 3 times, most recently from f138529 to 58bb2d3 Compare March 8, 2021 07:45
src/Psl/Type/cast.php Outdated Show resolved Hide resolved
@veewee veewee force-pushed the regex-matching branch 2 times, most recently from ce89775 to dd8cc8f Compare March 8, 2021 13:27
@veewee
Copy link
Collaborator Author

veewee commented Mar 8, 2021

Introduced the every_match function as well. The failing test is not caused by the code, but by GH actions:

✗ PHP Could not setup PHP 7.4

@veewee veewee changed the title [Regex] Introduce matching() function [Regex] Introduce *_match() functions to fetch captured data groups Mar 8, 2021
@veewee veewee force-pushed the regex-matching branch 2 times, most recently from d8b9338 to 0b1613f Compare March 15, 2021 14:43
@veewee
Copy link
Collaborator Author

veewee commented Mar 15, 2021

@azjezz Fixed it without using a psalm plugin. Instead the conditional return types of psalm did the trick together with a fallback to a custom $capture_groups fallback type.

@azjezz azjezz modified the milestones: 1.5.0, 1.6.0 Mar 19, 2021
@azjezz azjezz changed the base branch from 1.5.x to 1.6.x March 19, 2021 04:13
@veewee
Copy link
Collaborator Author

veewee commented Mar 19, 2021

Rebased it to 1.6.X. Let me know if there is anything else I can do in order to finish this one!

@veewee
Copy link
Collaborator Author

veewee commented Mar 30, 2021

@azjezz How do you want to continue with this PR? Is it something you want to include? Because if so, I need to move a part to the new plugin repository.

@azjezz
Copy link
Owner

azjezz commented Mar 30, 2021

yes, let's move things to the new plugin, merge changes there, then we merge this.

@veewee
Copy link
Collaborator Author

veewee commented Mar 30, 2021

Allright, will do it soon - but having some time constraints due to yet another lockdown here.
Hopefully something for next week.

@azjezz azjezz modified the milestones: 1.6.0, 1.7.0 Apr 7, 2021
@veewee
Copy link
Collaborator Author

veewee commented Apr 9, 2021

Added the PR to the psalm plugin, but It will conflict with the open issue - so it's not completely done.
php-standard-library/psalm-plugin#5

Next rebased against 1.7.x - shall I keep the psalm plugin changes in here as well or remove it from the PR?

@azjezz
Copy link
Owner

azjezz commented Apr 9, 2021

Next rebased against 1.7.x - shall I keep the psalm plugin changes in here as well or remove it from the PR?

remove it, we don't want to add anything to the old plugin, just keeping it to not break BC.

php-standard-library/psalm-plugin#3 has been merged, so i guess you can rebase php-standard-library/psalm-plugin#5

@azjezz
Copy link
Owner

azjezz commented Apr 9, 2021

overall, this LGTM.

I'll probably do another review later and merge if everything is okay.

we can release this, along side #138 in 1.7 :)

@azjezz azjezz changed the base branch from 1.6.x to 1.7.x April 9, 2021 19:21
@azjezz azjezz added the Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness label Apr 9, 2021
@azjezz azjezz linked an issue Apr 9, 2021 that may be closed by this pull request
@azjezz azjezz merged commit 5a7fde2 into azjezz:1.7.x Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High After critical issues are fixed, these should be dealt with before any further issues. Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regex API
3 participants