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

Switch to -W versions of registry API #60

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

Conversation

RupW
Copy link

@RupW RupW commented Jun 15, 2018

using UTF-16 strings throughout, to fix encoding issues in the key paths and value strings read and written.

This is my attempt to fix issue #44. For the wide strings I chose not to use the ref-wchar package since it uses iconv - which seems overkill for UTF-8 to UTF-16 conversion - and because it doesn't give us a mechanism to create a UTF-16 string either. We need this when preparing a buffer for RegSetValueEx, so I'd then either have to duplicate the iconv setup that ref-wchar does or just use Buffer.from(..., 'utf16le') anyway. So I've added a simple implementation reg-LPWSTR that uses Buffer's utf16le for the string conversion. It's possible that this won't work with characters outside the unicode BMP, and that's the reason that ref-wchar used iconv: I haven't tested that.

It passes your test suite, and some simple manual tests with non-Latin-1 characters.

@RupW RupW force-pushed the issue-44 branch 2 times, most recently from d4eb405 to 1d5a9a1 Compare June 21, 2018 10:40
RupW added 3 commits June 21, 2018 11:41
using UTF-16 strings throughout, to fix encoding issues in the key paths
and value strings read and written.
Now store saved registry strings as copies of the buffer passed,
not as strings.
@githoniel
Copy link

It work in my project, plz merge this

@ottosson
Copy link

When will this PR be merged?

@phuonghuynh
Copy link

+1 to merge this PR + release to NPM

@phuonghuynh
Copy link

@ritazh could you help to review this? You seem to have permission to merge the PR

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.

4 participants