Several fixes - Githubissues

swaroopch / edn_format

EDN reader and writer implementation in Python, using PLY (lex, yacc)

https://swaroopch.com/2012/12/24/edn-format-python/

Other

133 stars 31 forks source link

Several fixes #27

Closed ghost closed 9 years ago

ghost commented 9 years ago

Fixes #26 and makes package unicode-aware

Fixed string parsing and escaping
Unicode now consistently used internally
New method dumpu added that returns unicode
dumps method now takes optional encoding arguments, output_encoding (specifies encoding of output string) and string_encoding (specifies encoding of non-unicode strings in object to be serialized), both default to 'utf-8'
loads now takes optional argument input_encoding, defaults to 'utf-8'
New method loadu added that assumes unicode input
Tests updated

swaroopch commented 9 years ago

@exilef Thank you for this PR! May I make a suggestion - can you please use from __future__ import unicode_literals and six library to simplify this PR?

ghost commented 9 years ago

I included unicode_literals, but where do you want to use six?

swaroopch commented 9 years ago

@exilef Yep, it looks like we don't six - patch looks good - just one question though, why do we need both dumpu and dump, both uparse and parse, why not have assume unicode and have one?

Thanks!

petrounias commented 9 years ago

@swaroopch it would be safe to expect that almost all edn encoding and decoding will be in unicode (most likely for interacting with Clojure services), so we probably don't need both dump and dumpu and corresponding parsers. The default case can be unicode, and the user can encode as a special case outside this library.

swaroopch commented 9 years ago

@petrounias I agree. @exilef What do you say?

swaroopch commented 9 years ago

@exilef I already made a new release to PyPI with your PR merged, FYI.

ghost commented 9 years ago

Sure, I do not have strong feelings about this. Change to your liking!

ghost commented 9 years ago

Great, thank you!

On Mon, May 25, 2015 at 9:26 PM, Swaroop C H notifications@github.com wrote:

@exilef https://github.com/exilef I already made a new release to PyPI with your PR merged, FYI.

— Reply to this email directly or view it on GitHub https://github.com/swaroopch/edn_format/pull/27#issuecomment-105293266.