-
Notifications
You must be signed in to change notification settings - Fork 268
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
Consider changing default heuristic parse to https #356
Comments
Hey @sporkmonger is there any update on this? Happy to grab it if there's no reason as to why not default |
It would be a breaking change. So a new major release. I wonder if it is worth it. Maybe it should be opt-in at first? And it needs to be possible to go back to the old behavior. |
@dentarg ah yeah, you're right. Can you explain what you mean by opt-in? Isn't it opt-in today already? If I were to pass in I don't see why we shouldn't prefer |
Ah right, you can pass hints to the heuristic parse method. I forgot about that. addressable/lib/addressable/uri.rb Line 188 in 4229164
So it is only a matter of changing that. (Which we should only do in a new major version IMHO) |
Re: "is it worth it?" This issue has been open for five years and there hasn't been a lot of user requests to change this default. Maybe because it is so easy to adjust it? However, if we do change it, everyone that relies on the current default behavior has to take action. Addressable doesn't change a lot these days and is mostly considered very stable. If we would produce a major version with more major changes it would probably be worth changing this. |
Currently
heuristic_parse("example.com")
returns"http://example.com/"
. At some point I think it'd be better to return"https://example.com/"
but we may not be there just yet.The text was updated successfully, but these errors were encountered: