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 assertion for URLs #199

Closed
wants to merge 2 commits into from
Closed

Conversation

tomaszhanc
Copy link

@tomaszhanc tomaszhanc commented Jun 6, 2020

It validates the URL using filter_var. The validation is not so strict to accept only http or https protocols but it might be good. It's definitely lighter than using that monster regex I initially used in that PR.

@zerkms
Copy link
Contributor

zerkms commented Jun 26, 2020

I don't generally agree that something as this should be added to the library, but I'm not the decider.

What I want to note though is: I believe we should accept mixed instead @param string $value. Relevant: #197 (comment)

@tomaszhanc tomaszhanc force-pushed the feature/url branch 3 times, most recently from e0886fa to dfdcb05 Compare July 25, 2020 14:03
@tomaszhanc
Copy link
Author

I changed the type to mixed as adjusted the code after rebasing with the master branch. I'm still not sure if there is something I could do to improve it. If there is, please let me know. If it's not and it's not going to be merged than let's close it, no hurt feelings :)

@tomaszhanc tomaszhanc closed this Aug 5, 2020
@tomaszhanc tomaszhanc reopened this Aug 5, 2020
@tomaszhanc tomaszhanc closed this Aug 5, 2020
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