support 'domain' and 'registrar' properties on whois.dns.be.rb (now with tests) #216

Merged
merged 3 commits into from Mar 30, 2013

Conversation

Projects
None yet
2 participants

As before, supporting the "domain" and "registrar" properties, plus tests added -- I piggybacked them on the status_registered.expected tests since it seems to contain a general list of other properties it tests as well.

lib/whois/record/parser/whois.dns.be.rb
+
+ property_supported :registrar do
+ name = content_for_scanner.slice(/Registrar:\s+Name:(.+?)\s*Website:(.+?)\n/, 1)
+ url = content_for_scanner.slice(/Registrar:\s+Name:(.+?)\s*Website:(.+?)\n/, 2)
@weppos

weppos Mar 29, 2013

Owner

Since the match is the same, I would suggest something like

if (match = content_for_scanner.match(/Registrar:\s+Name:(.+?)\s*Website:(.+?)\n/))
  name, url = match.to_a[1..2]
  Record::Registrar.new(name: name.strip, url: url.strip)
end
Owner

weppos commented Mar 29, 2013

Hey @chuckadams, this is great! Can I ask you one more favor? Please also make sure to add the tests for status_available.expected so that we are sure the parser won't crash in case the domain is available.

Owner

weppos commented Mar 29, 2013

I'll leave this PR open so that you can push the changes to your branch and it will update automatically.

That certainly is a better regex match -- I'm something of a novice in ruby (perl guy mostly) so I wasn't quite up to speed on ruby's matching API, and cargo-culted it from other parsers. :) I made the matcher change and added the extra tests.

@ghost ghost assigned weppos Mar 30, 2013

weppos added a commit that referenced this pull request Mar 30, 2013

Merge pull request #216 from chuckadams/master
support 'domain' and 'registrar' properties on whois.dns.be.rb (now with tests)

@weppos weppos merged commit 8ac343d into weppos:master Mar 30, 2013

1 check passed

default The Travis build passed
Details
Owner

weppos commented Mar 30, 2013

Merged, thank you!

weppos added a commit that referenced this pull request Mar 30, 2013

weppos added a commit that referenced this pull request Mar 30, 2013

Owner

weppos commented Mar 30, 2013

@chuckadams I committed a small change, however the patch was great! Thank you for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment