mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-08-29 10:09:43 +02:00
[Keyboard Manager] Apply valid entries from partial profiles
This commit is contained in:
@@ -148,12 +148,11 @@ KeyboardManagerEditor::KeyboardManagerEditor(HINSTANCE hInst) :
|
||||
{
|
||||
std::this_thread::sleep_for(std::chrono::milliseconds(500));
|
||||
|
||||
// Retry only file-level or transient failures. Partial data is deterministic
|
||||
// and must never be opened and subsequently saved as a truncated profile.
|
||||
// Retry only file-level or transient failures.
|
||||
loadResult = mappingConfiguration.LoadSettingsWithResult();
|
||||
}
|
||||
|
||||
configurationLoaded = loadResult == MappingConfigurationLoadResult::Success;
|
||||
configurationLoaded = loadResult != MappingConfigurationLoadResult::Failure;
|
||||
}
|
||||
|
||||
KeyboardManagerEditor::~KeyboardManagerEditor()
|
||||
|
||||
@@ -710,7 +710,7 @@ namespace RemappingUITests
|
||||
Assert::IsTrue(configuration.textExpansions.empty());
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadSettingsFromFile_ShouldKeepSnapshotWhenExistingProfileCannotBeParsed)
|
||||
TEST_METHOD (LoadSettingsFromFile_ShouldLeaveStateUnchangedWhenExistingProfileCannotBeParsed)
|
||||
{
|
||||
ScopedProfileTestPath corruptProfile{ L"corrupt.json" };
|
||||
{
|
||||
@@ -731,6 +731,75 @@ namespace RemappingUITests
|
||||
Assert::AreEqual(std::wstring(TextExpansionId1), configuration.textExpansions[0].id);
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadSettingsFromFile_ShouldApplyValidSubsetAndUpdateConfigurationOnPartial)
|
||||
{
|
||||
ScopedProfileTestPath partialProfile{ L"partial.json" };
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
json::JsonObject keyRemap;
|
||||
keyRemap.SetNamedValue(KeyboardManagerConstants::OriginalKeysSettingName, json::value(L"65"));
|
||||
keyRemap.SetNamedValue(KeyboardManagerConstants::NewRemapKeysSettingName, json::value(L"66"));
|
||||
profile.GetNamedObject(KeyboardManagerConstants::RemapKeysSettingName)
|
||||
.GetNamedArray(KeyboardManagerConstants::InProcessRemapKeysSettingName)
|
||||
.Append(keyRemap);
|
||||
auto rules = GetTextExpansionArray(profile);
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId1,
|
||||
L"valid",
|
||||
CreateActivationJson({ VK_SPACE }),
|
||||
L"loaded",
|
||||
true));
|
||||
auto invalidRule = CreateTextExpansionJson(
|
||||
TextExpansionId2,
|
||||
L"invalid",
|
||||
CreateActivationJson({ VK_SPACE }),
|
||||
L"skipped",
|
||||
true);
|
||||
invalidRule.Remove(KeyboardManagerConstants::TextExpansionReplacementTextSettingName);
|
||||
rules.Append(invalidRule);
|
||||
json::to_file(partialProfile.path.wstring(), profile);
|
||||
|
||||
MappingConfiguration configuration;
|
||||
configuration.currentConfig = L"previous";
|
||||
Assert::IsTrue(configuration.AddSingleKeyRemap(L'C', static_cast<DWORD>(L'D')));
|
||||
Assert::IsTrue(configuration.AddTextExpansion(CreateTextExpansionRule(TextExpansionId3)));
|
||||
|
||||
const auto result = configuration.LoadSettingsFromFile(L"partial-profile", partialProfile.path.wstring());
|
||||
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(result));
|
||||
Assert::AreEqual(std::wstring(L"partial-profile"), configuration.currentConfig);
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.singleKeyReMap.size());
|
||||
Assert::IsTrue(configuration.singleKeyReMap.contains(L'A'));
|
||||
Assert::AreEqual(static_cast<DWORD>(L'B'), std::get<DWORD>(configuration.singleKeyReMap.at(L'A')));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(std::wstring(TextExpansionId1), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"loaded"), configuration.textExpansions[0].replacementText);
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadSettingsFromJson_ShouldLoadValidAppSpecificShortcutsWhenGlobalShortcutIsInvalid)
|
||||
{
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
auto shortcuts = profile.GetNamedObject(KeyboardManagerConstants::RemapShortcutsSettingName);
|
||||
shortcuts.GetNamedArray(KeyboardManagerConstants::GlobalRemapShortcutsSettingName)
|
||||
.Append(json::value(L"invalid"));
|
||||
|
||||
json::JsonObject appSpecificShortcut;
|
||||
appSpecificShortcut.SetNamedValue(KeyboardManagerConstants::OriginalKeysSettingName, json::value(L"17;65"));
|
||||
appSpecificShortcut.SetNamedValue(KeyboardManagerConstants::NewRemapKeysSettingName, json::value(L"66"));
|
||||
appSpecificShortcut.SetNamedValue(KeyboardManagerConstants::TargetAppSettingName, json::value(L"test.exe"));
|
||||
shortcuts.GetNamedArray(KeyboardManagerConstants::AppSpecificRemapShortcutsSettingName)
|
||||
.Append(appSpecificShortcut);
|
||||
|
||||
MappingConfiguration configuration;
|
||||
const auto result = configuration.LoadSettingsFromJson(profile);
|
||||
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(result));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.appSpecificShortcutReMap.size());
|
||||
Assert::IsTrue(configuration.appSpecificShortcutReMap.contains(L"test.exe"));
|
||||
const auto& appMappings = configuration.appSpecificShortcutReMap.at(L"test.exe");
|
||||
Assert::AreEqual(static_cast<size_t>(1), appMappings.size());
|
||||
Assert::IsTrue(appMappings.contains(Shortcut(L"17;65")));
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadTextExpansions_ShouldAcceptCanonicalSchema_NormalizeActivationAndPreserveDuplicates)
|
||||
{
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
@@ -780,7 +849,7 @@ namespace RemappingUITests
|
||||
Assert::IsTrue(configuration.textExpansions.empty());
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadTextExpansions_ShouldRejectNonCanonicalOrDuplicateGuidsWithoutCommittingPartialSnapshot)
|
||||
TEST_METHOD (LoadTextExpansions_ShouldSkipInvalidOrDuplicateGuidsAndApplyValidSubset)
|
||||
{
|
||||
const auto existingRule = CreateTextExpansionRule(TextExpansionId3, L"existing", Shortcut(VK_TAB), L"unchanged", false);
|
||||
MappingConfiguration configuration;
|
||||
@@ -792,7 +861,14 @@ namespace RemappingUITests
|
||||
std::wstring(L"{11111111-1111-4111-8111-111111111111}") })
|
||||
{
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
GetTextExpansionArray(profile).Append(CreateTextExpansionJson(
|
||||
auto rules = GetTextExpansionArray(profile);
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId2,
|
||||
L"valid",
|
||||
CreateActivationJson({ VK_TAB }),
|
||||
L"loaded",
|
||||
true));
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
invalidId,
|
||||
L"brb",
|
||||
CreateActivationJson({ VK_SPACE }),
|
||||
@@ -802,7 +878,8 @@ namespace RemappingUITests
|
||||
const auto result = configuration.LoadSettingsFromJson(profile);
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(result));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(existingRule.id, configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(TextExpansionId2), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"loaded"), configuration.textExpansions[0].replacementText);
|
||||
}
|
||||
|
||||
auto duplicateProfile = CreateEmptyMappingProfile();
|
||||
@@ -823,10 +900,11 @@ namespace RemappingUITests
|
||||
const auto duplicateResult = configuration.LoadSettingsFromJson(duplicateProfile);
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(duplicateResult));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(existingRule.id, configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(TextExpansionId1), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"first"), configuration.textExpansions[0].sourceText);
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadTextExpansions_ShouldRejectMissingRequiredFieldsWithoutCommittingPartialSnapshot)
|
||||
TEST_METHOD (LoadTextExpansions_ShouldSkipEntriesWithMissingRequiredFieldsAndApplyValidSubset)
|
||||
{
|
||||
MappingConfiguration configuration;
|
||||
Assert::IsTrue(configuration.AddTextExpansion(CreateTextExpansionRule(TextExpansionId3)));
|
||||
@@ -839,6 +917,13 @@ namespace RemappingUITests
|
||||
KeyboardManagerConstants::TextExpansionEnabledSettingName })
|
||||
{
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
auto rules = GetTextExpansionArray(profile);
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId2,
|
||||
L"valid",
|
||||
CreateActivationJson({ VK_TAB }),
|
||||
L"loaded",
|
||||
true));
|
||||
auto rule = CreateTextExpansionJson(
|
||||
TextExpansionId1,
|
||||
L"brb",
|
||||
@@ -846,16 +931,17 @@ namespace RemappingUITests
|
||||
L"replacement",
|
||||
true);
|
||||
rule.Remove(missingField);
|
||||
GetTextExpansionArray(profile).Append(rule);
|
||||
rules.Append(rule);
|
||||
|
||||
const auto result = configuration.LoadSettingsFromJson(profile);
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(result));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(std::wstring(TextExpansionId3), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(TextExpansionId2), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"loaded"), configuration.textExpansions[0].replacementText);
|
||||
}
|
||||
}
|
||||
|
||||
TEST_METHOD (LoadTextExpansions_ShouldRejectInvalidActivationShapesWithoutCommittingPartialSnapshot)
|
||||
TEST_METHOD (LoadTextExpansions_ShouldSkipInvalidActivationShapesAndApplyValidSubset)
|
||||
{
|
||||
MappingConfiguration configuration;
|
||||
Assert::IsTrue(configuration.AddTextExpansion(CreateTextExpansionRule(TextExpansionId3)));
|
||||
@@ -874,7 +960,14 @@ namespace RemappingUITests
|
||||
for (const auto& activation : invalidActivations)
|
||||
{
|
||||
auto profile = CreateEmptyMappingProfile();
|
||||
GetTextExpansionArray(profile).Append(CreateTextExpansionJson(
|
||||
auto rules = GetTextExpansionArray(profile);
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId2,
|
||||
L"valid",
|
||||
CreateActivationJson({ VK_TAB }),
|
||||
L"loaded",
|
||||
true));
|
||||
rules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId1,
|
||||
L"brb",
|
||||
activation,
|
||||
@@ -884,10 +977,18 @@ namespace RemappingUITests
|
||||
const auto result = configuration.LoadSettingsFromJson(profile);
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(result));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(std::wstring(TextExpansionId3), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(TextExpansionId2), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"loaded"), configuration.textExpansions[0].replacementText);
|
||||
}
|
||||
|
||||
auto nonArrayProfile = CreateEmptyMappingProfile();
|
||||
auto nonArrayRules = GetTextExpansionArray(nonArrayProfile);
|
||||
nonArrayRules.Append(CreateTextExpansionJson(
|
||||
TextExpansionId2,
|
||||
L"valid",
|
||||
CreateActivationJson({ VK_TAB }),
|
||||
L"loaded",
|
||||
true));
|
||||
auto nonArrayRule = CreateTextExpansionJson(
|
||||
TextExpansionId1,
|
||||
L"brb",
|
||||
@@ -895,12 +996,13 @@ namespace RemappingUITests
|
||||
L"replacement",
|
||||
true);
|
||||
nonArrayRule.SetNamedValue(KeyboardManagerConstants::TextExpansionActivationKeysSettingName, json::value(L"32"));
|
||||
GetTextExpansionArray(nonArrayProfile).Append(nonArrayRule);
|
||||
nonArrayRules.Append(nonArrayRule);
|
||||
|
||||
const auto nonArrayResult = configuration.LoadSettingsFromJson(nonArrayProfile);
|
||||
Assert::AreEqual(static_cast<int>(MappingConfigurationLoadResult::Partial), static_cast<int>(nonArrayResult));
|
||||
Assert::AreEqual(static_cast<size_t>(1), configuration.textExpansions.size());
|
||||
Assert::AreEqual(std::wstring(TextExpansionId3), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(TextExpansionId2), configuration.textExpansions[0].id);
|
||||
Assert::AreEqual(std::wstring(L"loaded"), configuration.textExpansions[0].replacementText);
|
||||
}
|
||||
|
||||
TEST_METHOD (TextExpansionCrud_ShouldUseGuidPreserveOrderAndAllowDuplicateSourceAndActivation)
|
||||
|
||||
@@ -181,12 +181,12 @@ void KeyboardManager::LoadSettings()
|
||||
std::this_thread::sleep_for(std::chrono::milliseconds(500));
|
||||
|
||||
// Retry only file-level/transient failures. A Partial result is deterministic
|
||||
// invalid data and has already retained the last-known-good snapshot.
|
||||
// invalid data whose valid entries have already been applied.
|
||||
loadResult = state.LoadSettingsWithResult();
|
||||
}
|
||||
if (loadResult == MappingConfigurationLoadResult::Partial)
|
||||
{
|
||||
Logger::error(L"Keyboard Manager settings contained invalid entries; retained the last-known-good runtime snapshot.");
|
||||
Logger::error(L"Keyboard Manager settings contained invalid entries; skipped them and loaded the valid entries.");
|
||||
}
|
||||
try
|
||||
{
|
||||
|
||||
@@ -776,7 +776,7 @@ bool MappingConfiguration::LoadShortcutRemaps(const json::JsonObject& jsonData,
|
||||
}
|
||||
|
||||
// Load app specific shortcut remaps
|
||||
result = result && LoadAppSpecificShortcutRemaps(remapShortcutsData);
|
||||
result = LoadAppSpecificShortcutRemaps(remapShortcutsData) && result;
|
||||
}
|
||||
}
|
||||
catch (...)
|
||||
@@ -804,10 +804,6 @@ MappingConfigurationLoadResult MappingConfiguration::LoadSettingsFromJson(const
|
||||
result = candidate.LoadShortcutRemaps(configFile, KeyboardManagerConstants::RemapShortcutsToTextSettingName) && result;
|
||||
result = candidate.LoadSingleKeyToTextRemaps(configFile) && result;
|
||||
result = candidate.LoadTextExpansions(configFile) && result;
|
||||
if (!result)
|
||||
{
|
||||
return MappingConfigurationLoadResult::Partial;
|
||||
}
|
||||
|
||||
singleKeyReMap = std::move(candidate.singleKeyReMap);
|
||||
scanMap = std::move(candidate.scanMap);
|
||||
@@ -818,7 +814,7 @@ MappingConfigurationLoadResult MappingConfiguration::LoadSettingsFromJson(const
|
||||
appSpecificShortcutReMap = std::move(candidate.appSpecificShortcutReMap);
|
||||
appSpecificShortcutReMapSortedKeys = std::move(candidate.appSpecificShortcutReMapSortedKeys);
|
||||
|
||||
return MappingConfigurationLoadResult::Success;
|
||||
return result ? MappingConfigurationLoadResult::Success : MappingConfigurationLoadResult::Partial;
|
||||
}
|
||||
|
||||
MappingConfigurationLoadResult MappingConfiguration::LoadSettingsFromFile(
|
||||
@@ -856,10 +852,7 @@ MappingConfigurationLoadResult MappingConfiguration::LoadSettingsFromFile(
|
||||
}
|
||||
|
||||
const auto result = LoadSettingsFromJson(*configFile);
|
||||
if (result == MappingConfigurationLoadResult::Success)
|
||||
{
|
||||
currentConfig = configurationName;
|
||||
}
|
||||
currentConfig = configurationName;
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ using TextExpansionTable = std::vector<TextExpansionRule>;
|
||||
enum class MappingConfigurationLoadResult
|
||||
{
|
||||
Success = 0,
|
||||
// The parsed profile was applied, but one or more invalid entries were skipped.
|
||||
Partial = 1,
|
||||
Failure = 2,
|
||||
};
|
||||
@@ -55,10 +56,10 @@ public:
|
||||
// Load while distinguishing rejected entries from a file-level failure.
|
||||
MappingConfigurationLoadResult LoadSettingsWithResult();
|
||||
|
||||
// Load an already parsed profile. Invalid entries are rejected and reported as Partial.
|
||||
// Load an already parsed profile. Valid entries are applied, while rejected entries are reported as Partial.
|
||||
MappingConfigurationLoadResult LoadSettingsFromJson(const json::JsonObject& configFile);
|
||||
|
||||
// Load a named profile file. A profile that has not been created yet is a valid empty snapshot.
|
||||
// Load a named profile file. A missing profile is a valid empty snapshot; a partial profile applies its valid entries.
|
||||
MappingConfigurationLoadResult LoadSettingsFromFile(const std::wstring& configurationName, const std::wstring& filePath);
|
||||
|
||||
bool IsConfigurationNameResolved() const;
|
||||
|
||||
Reference in New Issue
Block a user