search

insanum / sncli

Simplenote CLI
MIT License
396 stars 34 forks source link

fixed crash when viewing unicode jsons #28

Closed retrography closed 8 years ago

retrography commented 8 years ago

tempfile is opened in binary mode by default unless specified otherwise: https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile

This crashed the JSON view whenever the text contained some unicode characters.

samuelallan72 commented 8 years ago

I would prefer the tempfile continue to be opened in the default binary mode. According to the python docs:

The mode parameter defaults to 'w+b' so that the file created can be read and written without being closed. Binary mode is used so that it behaves consistently on all platforms without regard for the data that is stored. buffering, encoding and newline are interpreted as for open().

Already everything else is converted to bytes before writing to the temporary file, so might as well be consistent.

retrography commented 8 years ago

I can't find any other way to fix the crash... The JSON.dump should supposedly escape the Unicode correctly, so I am not sure where the crash comes from. Maybe we should turn the ascii convertor off when dumping the JSON and see what ensues.

Mahmood Shafeie Zargar Sent with Airmail

On May 30, 2016 at 21:52:58, Samuel Walladge (notifications@github.com) wrote:

I would prefer the tempfile continue to be opened in the default binary mode. According to the python docs https://docs.python.org/3/library/tempfile.html:

The mode parameter defaults to 'w+b' so that the file created can be read and written without being closed. Binary mode is used so that it behaves consistently on all platforms without regard for the data that is stored. buffering, encoding and newline are interpreted as for open().

Already everything else is converted to bytes before writing to the temporary file, so might as well be consistent.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/insanum/sncli/pull/28#issuecomment-222575948, or mute the thread https://github.com/notifications/unsubscribe/AFEChVAo5frb7TfJtA1K7CYWb70DIbE5ks5qG5R6gaJpZM4IqGdF .

samuelallan72 commented 8 years ago

@retrography Yes, json.dump escapes unicode sequences, but doesn't encode the string as bytes (which the binary mode needs). See commit a1ab124 for more info. :)

retrography commented 8 years ago

Great!

Mahmood Shafeie Zargar Sent with Airmail

On May 30, 2016 at 22:07:21, Samuel Walladge (notifications@github.com) wrote:

Closed #28 https://github.com/insanum/sncli/pull/28.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/insanum/sncli/pull/28#event-676447602, or mute the thread https://github.com/notifications/unsubscribe/AFEChWWmtfsoBw3IiZK_fT-kUq5Z9vRcks5qG5fZgaJpZM4IqGdF .