Add HTTP#force_response_body_encoding for forcing response body encoding #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows for the ability to opt-in to a method to force the encoding of response bodies. By setting the accessor to a String or Encoding instance, it will force the specified encoding. Setting the value of true will try to detect the encoding of the response body, either using the Content-Type header (assuming it specifies charset) or by scanning for a <meta> tag in the document that specifies the encoding. The default is false in which case no forcing of encoding will be done (same as before the patch). Implements [Feature #2567] Implements [Feature #15517] Co-authored-by: Yui Naruse <naruse@ruby-lang.org>
nurse
reviewed
Apr 1, 2022
| # :nodoc: | ||
| def sniff_encoding(str, encoding=nil) | ||
| # the encoding sniffing algorithm | ||
| # http://www.w3.org/TR/html5/parsing.html#determining-the-character-encoding |
There was a problem hiding this comment.
Suggested change
| # http://www.w3.org/TR/html5/parsing.html#determining-the-character-encoding | |
| # https://html.spec.whatwg.org/multipage/parsing.html#encoding-sniffing-algorithm |
Algorithm also should be updated if needed
nurse
reviewed
Apr 1, 2022
| # the response body encoding. If String or Encoding, uses the specified | ||
| # encoding. | ||
| attr_accessor :force_response_body_encoding | ||
|
|
There was a problem hiding this comment.
The name force_encoding is intended to indicate that the method is a dirty, low-level operation. On the other hand, I think this method is highly recommended. The name should reflect the difference.
Also maybe the given String should be verified when it was set instead of the time when it calls force_encoding.
There was a problem hiding this comment.
I added commits to fix both of these issues. I can update NEWS for ruby after this is merged.
|
Generally looks good. Note that this also should update |
nurse
approved these changes
Apr 8, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows for the ability to opt-in to a method to force the
encoding of response bodies. By setting the accessor to a String
or Encoding instance, it will force the specified encoding.
Setting the value of true will try to detect the encoding of the
response body, either using the Content-Type header (assuming it
specifies charset) or by scanning for a tag in the document
that specifies the encoding. The default is false in which case
no forcing of encoding will be done (same as before the patch).
Implements [Feature #2567]
Implements [Feature #15517]
Co-authored-by: Yui Naruse naruse@ruby-lang.org