Skip to content

Ophilon issue4 #5

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ophilon
Copy link

@ophilon ophilon commented Nov 29, 2021

hi Talysson and the team
Sorry to bother with this issue - hopefully, subj. PR able to fix

links_regex fix - Top Level Domains strict check and links delimiters part
expression changed into multi-line free-spacing mode with comments
@talyssonoc
Copy link
Owner

talyssonoc commented Nov 29, 2021

Hello, @ophilon. What are those listed options? Are those every TLDs that exist? If so, will we need a new PR every time that list is updated? This library does not have the intent to be a validator but a way to extract data from strings, I don't think it's safe to implement the domain regex so strictly.

@ophilon
Copy link
Author

ophilon commented Nov 29, 2021

thanks for the comment Talysson. My name is Oleg, I'm from Belarus.

  1. I think the IANA.ORG did this big update of root zones 1st time since initial ~120 domains and that's the standard for long time. It seems you created this gem after this python one -
    commonregex python lib.
    You can check - python lib already uses more root zone names then original TLD and 2 letters country.
    I hope that this list of ~1400 will serve many many years. It's limited to Latin only names - see my comment in links_regex
  2. While rather long and complete subj. TLD list works effectively, all my checks run in milliseconds:
[ophil@philon-op test]$ ruby test_commonregex.rb 
Run options: --seed 48136
# Running:
.............
Finished in 0.002584s, 5031.2499 runs/s, 5031.2499 assertions/s.
13 runs, 13 assertions, 0 failures, 0 errors, 0 skips
  1. Me also changed expression into multi-line free-spacing mode - for me it looks and reads much better then old one.
    Please, note my comments - I explained every part of regex for clarity and easy support in the future.

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