thriftrw / thriftrw-node

A thrift binary encoding library using bufrw
MIT License
57 stars 25 forks source link

Comment parse error on specific Japanese #151

Closed KeitaMoromizato closed 4 years ago

KeitaMoromizato commented 7 years ago

Parse error occurred when specific Japanese comment written in the file.

CLAassistant commented 7 years ago

CLA assistant check
All committers have signed the CLA.

KeitaMoromizato commented 7 years ago

Test faled... I think should not support node v0.10.

kriskowal commented 7 years ago

@KeitaMoromizato Sorry for the delay on this. We do need to retain compatibility with Node 0.10. Also, across languages Apache Thrift does not consistently support UTF-8, so this feature limits cross-language compatibility. It would be good to thread an option to explicitly opt-in for UTF-8 encoded IDL files, with documentation of that option that makes it clear that the ThriftRW will not work with Apache Thrift for Python, for example. ThriftRW Go does support UTF-8, so we should for consistency between these projects.

KeitaMoromizato commented 7 years ago

so this feature limits cross-language compatibility.

Thank you for your advice. I changed to use encoding option (It's support to include utf-8 encoded files).

We do need to retain compatibility with Node 0.10.

I see. But test failed in npm install command. How can I fix that?