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

feature: support most of encodings in options parameter #129

Closed

Conversation

akaguny
Copy link
Contributor

@akaguny akaguny commented Sep 24, 2020

No description provided.

@coveralls
Copy link

coveralls commented Sep 24, 2020

Coverage Status

Coverage decreased (-1.4%) to 98.582% when pulling 4f7f00f on akaguny:feature/support-most-of-encodings into ebb3c7d on adamreisnz:master.

@adamreisnz
Copy link
Owner

I am a bit concerned about introducing more dependencies and the extra call to decode the contents first.

Not sure if this is something this library should concern itself with. Is it not possible to convert the file encoding prior to loading them into this package?

Would be good to get some wider feedback on this one, as well as detailed reasoning as to why this is a problem and how this solution solves it.

@akaguny
Copy link
Contributor Author

akaguny commented Sep 29, 2020

Would be good to get some wider feedback on this one, as well as detailed reasoning as to why this is a problem and how this solution solves it.

support more that nodejs support native (ex: windows-1251)

Not sure if this is something this library should concern itself with. Is it not possible to convert the file encoding prior to loading them into this package?

Have any ideas?

I am a bit concerned about introducing more dependencies and the extra call to decode the contents first.

Yes, but I don't know about another solution to supply this feature

@adamreisnz
Copy link
Owner

Closing as not wanting to introduce additional 3rd party dependencies to this package, and considering content encoding/decoding to be out of scope for this package.

@adamreisnz adamreisnz closed this Oct 27, 2023
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