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 option to specify custom TLDs #2

Merged
merged 7 commits into from
Jul 2, 2013

Conversation

JeanMertz
Copy link
Contributor

This PR makes it possible to specify custom TLDs, which don't officially exist, but can be used in private environments.

One use case would be intranets with specific host files, allowing for company TLDs (like intranet.disney). In my case, the use-case would be a development environment where I use the dev TLD locally.

You can specify custom TLDs like so:

Addressable::URI.custom_tlds = {
  'dev' => {}
}

There was one unrelated spec failing (even on master). I left it untouched. The new specs all pass.

Failing spec is:

  1) Addressabler should support adding nested values to the query
     Failure/Error: uri.query_hash[:foo] = {:bar => "baz", :sommat => [:else, 1, true, false]}
     TypeError:
       Can't convert Hash into String.

@flipsasser
Copy link
Owner

This looks awesome! Great feature. Thanks for adding it/

One quick note - I can't merge the gemspec commit (ac01353). I build the gemspec from the Gemfile, and not vice-versa, so I need to keep the behavior the way it was.

So I can either rebase without the gemspec bits, or you can open a new pull request. I'm kind of swamped at work so the soonest I can get to this would be some time next week. If you're not in a hurry, I'm happy to do it myself.

Thanks again!

Flip

PS. I've never seen the Markdown link syntax you added to the README. What's the benefit to that instead of explicitly specifying the links inline?

@JeanMertz
Copy link
Contributor Author

I'll revert the gemspec change somewhere this week. The reason I changed this was that it made setting up the development environment a lot easier (bundle install, done). Unless there is a way to install all required gems from a gemspec, that I am not aware of?

As for the Markdown formatting. Markdown was created for formatted text to be readable in environments where formatting is not available. As such, the style I use moves links into a separate footnotes part of the document, removing irrelevant text out of the different contexts.

It's just something I've learned to do while reading up on the original idea behind Markdown, and writing lots of documentation in Markdown ever since.

I can revert it if you want, it doesn't affect the end-result either way.

@flipsasser
Copy link
Owner

bundle install should work fine without sourcing the gemspec. I use Jeweler to package the app, which takes my test and production requirements and adds them to the gemspec automatically, hence the preference for gem definitions in the Gemfile.

I found Markdown to be more readable with the links inline (personally), but the 80 character wrapping is certainly welcome.

@JeanMertz
Copy link
Contributor Author

I've made the requested changes.

By the way, since the original creator of Markdown can explain it better than
I can, here is a snippet from the Markdown documentation:

Here’s an example of reference links in action:

I get 10 times more traffic from [Google] [1] than from
[Yahoo] [2] or [MSN] [3].

  [1]: http://google.com/        "Google"
  [2]: http://search.yahoo.com/  "Yahoo Search"
  [3]: http://search.msn.com/    "MSN Search"

Using the implicit link name shortcut, you could instead write:

I get 10 times more traffic from [Google][] than from
[Yahoo][] or [MSN][].

  [google]: http://google.com/        "Google"
  [yahoo]:  http://search.yahoo.com/  "Yahoo Search"
  [msn]:    http://search.msn.com/    "MSN Search"

Both of the above examples will produce the following HTML output:

<p>I get 10 times more traffic from <a href="http://google.com/"
title="Google">Google</a> than from
<a href="http://search.yahoo.com/" title="Yahoo Search">Yahoo</a>
or <a href="http://search.msn.com/" title="MSN Search">MSN</a>.</p>

For comparison, here is the same paragraph written using Markdown’s inline
link style:

I get 10 times more traffic from [Google](http://google.com/ "Google")
than from [Yahoo](http://search.yahoo.com/ "Yahoo Search") or
[MSN](http://search.msn.com/ "MSN Search").

The point of reference-style links is not that they’re easier to write. The
point is that with reference-style links, your document source is vastly more
readable. Compare the above examples: using reference-style links, the
paragraph itself is only 81 characters long; with inline-style links, it’s 176
characters; and as raw HTML, it’s 234 characters. In the raw HTML, there’s
more markup than there is text.

With Markdown’s reference-style links, a source document much more closely
resembles the final output, as rendered in a browser. By allowing you to move
the markup-related metadata out of the paragraph, you can add links without
interrupting the narrative flow of your prose
.

@JeanMertz
Copy link
Contributor Author

@flipsasser any chance of this getting merged in?

@flipsasser
Copy link
Owner

Oops, I forgot about this. Apologies, and absolutely. But it may be a few days - my wife just went into labor.

Huh. This is a weird place to share that. Anyway! I'll merge it when we get back.

On Jun 15, 2013, at 7:12 AM, Jean Mertz notifications@github.com wrote:

@flipsasser any chance of this getting merged in?


Reply to this email directly or view it on GitHub.

@JeanMertz
Copy link
Contributor Author

Weird place or not, best of luck to you and your wife! Let me be the first GitHubber to congratulate you two 👶

@flipsasser flipsasser merged commit ff5d4f8 into flipsasser:master Jul 2, 2013
@flipsasser
Copy link
Owner

Okay, wow, so it turns out having a kid is cuh-razy time consuming. Merged and pushed to Rubygems. Thanks for contributing and being so patient!

In hilarious news, I went ahead and used that implicit link formatting in the changelog with a link to this pull request. The more you know! Thanks again.

Flip

@JeanMertz JeanMertz deleted the custom_tlds branch July 3, 2013 06:37
@JeanMertz
Copy link
Contributor Author

Awesome, thank you. And again, good luck with the future. May you experience many sleepless nights 😪 😆

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