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

Add Twingly::URL#ttld, "true TLD" getter #88

Merged
merged 5 commits into from
Aug 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ url.scheme # => "http"
url.trd # => "www"
url.sld # => "twingly"
url.tld # => "co.uk"
url.ttld # => "uk"
url.domain # => "twingly.co.uk"
url.host # => "www.twingly.co.uk"
url.origin # => "http://www.twingly.co.uk"
Expand All @@ -43,6 +44,7 @@ url.scheme # => "https"
url.trd # => ""
url.sld # => "example"
url.tld # => "com"
url.ttld # => "com"
url.domain # => "example.com"
url.host # => "example.com"
url.origin # => "https://example.com"
Expand Down Expand Up @@ -77,7 +79,9 @@ Note that this isn't a benchmark, we're using [ruby-prof] which will slow things

## Release workflow

* Update the [examples] in this README if needed.
* Update the [examples] in this README if needed, generate the output with

ruby examples/url.rb

* Bump the version in `lib/twingly/version.rb` in a commit, no need to push (the release task does that).

Expand Down
5 changes: 4 additions & 1 deletion examples/url.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "twingly/url"
require "bundler/setup"
require_relative "../lib/twingly/url"

url_as_string = "http://www.twingly.co.uk/search"
url = Twingly::URL.parse(url_as_string)
Expand All @@ -12,6 +13,7 @@
puts "url.trd # => \"#{url.trd}\""
puts "url.sld # => \"#{url.sld}\""
puts "url.tld # => \"#{url.tld}\""
puts "url.ttld # => \"#{url.ttld}\""
puts "url.domain # => \"#{url.domain}\""
puts "url.host # => \"#{url.host}\""
puts "url.origin # => \"#{url.origin}\""
Expand All @@ -29,6 +31,7 @@
puts "url.trd # => \"#{url.trd}\""
puts "url.sld # => \"#{url.sld}\""
puts "url.tld # => \"#{url.tld}\""
puts "url.ttld # => \"#{url.ttld}\""
puts "url.domain # => \"#{url.domain}\""
puts "url.host # => \"#{url.host}\""
puts "url.origin # => \"#{url.origin}\""
Expand Down
8 changes: 8 additions & 0 deletions lib/twingly/url.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ def tld
public_suffix_domain.tld
end

# Many ccTLDs have a second level[1] underneath their ccTLD, use this when
# you don't care about the second level.
#
# [1]: https://en.wikipedia.org/wiki/Second-level_domain
def ttld
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a comment why we call it ttld?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What would the comment say? Wouldn't it be better to call the method true_tld then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jage Like this?

    # Many ccTLDs have a second level[1] underneath their ccTLD, use this when
    # you don't care about the second level.
    # 
    # [1]: https://en.wikipedia.org/wiki/Second-level_domain
    def ttld
      tld.split(".").last
    end

Copy link
Contributor

Choose a reason for hiding this comment

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

What would the comment say? Wouldn't it be better to call the method true_tld then?

Nah, I don't think true is any better. "True" doesn't add any context when to use this.

@jage Like this?

Yes, that's better.

tld.split(".").last
end

def domain
public_suffix_domain.domain
end
Expand Down
5 changes: 5 additions & 0 deletions spec/lib/twingly/url/null_url_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@
it { is_expected.to eq("") }
end

describe "#ttld" do
subject { url.ttld }
it { is_expected.to eq("") }
end

describe "#domain" do
subject { url.domain }
it { is_expected.to eq("") }
Expand Down
11 changes: 11 additions & 0 deletions spec/lib/twingly/url_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,17 @@ def valid_urls
it { is_expected.to eq("co.uk") }
end

describe "#ttld" do
subject { url.ttld }
it { is_expected.to eq("uk") }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe me being a 🐔 but I think I'd like a test for #ttld when we have a normal domain as well, like a simple .com. I am thinking to expose problems in the way we split the #tld.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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


context "when the TLD is just one level" do
let(:test_url){ "http://twingly.com" }

it { is_expected.to eq("com") }
end
end

describe "#domain" do
subject { url.domain }
it { is_expected.to eq("twingly.co.uk") }
Expand Down