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

[WIP] Handling Encoding Error. #1496

Closed

Conversation

anirudnits
Copy link
Collaborator

@anirudnits anirudnits commented Sep 24, 2020

Its failing for the windows builds with a UnicodeEncodeError, please have a look and also review the other changes that I have made. I don't think that this is complete but still opening this PR to get suggestions and correct it accordingly :)

Thanks

@anirudnits anirudnits changed the title [WIP] Encoding Error. [WIP] Handling Encoding Error. Sep 24, 2020
ask_to_apply
and not ask_whether_to_apply_changes_to_file(
str(source_file.path)
try:
Copy link
Member

Choose a reason for hiding this comment

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

I think the try except should be elsewhere (probably on the call of this function, or , better on encoding, _ = tokenize.detect_encoding(buffer.readline) in io.py so it's more localized and the change affect less code than that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Pierre-Sassoulas I have also added a try clause on encoding, _ = tokenize.detect_encoding in isort/io.py. The reason why I also put a try catch block here as well as isort/main.py is that I'm trying to throw a warning for each file with invalid encoding(if verbose flag is set) and not an error even if at least one file has valid encoding. The error is only for the case when all the files processed have invalid encodings. So I needed a way to propagate this invalid encodings exceptions to isort/main.py where I am finally deciding if to throw an error or not. isort/io.py and isort/api.py are only called per file so I couldn't come up with a way to do it there.

@timothycrosley
Copy link
Member

Closing this, as I've used your work (copying and pasting most of it) to create this PR which I've merged: #1521 (please let me know if you see anything wrong with it, or that I missed!)

I believe the reason for the windows error, was just the default encoding being used when saving the example file with an encoding error to disk. Specifying the encoding of that file as utf8 and using write_text fixed it for me.

Thanks!

~Timothy

@anirudnits
Copy link
Collaborator Author

@timothycrosley nothing wrong or missed from my side :)

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.

None yet

3 participants