Change notification implementation for windows

[funbiscuit]

Current implementation of notify on windows uses powershell. And it may take even few seconds to start to show it. I would suggest changing code to native implementation.
I tried to do that and I got it working on win xp, win 7 and win 10:

The only drawback I currently face - it doesn't show app icon, just an empty spot.
Another problem is that I rely on object constructor/destructor to clean up notification. So I need to change
pfd::notify(...);
to
pfd::notify notification(...);

And now it doesn't look very logical in terms of naming. And we need to have an object that will be destroyed so we are 100% sure that notification will be deleted if app is closed before timeout.
And we can't just rename notify to something else because notifications don't require object name on other platforms.
@samhocevar what would you suggest? Native implementation is much better than powershell

[funbiscuit]

Currently the only option I see is to create some helper class like WinNotification to handle construction and destruction and in notify function have static instance:

static WinNotification notification;
notification.create(title, message, icon);

If owner approves this approach, I can make another PR.

UPD.
I figured out how to show default app icon (windows default):

But if application have any custom icon, it will not be used. Currently I use:
LoadIcon(NULL, IDI_APPLICATION);
If anyone knows how to get real application icon that would be great!

UPD2.
I figured out how to get actual app icon if any is present, by using
EnumResourceNamesA(nullptr, RT_GROUP_ICON, &func, (LONG_PTR)(nullptr))
to get first icon name, and if any, call
LoadIcon(GetModuleHandle(NULL), "ID" )
if nothing is present, use LoadIcon(NULL, IDI_APPLICATION); to get default icon.

[funbiscuit]

@samhocevar I've added notification part to separate branch for a review:
compare
What would you like to change? Currently it works (no need to change how pfd::notify is called), the only thing to change is to do a check of buffer length where I copy title and message. But this is a minor tweak, I'll add it before creating another pull request.
It's better than current implementation since its faster than calling powershell and it also uses custom application icon if there is one (falls back to default windows app icon). And it also works on windows XP (where you don't have powershell).
In summary, I've added new helper class WinNotification to namespace internal. This helper class is created as static object in notify function. At each notify call this object destroy previous notification and shows a new one. If executable is terminated before notification disappears, object will destroy it in its destructor.
I've tested this code and don't see any problems with it.

[samhocevar]

Thanks, I have merged your branch in master and changed the code using a different pattern so that WinNotification could be reduced to a class with only a destructor. Let me know what you think!

[funbiscuit]

It looks much better! There is a one catch though. RT_GROUP_ICON is different with unicode enabled and disabled. That's why I had ifdefs to generate different callback functions to enumerate icons. You can either rollback those changes or I can make a PR that will compile with both unicode enabled and disabled. For example, mingw by default disables it.
Or if you know how to bypass this in a different way - that'll be good too.

[samhocevar]

I don’t think I understand. The callback now uses LPCTSTR and LPTSTR instead of LPCSTR/LPSTR or LPCWSTR/LPWSTR so that the #ifdef should no longer be necessary. Did I miss something? It seems to work with both Unicode and MultiByte for me (the icon appears in the SysTray in both cases).

[funbiscuit]

You're right I didn't actually test code at that time, only looked around and didn't notice your change to LPCTSTR/LPTSTR. Now I ran it, everything is working, I'm closing this issue.