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

Add Support for converting ENV to UTF8. Issue #36 #49

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add Support for converting ENV to UTF8. Issue #36 #49

wants to merge 7 commits into from

Conversation

ajbrowe
Copy link

@ajbrowe ajbrowe commented Jan 26, 2018

  • Add Support for converting ENV to UTF8. Issue Handle %ENV #36
  • Add POD for ENV changes
  • Restore %ENV after unimport

This is the first PR on behalf of Adestra for the CPAN PRC for January 2018

There will be a performance hit to using a tied %ENV, however In my experience %ENV is not accessed on such a scale that the performance will be an issue. perhaps I'm wrong.

It is possible to disable the tie with NO-GLOBAL or no utf8::all

ajbrowe and others added 4 commits January 26, 2018 15:43
@Leont
Copy link
Contributor

Leont commented Jan 26, 2018

I think this implementation is both incorrectly scoped (it should only decode/encode when the caller is using the utf8::all pragma), and unnecessarily poorly performing (because it decodes all entries, even when typically few are used).

Decoding (and encoding) on request would automatically solve the latter issue, and will solve the former too if done well.

@ajbrowe
Copy link
Author

ajbrowe commented Jan 26, 2018

Thanks for that, will see what we can do about that.

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.

2 participants