Refactoring and cleanup

Close #21

Author: GurliGebis

.gitignore

@@ -4,3 +4,4 @@ Packaging/NppShell.msix
 *.user
 Debug
 Release
+*.user

Installer.cpp

@@ -4,6 +4,7 @@
 #include "ClassicEditWithNppExplorerCommandHandler.h"
 #include "PathHelper.h"
 #include "AclHelper.h"
+#include "RegistryKey.h"
 
 #define GUID_STRING_SIZE 40
 
@@ -14,6 +15,7 @@ using namespace winrt::Windows::Management::Deployment;
 
 using namespace NppShell::Helpers;
 using namespace NppShell::Installer;
+using namespace NppShell::Registry;
 
 extern HMODULE thisModule;
 
@@ -30,37 +32,12 @@ const wstring ShellExtensionKey = L"Software\\Classes\\*\\shellex\\ContextMenuHa
 
 bool IsWindows11Installation()
 {
-    wstring keyName = L"SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion";
-    wstring valueName = L"CurrentBuildNumber";
+    RegistryKey registryKey(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion");
+    wstring buildNumberString = registryKey.GetStringValue(L"CurrentBuildNumber");
 
-    bool result = false;
+    const int buildNumber = stoi(buildNumberString);
 
-    HKEY hkey;
-    HRESULT hResult = RegOpenKeyEx(HKEY_LOCAL_MACHINE, keyName.c_str(), 0, KEY_READ, &hkey);
-
-    if (hResult == ERROR_SUCCESS)
-    {
-        DWORD cbData = 0;
-        hResult = RegGetValue(hkey, nullptr, valueName.c_str(), RRF_RT_REG_SZ, nullptr, nullptr, &cbData);
-
-        if (hResult == ERROR_SUCCESS)
-        {
-            vector<BYTE> buffer(cbData + 1);
-
-            cbData = (DWORD)buffer.size();
-            hResult = RegGetValue(hkey, nullptr, valueName.c_str(), RRF_RT_REG_SZ, nullptr, &buffer[0], &cbData);
-
-            wstring buildNumberString(reinterpret_cast<wchar_t*>(&buffer[0]));
-
-            const int buildNumber = stoi(buildNumberString);
-
-            result = buildNumber >= FirstWindows11BuildNumber;
-        }
-    }
-
-    RegCloseKey(hkey);
-
-    return result;
+    return buildNumber >= FirstWindows11BuildNumber;
 }
 
 wstring GetCLSIDString()
@@ -90,120 +67,63 @@ wstring GetCLSIDString()
     return guid;
 }
 
