Change accelerators that use Ctrl+Alt

As per https://devblogs.microsoft.com/oldnewthing/20040329-00/?p=40003,
Ctrl+Alt shouldn't be used as a shortcut modifier, since it acts as an
alternate shift key on some keyboard layouts. Therefore, the shortcuts
using Ctrl+Alt have been updated from:

- Ctrl+Alt+S (wildcard select)
- Ctrl+Alt+D (wildcard deselect)

to:

- Ctrl+Shift+S (wildcard select)
- Ctrl+Shift+W (wildcard deselect)

Some checks have been added to make sure that no other Ctrl+Alt
shortcuts are added.

Author: derceg

Explorer++/Explorer++/AcceleratorHelper.h

@@ -13,3 +13,41 @@ std::wstring BuildAcceleratorString(const ACCEL &accelerator);
 
 std::vector<ACCEL> TableToAcceleratorItems(HACCEL acceleratorTable);
 wil::unique_haccel AcceleratorItemsToTable(std::span<const ACCEL> accelerators);
+
+// As per https://devblogs.microsoft.com/oldnewthing/20040329-00/?p=40003, Ctrl+Alt shouldn't be
+// used as a shortcut modifier, since it acts as an alternate shift key on some keyboard layouts.
+constexpr bool DoAcceleratorsContainCtrlAlt(std::span<const ACCEL> accelerators)
+{
+	auto itr = std::find_if(accelerators.begin(), accelerators.end(),
+		[](const auto &accel) { return WI_AreAllFlagsSet(accel.fVirt, FCONTROL | FALT); });
+	return itr != accelerators.end();
+}
+
+// Checks whether there are any duplicate key entries within the specified set of accelerators.
+// (i.e. two or more entries that have the same key combination). Duplicate entries are invalid,
+// since only the first will actually be used.
+constexpr bool DoAcceleratorsContainDuplicates(std::span<const ACCEL> accelerators)
+{
+	for (const auto &accelerator : accelerators)
+	{
+		auto numMatches = std::count_if(accelerators.begin(), accelerators.end(),
+			[&accelerator](const auto &currentAccelerator)
+			{
+				return currentAccelerator.fVirt == accelerator.fVirt
+					&& currentAccelerator.key == accelerator.key;
+			});
+
+		if (numMatches > 1)
+		{
+			return true;
+		}
+	}
+
+	return false;
+}
+
+constexpr bool AreAcceleratorsValid(std::span<const ACCEL> accelerators)
+{
+	return !DoAcceleratorsContainCtrlAlt(accelerators)
+		&& !DoAcceleratorsContainDuplicates(accelerators);
+}

Explorer++/Explorer++/AcceleratorManager.cpp

@@ -23,6 +23,8 @@ const std::vector<ACCEL> &AcceleratorManager::GetAccelerators() const
 
 void AcceleratorManager::SetAccelerators(std::span<const ACCEL> updatedAccelerators)
 {
+	DCHECK(AreAcceleratorsValid(updatedAccelerators));
+
 	m_acceleratorTable = AcceleratorItemsToTable(updatedAccelerators);
 	m_accelerators = { updatedAccelerators.begin(), updatedAccelerators.end() };
 }

Explorer++/Explorer++/DefaultAccelerators.cpp

@@ -4,6 +4,7 @@
 
 #include "stdafx.h"
 #include "DefaultAccelerators.h"
+#include "AcceleratorHelper.h"
 #include "AcceleratorManager.h"
 #include "MainResource.h"
 
@@ -50,8 +51,8 @@ constexpr ACCEL g_defaultAccelerators[] = {
 	{ FVIRTKEY | FCONTROL | FSHIFT, 'M', IDM_EDIT_MOVETOFOLDER },
 	{ FVIRTKEY | FCONTROL | FSHIFT, 'V', IDM_EDIT_PASTESHORTCUT },
 	{ FVIRTKEY | FCONTROL | FSHIFT, 'N', IDM_EDIT_SELECTNONE },
-	{ FVIRTKEY | FCONTROL | FALT, 'D', IDM_EDIT_WILDCARDDESELECT },
-	{ FVIRTKEY | FCONTROL | FALT, 'S', IDM_EDIT_WILDCARDSELECTION },
+	{ FVIRTKEY | FCONTROL | FSHIFT, 'W', IDM_EDIT_WILDCARDDESELECT },
+	{ FVIRTKEY | FCONTROL | FSHIFT, 'S', IDM_EDIT_WILDCARDSELECTION },
 	{ FVIRTKEY | FCONTROL, 'W', IDM_FILE_CLOSETAB },
 	{ FVIRTKEY | FCONTROL, VK_F4, IDM_FILE_CLOSETAB },
 	{ FVIRTKEY | FCONTROL | FSHIFT, 'P', IDM_FILE_COPYFOLDERPATH },
@@ -88,6 +89,8 @@ constexpr ACCEL g_defaultAccelerators[] = {
 };
 // clang-format on
 
+static_assert(AreAcceleratorsValid(g_defaultAccelerators));
+
 }
 
 AcceleratorManager InitializeAcceleratorManager()

Explorer++/Explorer++/Explorer++.rc

@@ -40,3 +40,19 @@ TEST(AcceleratorHelperTest, AcceleratorTableConversion)
 	auto outputAccelerators = TableToAcceleratorItems(table.get());
 	EXPECT_THAT(outputAccelerators, ElementsAreArray(accelerators));
 }
+
+TEST(AcceleratorHelperTest, Validity)
+{
+	WORD commandIdCounter = 1;
+	EXPECT_FALSE(DoAcceleratorsContainCtrlAlt({ { { FVIRTKEY | FCONTROL, 'T', commandIdCounter++ },
+		{ FVIRTKEY | FCONTROL | FSHIFT, 'N', commandIdCounter++ } } }));
+	EXPECT_TRUE(DoAcceleratorsContainCtrlAlt({ { { FVIRTKEY, VK_F1, commandIdCounter++ },
+		{ FVIRTKEY | FCONTROL | FALT, 'W', commandIdCounter++ } } }));
+
+	EXPECT_FALSE(DoAcceleratorsContainDuplicates(
+		{ { { FVIRTKEY | FCONTROL | FSHIFT, 'P', commandIdCounter++ },
+			{ FVIRTKEY, VK_F3, commandIdCounter++ } } }));
+	EXPECT_TRUE(
+		DoAcceleratorsContainDuplicates({ { { FVIRTKEY | FCONTROL, 'A', commandIdCounter++ },
+			{ FVIRTKEY | FCONTROL, 'A', commandIdCounter++ } } }));
+}