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
x/text/encoding: ianaindex.Index.Encoding returns nil for valid encoding names #19421
Comments
cc @mpvl I just ran into this issue as well. |
Me too. The 55 implemented encodings are more than enough for me. The problem is that no function is supposed to fail with a (It *is* a failure, because I think returning The best solution I can think of, is to return --- a/encoding/ianaindex/ianaindex.go
+++ b/encoding/ianaindex/ianaindex.go
@@ -79,7 +79,11 @@ func (x *Index) Encoding(name string) (encoding.Encoding, error) {
return nil, errInvalidName
}
}
- return x.enc[i], nil
+ enc := x.enc[i]
+ if enc == nil {
+ return nil, errUnsupported
+ }
+ return enc, nil
}
// Name reports the canonical name of the given Encoding. It will return an While we are waiting for the right people to look into this bug, I propose to add a This bug is already two years old and users are going to get unexpected panics (like I did), just because the information is not at hand. |
Opened a pull request to fix this issue: golang/text#10 |
@emersion, thanks. But to treat ASCII as I would prefer to either do nothing, or implement a "correct" ASCII Encoding. |
That's true. I updated the PR to make the ASCII encoding:
|
@emersion, thanks for the update. Unlike Decoders, it seems that an Encoder is supposed to return a real Existing Encoders return I haven't looked into it, but an alternative could be to add ASCII as a |
Ah, thanks, I missed that. Updated again to return a I considered adding ASCII as a charmap but decided against because the script generating tables fetches an ICU table from a remote repository. |
Change https://golang.org/cl/212077 mentions this issue: |
I think it's a good idea to give users access to the ascii encoding. If a user thinks he needs an ascii Note that
SGTM (to be tested with |
Are there any encodings compatible with Unicode that are returning |
@makeworld-the-better-one, by "compatible with Unicode" I assume you actually mean "a subset of utf-8", otherwise you will have to explain better. But you may want to treat text labeled us-ascii as windows-1252, because in case of mislabeling there is usually a higher chance for it to be windows-1252 or latin1 than utf-8 (and in case of correct labeling it makes no difference). It depends on your use case. |
Hmm, thanks. But it seems that in general I should just not be displaying text that has a |
Index.Encoding returns a nil Encoding in case the charset is valid but unsupported by the library. Document this behavior. Because of this, US-ASCII is seen as unsupported. Register it as a regular encoding. The decoder replaces non-ASCII bytes with the unicode replacement character. The encoder returns a RepertoireError when a non-ASCII rune is encountered. Fixes golang/go#19421 Change-Id: I4c24ba2114a5012be88488e63aa6e57df955eb96 GitHub-Last-Rev: 418ee6d GitHub-Pull-Request: golang#10 Reviewed-on: https://go-review.googlesource.com/c/text/+/212077 Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Marcel van Lohuizen <mpvl@golang.org> Run-TryBot: Daniel Martí <mvdan@mvdan.cc> TryBot-Result: Gobot Gobot <gobot@golang.org>
ianaindex.MIME.Encoding("us-ascii")
for example returns (nil, nil).This is quite surprising for me because the documentation does not say that it may return nil for valid names.
Would you please consider updating the documentation or changing it to return
encoding.Nop
?Thank you!
What did you do?
What did you expect to see?
non-nil
Encoding
object and nil error.What did you see instead?
nil, nil
The text was updated successfully, but these errors were encountered: