PR: Add encoding handling to stderr files in the IPython Console #4369
Conversation
| @@ -32,6 +32,7 @@ | |||
| from spyder.config.gui import get_font, get_shortcut | |||
| from spyder.utils import icon_manager as ima | |||
| from spyder.utils import sourcecode | |||
| from spyder.utils.encoding import getfilesystemencoding | |||
There was a problem hiding this comment.
Let's use get_coding from the same module instead
There was a problem hiding this comment.
Sorry, this is not correct. Let me think better about it.
| try: | ||
| stderr = codecs.open(self.stderr_file, 'r', | ||
| encoding='utf-8').read() | ||
| except(UnicodeDecodeError): |
There was a problem hiding this comment.
There's no need to use parentheses here.
| try: | ||
| encoding = get_coding(open(self.stderr_file, 'rb').read()) | ||
| stderr = codecs.open(self.stderr_file, 'r', | ||
| encoding=encoding).read() |
There was a problem hiding this comment.
Let's change this to
stderr_text = open(self.stderr_file, 'rb').read()
encoding = get_coding(stderr_text)
stderr = to_text_string(stderr_text, encoding)| encoding='cp437').read() | ||
| except UnicodeDecodeError: | ||
| raise EncodingError("Could not read the stderr " | ||
| "file of the kernel while restarting.") |
There was a problem hiding this comment.
and remove this whole block, i.e
except UnicodeDecodeError:
try:
stderr = codecs.open(self.stderr_file, 'r',
encoding='cp437').read()
except UnicodeDecodeError:
raise EncodingError("Could not read the stderr "
"file of the kernel while restarting.")and change it to
except:
stderr = NoneThe thing is cp437 is another particular encoding, and we should have detected it in the previous block. If we're not able to read self.stderr_file, then we need to skip this.
|
Could you add Tests :-p @dalthviz |
| # Encoding Exception | ||
| #----------------------------------------------------------------------------- | ||
| class EncodingError(Exception): | ||
| """Exception when stderr file can't be opened.""" |
There was a problem hiding this comment.
This is unnecessary now, please remove it.
| stderr_text = open(self.stderr_file, 'rb').read() | ||
| encoding = get_coding(stderr_text) | ||
| stderr = to_text_string(stderr_text, encoding) | ||
| except UnicodeDecodeError: |
There was a problem hiding this comment.
Please change this to only except. We want to catch all errors at this point.
| try: | ||
| stderr_text = open(self.stderr_file, 'rb').read() | ||
| encoding = get_coding(stderr_text) | ||
| stderr = to_text_string(stderr_text, encoding) |
There was a problem hiding this comment.
Let's move these three lines to a new method called _read_stderr, so we can test it independently of this one.
| stderr = codecs.open(self.stderr_file, 'r', | ||
| encoding='utf-8').read() | ||
| except UnicodeDecodeError: | ||
| # This handling is needed since the stderr file could be encoded |
| except UnicodeDecodeError: | ||
| # This handling is needed since the stderr file could be encoded | ||
| # in something different to utf-8. In case of fail at least | ||
| # we raise an informative exception. |
There was a problem hiding this comment.
Remove In case of fail ... an informative exception.
|
Yes, that's what's missing. The test needs to go in
|
|
Great work @dalthviz!! I think this is ready, so nothing else to do for me than merge ;-) |
Fixes #4191