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

Fix issue with LPTSTR #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hovancik
Copy link

Fix issue with newer electron and node #6 .

Seems to work fine for me, but not C dev here 😕

@@ -16,15 +17,15 @@ Napi::Boolean Method(const Napi::CallbackInfo& info) {

#ifdef _WIN32
HKEY hKey;
LPTSTR lpValueName = "NOC_GLOBAL_SETTING_TOASTS_ENABLED";
std::string lpValueName = "NOC_GLOBAL_SETTING_TOASTS_ENABLED";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ought to be

- LPTSTR lpValueName = "NOC_GLOBAL_SETTING_TOASTS_ENABLED";
+ LPCTSTR lpValueName = TEXT("NOC_GLOBAL_SETTING_TOASTS_ENABLED");

but do not worry. It's not gonna work even if you fix it:

Since a recent major update to Windows 10, "Quiet Hours" has changed to "Focus Assist" - and it seems to work differently, so previous answers don't apply any more.

A replacement https://github.com/bitdisaster/windows-focus-assist is available, but:

This native node module uses an unsupported/undocumented API to get the status of the Focus Assist. …

⚠ The API used by windows-focus-assist can change/break at any time in the future. ⚠

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point is to make this work for people who still use OS version that supports this

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Focus assist was introduced back in 2018. Multiple OS upgrades have been released since then. Not only that, Windows 10 will reach EoL the next year.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh interesting, I didn't realize this wasn't a thing in Windows anymore. Sounds like we should just remove this check entirely, and let the OS mute the notifications if it wants to? Thank you!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it can still be enabled? But I am not Windows user so I don't know: https://windowsreport.com/enable-or-disable-quiet-hours/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it can't be.
turn-off-quiet-hours-930x620
‘Requirements: Windows Server 2012 R2, Windows 8.1 or Windows 8.1 RT only’ is clearly visible in upper-left corner.

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.

3 participants