Fixed RefreshShellIcons.nsh

[daemondevin]

The post check was incorrect. Used the system plugin for better performance and reliability.

[daemondevin]

@GordCaswell, I'm so confused by these commits, pull, merge requests. Forgive me if I'm not doing this correctly. Lol.

I couldn't tell you what exactly was wrong with it because whatever issue I had with this segment I had over a year ago and what I did here was a bit more reliable.

I should warn you before I continue further that a lot of my enhancements to the version of PAL I am working on was to reduce the overhead created by the current code base. For instance, and this is just an example, I removed checking for Windows XP x64 because the benefits aren't worth the workload. I figured that by now.. the majority of the end-users are tech-savvy enough to know Windows XP can be dangerous and have already upgraded past Windows Vista anyway. However, just in case I did need to check for this edge-case scenario, I've added something similar to following:

${SegmentFile}
${Segment.onInit}
    !ifmacrodef OS
        !insertmacro OS
    !endif
!macroend

Now I can simply declare !macro OS in the custom.nsh file for the ability to check for XP.

I know, unrelated but I thought I should explain the reason why you see me using certain techniques over other practices. Like using IfFileExists instead of ${If} ${FileExists} which just creates more overhead; well, in the long run anyway. Lol. =P

[GordCaswell]

@demondevin The concern I have with what you've implemented for RefreshShellIcons, is that you've re-implemented a feature that is already available within NSIS itself - why rewrite it in our code, when we can call the already-existing function?

EDIT - Regarding the rest of your explanation: The rational makes sense.

[daemondevin]

I always figured calling the Windows API or calling a DLL function directly was always more reliable.

Besides, sending SHCNE_ASSOCCHANGED is a bit like hitting Explorer over the head with a frying pan. It causes the whole desktop to refresh without fail; or at least for me anyway.

[GordCaswell]

That's what's happening already. Here's the applicable chunks of FileFunc.nsh, called by ${RefreshShellIcons}:

!define RefreshShellIcons `!insertmacro RefreshShellIconsCall`

!macro RefreshShellIconsCall
    !verbose push
    !verbose ${_FILEFUNC_VERBOSE}
    ${CallArtificialFunction} RefreshShellIcons_
    !verbose pop
!macroend

!macro RefreshShellIcons_
    !verbose push
    !verbose ${_FILEFUNC_VERBOSE}

    System::Call 'shell32::SHChangeNotify(i 0x08000000, i 0, i 0, i 0)'

    !verbose pop
!macroend

[daemondevin]

Would you look at that... Now that's cool. Lol.

Well, there is still one good thing in favor of my proposal and that's the avoidance of all the code having to be expanded (or copied and pasted) during compile time.

Not so important in the bigger picture. =P

On a side note, I cannot seem to figure out how to open a new pull request for the RunAsAdmin segment. This whole thing is just lost on me. =\

[3D1T0R]

A pull request (as they're called on GitHub; merge requests on GitLab) references a branch (actually two, a "base branch" and a "head branch", but in this case the base branch is "master"), so that the pull request can be updated to include new changes by making new commits to the same branch (or by rebasing the head branch if necessary), thus you have to have a separate branch for each pull request; it thus follows that branches should be named for the feature they're adding or the bug they're fixing as opposed to the person who's using them.