-LRESULT RemoveRegistryKeyIfFound(HKEY hive, wstring keyName)
+void inline CleanupRegistry(const wstring& guid)
 {
-    HKEY hkey;
-    const LRESULT lResult = RegOpenKeyEx(hive, keyName.c_str(), 0, KEY_READ, &hkey);
-
-    if (lResult == ERROR_FILE_NOT_FOUND)
+    // First we remove the shell key if it exists.
+    if (RegistryKey::KeyExists(HKEY_LOCAL_MACHINE, ShellKey))
     {
-        // Does not exist, so nothing to remove
-        return ERROR_SUCCESS;
+        RegistryKey registryKey(HKEY_LOCAL_MACHINE, ShellKey, KEY_READ | KEY_WRITE);
+        registryKey.DeleteKey();
     }
 
-    if (lResult != ERROR_SUCCESS)
+    // Then we remove the shell extension key if it exists.
+    if (RegistryKey::KeyExists(HKEY_LOCAL_MACHINE, ShellExtensionKey))
     {
-        return lResult;
+        RegistryKey registryKey(HKEY_LOCAL_MACHINE, ShellExtensionKey, KEY_READ | KEY_WRITE);
+        registryKey.DeleteKey();
     }
 
-    RegCloseKey(hkey);
-
-    return RegDeleteTree(hive, keyName.c_str());
-}
-
-LRESULT CreateRegistryKey(const HKEY hive, const wstring& key, const wstring& name, const wstring& value)
-{
-    HKEY regKey;
-    LRESULT lResult = RegCreateKeyEx(hive, key.data(), 0, NULL, REG_OPTION_NON_VOLATILE, KEY_SET_VALUE, NULL, &regKey, NULL);
-
-    if (lResult != ERROR_SUCCESS)
+    // Then we remove the Notepad++_file key if it exists.
+    if (RegistryKey::KeyExists(HKEY_LOCAL_MACHINE, L"Notepad++_file\\shellex"))
     {
-        return lResult;
+        RegistryKey registryKey(HKEY_LOCAL_MACHINE, L"Notepad++_file\\shellex", KEY_READ | KEY_WRITE);
+        registryKey.DeleteKey();
     }
 
-    lResult = RegSetKeyValue(regKey, NULL, name.empty() ? NULL : name.data(), REG_SZ, value.data(), static_cast<DWORD>(value.length() * sizeof TCHAR));
-    
-    RegCloseKey(regKey);
-
-    return lResult;
-}
-
-LRESULT CleanupRegistry(const wstring& guid)
-{
-    constexpr int bufferSize = MAX_PATH + GUID_STRING_SIZE;
-    WCHAR buffer[bufferSize];
-    const auto arraySize = bufferSize * sizeof(WCHAR);
-
-    LRESULT result;
-
-    result = RemoveRegistryKeyIfFound(HKEY_LOCAL_MACHINE, ShellKey);
-
-    if (result != ERROR_SUCCESS)
+    // Finally we remove the CLSID key if it exists.
+    if (RegistryKey::KeyExists(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid))
     {
-        return result;
-    }
-
-    result = RemoveRegistryKeyIfFound(HKEY_LOCAL_MACHINE, ShellExtensionKey);
-
-    if (result != ERROR_SUCCESS)
-    {
-        return result;
-    }
-
-    StringCbPrintf(buffer, arraySize, L"Notepad++_file\\shellex");
-    result = RemoveRegistryKeyIfFound(HKEY_CLASSES_ROOT, buffer);
-    if (result != ERROR_SUCCESS)
-    {
-        return result;
+        RegistryKey registryKey(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid, KEY_READ | KEY_WRITE);
+        registryKey.DeleteKey();
     }
+}
 
-    StringCbPrintf(buffer, arraySize, L"Software\\Classes\\CLSID\\%s", guid.c_str());
-    result = RemoveRegistryKeyIfFound(HKEY_LOCAL_MACHINE, buffer);
-    if (result != ERROR_SUCCESS)
+void inline CleanupHack()
+{
+    // First we test if the key even exists.
+    if (!RegistryKey::KeyExists(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Classes\\*\\shell\\pintohome"))
     {
-        return result;
+        return;
     }
 
-    return ERROR_SUCCESS;
-}
-
-LRESULT CleanupHack()
-{
-    wstring keyName = L"SOFTWARE\\Classes\\*\\shell\\pintohome";
+    // If it does, we open it and check if the value exists.
     wstring valueName = L"MUIVerb";
+    RegistryKey registryKey(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Classes\\*\\shell\\pintohome", KEY_READ | KEY_WRITE);
 
-    LRESULT result = 0;
-    bool found = false;
-
-    HKEY hkey;
-    HRESULT hResult = RegOpenKeyEx(HKEY_LOCAL_MACHINE, keyName.c_str(), 0, KEY_READ, &hkey);
-
-    if (hResult == ERROR_SUCCESS)
+    if (!registryKey.ValueExists(valueName))
     {
-        DWORD cbData = 0;
-        hResult = RegGetValue(hkey, nullptr, valueName.c_str(), RRF_RT_REG_SZ, nullptr, nullptr, &cbData);
-
-        if (hResult == ERROR_SUCCESS)
-        {
-            vector<BYTE> buffer(cbData + 1);
-
-            cbData = (DWORD)buffer.size();
-            hResult = RegGetValue(hkey, nullptr, valueName.c_str(), RRF_RT_REG_SZ, nullptr, &buffer[0], &cbData);
-
-            wstring valueString(reinterpret_cast<wchar_t*>(&buffer[0]));
-
-            found = valueString.find(L"Notepad++") != wstring::npos;
-        }
+        return;
     }
 
-    RegCloseKey(hkey);
+    // Then we get the value and see if it contains the text "Notepad++"
+    wstring currentValue = registryKey.GetStringValue(valueName);
+    bool found = currentValue.find(L"Notepad++") != wstring::npos;
 
+    // If we found the text, we delete the entire key.
     if (found)
     {
-        result = RegDeleteTree(HKEY_LOCAL_MACHINE, keyName.c_str());
+        registryKey.DeleteKey();
     }
-
-    return result;
 }
 
 HRESULT MoveFileToTempAndScheduleDeletion(const wstring& filePath, bool moveToTempDirectory)
@@ -215,7 +135,10 @@ HRESULT MoveFileToTempAndScheduleDeletion(const wstring& filePath, bool moveToTe
 
     if (moveToTempDirectory)
     {
+        // First we get the path to the temporary directory.
         GetTempPath(MAX_PATH, &tempPath[0]);
+
+        // Then we get a temporary filename in the temporary directory.
         GetTempFileName(tempPath.c_str(), L"tempFileName", 0, &tempFileName[0]);
 
         // Move the file into the temp directory - it can be moved even when it is loaded into memory and locked.
@@ -320,15 +243,22 @@ HRESULT NppShell::Installer::UnregisterSparsePackage()
 
 HRESULT NppShell::Installer::RegisterOldContextMenu()
 {
-    const wstring installationPath = GetContextMenuPath();
+    const wstring contextMenuFullName = GetContextMenuFullName();
     const wstring guid = GetCLSIDString();
 
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, ShellKey, L"ExplorerCommandHandler", guid.c_str());
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, ShellKey, L"", L"Notepad++ Context menu");
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, ShellKey, L"NeverDefault", L"");
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid, L"", L"notepad++");
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid + L"\\InProcServer32", L"", installationPath + L"\\NppShell.dll");
-    CreateRegistryKey(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid + L"\\InProcServer32", L"ThreadingModel", L"Apartment");
+    // First we set the shell extension values.
+    RegistryKey regKeyExtension(HKEY_LOCAL_MACHINE, ShellKey, KEY_READ | KEY_WRITE, true);
+    regKeyExtension.SetStringValue(L"", L"Notepad++ Context menu");
+    regKeyExtension.SetStringValue(L"ExplorerCommandHandler", guid);
+    regKeyExtension.SetStringValue(L"NeverDefault", L"");
+
+    // Then we create the CLSID for the handler with it's values.
+    RegistryKey regKeyClsid(HKEY_LOCAL_MACHINE, L"Software\\Classes\\CLSID\\" + guid, KEY_READ | KEY_WRITE, true);
+    regKeyClsid.SetStringValue(L"", L"notepad++");
+
+    RegistryKey regKeyInProc = regKeyClsid.GetSubKey(L"InProcServer32", true);
+    regKeyInProc.SetStringValue(L"", contextMenuFullName);
+    regKeyInProc.SetStringValue(L"ThreadingModel", L"Apartment");
 
     return S_OK;
 }
@@ -356,8 +286,11 @@ HRESULT NppShell::Installer::Install()
     {
         // We need to unregister the old menu on Windows 11 to prevent double entries in the old menu.
         UnregisterOldContextMenu();
+
+        // Since we are on Windows 11, we unregister the sparse package as well.
         UnregisterSparsePackage();
 
+        // And then we register it again.
         result = RegisterSparsePackage();
     }
 
@@ -377,6 +310,7 @@ HRESULT NppShell::Installer::Install()
     MoveFileToTempAndScheduleDeletion(GetApplicationPath() + L"\\NppModernShell.dll", false);
     MoveFileToTempAndScheduleDeletion(GetApplicationPath() + L"\\NppModernShell.msix", false);
 
+    // Finally we notify the shell that we have made changes, so it refreshes the context menus.
     SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, 0, 0);
 
     return result;
@@ -387,6 +321,8 @@ HRESULT NppShell::Installer::Uninstall()
     const bool isWindows11 = IsWindows11Installation();
 
     HRESULT result;
+
+    // We remove the old context menu in all cases, since both Windows 11 and older versions can have it setup if upgrading.
     result = UnregisterOldContextMenu();
 
     if (result != S_OK)
@@ -396,6 +332,7 @@ HRESULT NppShell::Installer::Uninstall()
 
     if (isWindows11)
     {
+        // Since we are on Windows 11, we unregister the sparse package as well.
         result = UnregisterSparsePackage();
 
         if (result != S_OK)
@@ -404,15 +341,18 @@ HRESULT NppShell::Installer::Uninstall()
         }
     }
 
+    // Finally we notify the shell that we have made changes, so it refreshes the context menus.
     SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, 0, 0);
 
     return S_OK;
 }
 
 STDAPI CleanupDll()
 {
+    // First we get the full path to this DLL.
     wstring currentFilePath(MAX_PATH, L'\0');
     GetModuleFileName(thisModule, &currentFilePath[0], MAX_PATH);
 
+    // Then we get it moved out of the way and scheduled for deletion.
     return MoveFileToTempAndScheduleDeletion(currentFilePath, true);
 }

NppShell.vcxproj

@@ -272,6 +272,7 @@
     <ClInclude Include="Installer.h" />
     <ClInclude Include="pch.h" />
     <ClInclude Include="SharedCounter.h" />
+    <ClInclude Include="RegistryKey.h" />
     <ClInclude Include="SimpleFactory.h" />
   </ItemGroup>
   <ItemGroup>
@@ -292,6 +293,7 @@
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">Create</PrecompiledHeader>
     </ClCompile>
     <ClCompile Include="SharedCounter.cpp" />
+    <ClCompile Include="RegistryKey.cpp" />
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />

NppShell.vcxproj.filters

@@ -19,6 +19,9 @@
     <Filter Include="CommandHandlers\Implementation">
       <UniqueIdentifier>{fc167318-0574-47ba-aa80-a48008ef4b8c}</UniqueIdentifier>
     </Filter>
+    <Filter Include="Registry">
+      <UniqueIdentifier>{2de5b87c-2f6d-4a04-bd61-1e1be9a91fdf}</UniqueIdentifier>
+    </Filter>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="Installer.h">
@@ -48,6 +51,9 @@
     <ClInclude Include="ModernEditWithNppExplorerCommandHandler.h">
       <Filter>CommandHandlers\Implementation</Filter>
     </ClInclude>
+    <ClInclude Include="RegistryKey.h">
+      <Filter>Registry</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="Installer.cpp">
@@ -74,12 +80,12 @@
     <ClCompile Include="ModernEditWithNppExplorerCommandHandler.cpp">
       <Filter>CommandHandlers\Implementation</Filter>
     </ClCompile>
+    <ClCompile Include="RegistryKey.cpp">
+      <Filter>Registry</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />
     <None Include="source.def" />
   </ItemGroup>
-  <ItemGroup>
-    <Natvis Include="$(MSBuildThisFileDirectory)..\..\natvis\wil.natvis" />
-  </ItemGroup>
 </Project>

PathHelper.cpp

@@ -8,8 +8,8 @@ extern HMODULE thisModule;
 
 const path GetThisModulePath()
 {
-    wchar_t pathBuffer[FILENAME_MAX] = { 0 };
-    GetModuleFileName(thisModule, pathBuffer, FILENAME_MAX);
+    wchar_t pathBuffer[MAX_PATH] = { 0 };
+    GetModuleFileNameW(thisModule, pathBuffer, MAX_PATH);
     return path(pathBuffer);
 }
 
@@ -23,4 +23,10 @@ const wstring NppShell::Helpers::GetContextMenuPath()
 {
     path modulePath = GetThisModulePath();
     return modulePath.parent_path().wstring();
+}
+
+const wstring NppShell::Helpers::GetContextMenuFullName()
+{
+    path modulePath = GetThisModulePath();
+    return modulePath.wstring();
 }

PathHelper.h

@@ -7,4 +7,5 @@ namespace NppShell::Helpers
 {
     const wstring GetApplicationPath();
     const wstring GetContextMenuPath();
+    const wstring GetContextMenuFullName();
 }