Skip to content

utf8 identical checking#656

Closed
chenkovsky wants to merge 2 commits into
rails:mainfrom
chenkovsky:master
Closed

utf8 identical checking#656
chenkovsky wants to merge 2 commits into
rails:mainfrom
chenkovsky:master

Conversation

@chenkovsky

Copy link
Copy Markdown

File.binread returns string with ASCII-8BIT encoding. but the rendered result is Utf8 encoding. For non-ascii characters, although the content is same, the comparison will return false. when we invoke migration generate with force option. it will delete the old migration file, and create a new migration file with the same content.

@deivid-rodriguez

Copy link
Copy Markdown
Contributor

@chenkovsky Can we add a test for this change, so it doesn't break again in the future?

@chenkovsky

Copy link
Copy Markdown
Author

@deivid-rodriguez I change the test. add utf8 character to content.

@deivid-rodriguez

Copy link
Copy Markdown
Contributor

Sorry for dropping the ball here.

For the test, I would personally prefer a new separate test that uses UTF-8 content, rather that changing every test to create a file with UTF-8 content.

Other than that this PR looks good to me.

@enigmatt-pl

Copy link
Copy Markdown

Heyy, just a little suggestion from me:

How about introducing a 'wrapper' method for File.binread(destination) and similar that output the earlier mentioned ASCII-8BIT? Ideally, we could force the output of binread to Encoding.default_external which would be more flexible for sure.

Regards

@rafaelfranca

Copy link
Copy Markdown
Member

Closed by #786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants