Skip to content
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

Decode to utf8 #20

Merged
merged 6 commits into from
Nov 2, 2017
Merged

Decode to utf8 #20

merged 6 commits into from
Nov 2, 2017

Conversation

tgallice
Copy link
Member

@tgallice tgallice commented Nov 1, 2017

No description provided.


func TestDecodeToUTF8(t *testing.T) {
for _, testcase := range []struct {
Value string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it need to be a public variable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you squash those two lines, in, out string

return s
}

s, err := charmap.ISO8859_1.NewDecoder().String(s)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we store the decoder in a variable instead of instanciate it every time

s, err := charmap.ISO8859_1.NewDecoder().String(s)

if err != nil {
return ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we log the error ?

"github.com/upfluence/pkg/log"
)

var default_decoder = charmap.ISO8859_1.NewDecoder()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultDecoder

var defaultDecoder = charmap.ISO8859_1.NewDecoder()

func DecodeToUTF8(s string) string {
if s == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it really useful?

{"test", "test"},
{"testé", "testé"},
{"test\xc3", "testÃ"},
{"\xc3", "Ã"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to also test the case where it's a non standard set of bytes

@tgallice tgallice merged this pull request into master Nov 2, 2017
@tgallice tgallice deleted the tg-decode-utf8 branch November 2, 2017 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants