Skip to content

Conversation

@dipesh23-apt
Copy link

  • Are you fixing a bug or providing a failing unit test to demonstrate a bug?
    Yes, For checking a valid URL

  • How do you reproduce it?
    By updating the regular expression format for the URL, by correcting the misplaced brackets and updating the * character to
    '+'.The * char in regular expression mean the * means "0 or more occurrences of the preceding item" and + char means "1 or
    more occurrences of the preceding item"
    Also there was a misplaced bracket of the form: ( (exp1... ) OR exp2)....
    instead it should have been of the form: ( (exp1...) ) OR (exp2....)
    The * specified after the form (.[a-zA-Z])* means it can have nil (.com/.eu) but updating it to '+' ensures (.com/.in/.xyz) be
    specified in the url to make it a valid url

  • What was the previous behavior?
    Earlier it used to return true for http://abcd or xyz/qw
    which infact is not a valid URL as the domain name is not specified(.com/.eu/.in should be in the URL)
    which in turn makes the 'govalidator package' work similar to the net/url package with a bug.

  • What is the current behavior?
    After modifying it, it returns false for http://abcd or wxe/123 or nmhih
    and in turn returns true for http://abcd.eu or xyz.ab or jkl.com

  • Explain why the changes are necessary
    To check for a valid URL, there are two packages net/url and the govalidator package.But if this package does not return the
    correct output, there is no difference between the faulty package(net/url) and this one.
    So to be at a higher level and simplify the complexities of the user this commit needs to be merged.Also in various projects
    the checking of a valid URL is a crucial part, hence forth moving forward.

@sergeyglazyrindev
Copy link

Hello guys!
I forked this package cause owner disappeared. Hope, he will be back, but it would be easier to merge these changes back if he is back
Link to my repo: create issue there and we'll discuss it.

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