Deprecate omit_default_port option, add include_default_port option

[mathias]

• Port being added to URL can be forced by using include_default_port option
• Simplifies some of the port addition code
• Break out logic to determine if a default port into its own method
• Tests basic cases of scheme+port combinations

This is a big change in terms of default functionality, and I suspect it will affect the gem users enough that it is worth considering what kind of version bump it'd require.

Additionally, I'd like to have deprecation warnings when the omit_default_port option is used with Excon, but I am not sure which method to add the detection and warning output to. Thoughts?

Fixes #357. Per discussion in #357, this should only add the port to the URL when include_default_port is specified or the port was included in the original URL.

[mathias]

Build broke because I didn't use hashrockets in my hashes; will update later and push.

[geemus]

Someone pointed out that there is at least one case where it should include default port, which is when you specify a URI that explicitly includes it. Maybe we should just use that behavior (instead of a flag). So it would be like:

uri = URI.parse(url)
options = {}
if URL.include?(uri.port) # should be a regex or something instead to enforce position
  options[:port] = uri.port
end

So if you want to opt-in to including, just put it in your url and otherwise don't. What do you think?

[mathias]

That sounds cleaner and both the code and tests should be simpler as a result.I will look into this later today.

[geemus]

Yeah, we just need to make sure and check for the port value in the specific spot in a URL where it would appear, so that it won't be mistaken should it appear in host/path/query or something. My regex-fu has not been tested recently, but was once strong. Just let me know if you have questions or need input on that part of things.

[stale]

This issue has been automatically marked stale due to inactivity. It will be closed if no further activity occurs. Thank you for your contributions.

[geemus]

Pinning. I have every intention of doing this, just holding off for a major version bump, just in case.

[mathias]

Makes sense. 👍🏽

On Mon, Jul 30, 2018 at 9:57 AM Wesley Beary ***@***.***> wrote: Pinning. I have every intention of doing this, just holding off for a major version bump, just in case. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#448 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAANDGLHHei5pegtQCkR3Vjows2Q0deoks5uLx7ngaJpZM4DEjma> .

-- Sent from my supercomputing distraction machine. I blog at http://blog.mattgauger.com