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
Temporary fix for UTF-8 bug in addressable #79
Conversation
Temporary fix for #62. Related to sporkmonger/addressable#224
I think this looks good |
raise | ||
end | ||
|
||
private :new, :internal_parse, :to_addressable_uri, :addressable_display_uri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about splitting this line to multiple lines, so next time we get the nice diff :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. What is the best way of doing that?
private :new,
:internal_parse,
:to_addressable_uri,
:addressable_display_uri
# or
private :new
private :internal_parse
private :to_addressable_uri
private :addressable_display_uri
For the second one we wouldn't have to add a ,
on the last line when adding a symbol, so I think that one looks best :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the second one (easier to remove/add line).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the second one too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sure let's go with the second one :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll create a new gem version after #78 is merged too. |
…tf-8 Temporary fix for UTF-8 bug in addressable
Temporary fix for #62.
Related to sporkmonger/addressable#224
Used the code in #62 (comment) as inspiration, but I had to modify it so we didn't have to call
addressable.display_uri
twice.