Skip to content

Comments

Add RegexMatchValidator#19

Open
sukhwinder33445 wants to merge 4 commits intomainfrom
add-regex-validator
Open

Add RegexMatchValidator#19
sukhwinder33445 wants to merge 4 commits intomainfrom
add-regex-validator

Conversation

@sukhwinder33445
Copy link
Contributor

@sukhwinder33445 sukhwinder33445 commented Dec 28, 2022 •
edited by Al2Klimov
Loading

TODOs:

  • Add Tests

@cla-bot cla-bot bot added the cla/signed label Dec 28, 2022
@sukhwinder33445 sukhwinder33445 marked this pull request as draft December 28, 2022 14:58
@sukhwinder33445 sukhwinder33445 self-assigned this Dec 28, 2022
Al2Klimov
Al2Klimov reviewed Jan 13, 2026
View reviewed changes
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

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

Your validator is named RegexValidator, though it doesn't validate a regex. It validates plain text as specified by a regex.

Moreover: I shall make a validator which actually validates a regex.

Let's agree upon names which both don't conflict and are obvious to lib users! Any suggestions?

@sukhwinder33445
Copy link
Contributor Author

sukhwinder33445 commented Jan 13, 2026

What about PatternMatchValidator ? For example, InArrayValidator checks whether a value is in an array; this class would check whether a value matches a pattern.

@lippserd
Copy link
Member

lippserd commented Jan 13, 2026

What about PatternMatchValidator ? For example, InArrayValidator checks whether a value is in an array; this class would check whether a value matches a pattern.

I like that. What about RegexSyntaxValidator and RegexMatchValidator? That makes it perfectly clear in my opinion.

Al2Klimov
Al2Klimov reviewed Jan 13, 2026
View reviewed changes
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

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

Please rename to RegexMatchValidator.

@sukhwinder33445 sukhwinder33445 changed the title Add RegexValidator Add RegexMatchValidator Jan 14, 2026
@sukhwinder33445 sukhwinder33445 force-pushed the add-regex-validator branch 2 times, most recently from 9545d87 to d20f7fb Compare January 14, 2026 10:34
@lippserd
Copy link
Member

lippserd commented Jan 14, 2026

Since this PR has been in the draft stage for quite some time, I wonder if we really have a use case for this validator. If so, I'd suggest we finish it. Otherwise, we can just close the PR.

@sukhwinder33445
Copy link
Contributor Author

sukhwinder33445 commented Jan 14, 2026

Yes, there are currently no use cases.

@sukhwinder33445 sukhwinder33445 deleted the add-regex-validator branch January 14, 2026 15:01
@Al2Klimov
Copy link
Member

Al2Klimov commented Feb 5, 2026

Use case

In https://git.icinga.com/aklimov/pocssooidc/-/merge_requests/1, I'd like to ensure that URLs start only with ~^https?://~.

@Al2Klimov Al2Klimov restored the add-regex-validator branch February 5, 2026 09:11
@Al2Klimov Al2Klimov reopened this Feb 5, 2026
@cla-bot

This comment was marked as resolved.

Sign in to view
@cla-bot cla-bot bot removed the cla/signed label Feb 11, 2026
@Al2Klimov Al2Klimov assigned bobapple and unassigned Al2Klimov Feb 11, 2026
@Al2Klimov Al2Klimov marked this pull request as ready for review February 11, 2026 14:06
@bobapple
Copy link
Member

bobapple commented Feb 12, 2026

@cla-bot check

@cla-bot

This comment was marked as resolved.

Sign in to view
@cla-bot cla-bot bot added the cla/signed label Feb 12, 2026
@Al2Klimov Al2Klimov requested a review from lippserd February 12, 2026 09:53
lippserd
lippserd requested changes Feb 16, 2026
View reviewed changes
src/RegexMatchValidator.php Outdated
// Multiple isValid() calls must not stack validation messages
$this->clearMessages();

$status = @preg_match($this->pattern, $value);
Copy link
Member

@lippserd lippserd Feb 16, 2026

Choose a reason for hiding this comment

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

Couldn't you use RegexSyntaxValidator for that?

Copy link
Member

@Al2Klimov Al2Klimov Feb 16, 2026

Choose a reason for hiding this comment

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

Sure.

But while I'm on it, let's also question the code LOCATION of this kind of check. Shouldn't the syntax of the pattern, which (the pattern) is constant (relative to user input), be checked in the constructor and throw if necessary?

Copy link
Member

@lippserd lippserd Feb 16, 2026

Choose a reason for hiding this comment

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

Yeah, sounds good!

src/RegexMatchValidator.php Outdated
/**
* Create a RegexMatchValidator
*
* @param string|array $pattern
Copy link
Member

@lippserd lippserd Feb 16, 2026

Choose a reason for hiding this comment

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

Please document array shape.

@Al2Klimov Al2Klimov self-assigned this Feb 16, 2026
@Al2Klimov Al2Klimov removed their assignment Feb 16, 2026
@Al2Klimov Al2Klimov requested a review from lippserd February 16, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Al2Klimov Al2Klimov Al2Klimov left review comments

@lippserd lippserd Awaiting requested review from lippserd

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

cla/signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sukhwinder33445 @lippserd @Al2Klimov @bobapple