Skip to content

Commit 81e251c

Browse files
daveraymentMuyuanMSCopilot
authored
[Quick Accent] Move language data to PowerAccent.Common library, refactor (#47211)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Language data for Quick Accent was previously defined in **PowerAccent.Core/Languages.cs**, internal to the Quick Accent application itself. The Settings application had no access to the list and had to maintain a parallel, manually-synchronised list of the language names and groups, and there was no single place to look up the complete set of languages. This PR resolves this and extracts the language data into a new **PowerAccent.Common** project with no external or UI dependencies, making it the single source of truth for both the character popup and Settings UI. The implicit and non-obvious ordering of the old **Languages.cs** has been replaced with explicit ordering of the language groups and languages list. A new unit test project for the application has also been added. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #47159 - [x] Closes: #30000 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments The following changes and additions have been made: New project **PowerAccent.Common** contains: - **Language.cs** - an enum of Quick Accent's pseudo-ISO identifiers for each of the supported languages - **LetterKey.cs** - mirrors the LetterKey enum defined in the KeyboardListener.idl, and must be kept in sync with that. This exposes the VK_* data to the Settings UI without it having to reference the PowerAccentKeyboardService project - **LanguageGroup.cs** - classifies languages as `Language` (for spoken languages), `Special` (for sets like Currency and International Phonetic Alphabet), or `UserDefined` (reserved for future work) - **LanguageInfo.cs** - a record which combines a language's identity, group, resource identifier, and character mappings - **CharacterMappings.cs** - a new version of the old **Languages.cs**, providing: - `All` - the canonical registry of every `LanguageInfo` entry - `DisplayOrder` - the explicit within-group ordering for languages, for the default popup ordering, i.e. when the user has not got frequency-based ordering enabled. This is a culturally-neutral alphabetical ordering by our pseudo-ISO language IDs, and close to the previous order, minimising any impact to users' muscle memory - `GroupDisplayOrder` - the explicit ordering of language groups for both the popup and the Settings UI - `LanguageLookup` - new _O(1)_ `Language` to `LanguageInfo` dictionary - `GetCharacters(LetterKey, Language[])` - a deduplicating character collector and sorter A significant improvement over the old **Languages.cs** is that language ordering is now explicit and in one place. Previously, popup ordering was an implicit side-effect of a large Union chain across all language mappings, with the enum, the language mapping declarations and the union all carrying different orderings. This was a source of confusion and made adding new languages fragile. The original **Languages.cs** has been deleted. There's now a thin `CharacterMappings` adapter that casts the `LetterKey` to the managed equivalent by numeric value. This is another effort to decouple the Settings UI and Quick Accent and only share the required elements. In Settings, the `PowerAccentViewModel` class has been updated. It now derives the language list directly from `CharacterMappings.All`; the `InitializeLanguages()` call handles localisation, sorting and grouping in a single place using `GroupDisplayOrder`. A new unit test project covers `CharacterMappings`, the `LetterKey` IDL-to-managed code bridge and general language declaration checks. The application is now much more robust against changes which alter the declared order, introduce empty/null elements, and so on. It is now impossible to add a new language and forget a constituent part (the enum entry, its place in the display order, the character mappings themselves) without a test failing. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed See new unit test project for comprehensive tests. Manually confirmed: - Triggering Quick Accent with no language selected does not report an error - Language order is consistent in the popup, no matter what order the languages are selected in Settings - The order of languages in Settings is consistent, alphabetically by localised name - All declared languages are present in Settings and no resource IDs were missed --------- Co-authored-by: Muyuan Li (from Dev Box) <muyuanli@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f02b66c commit 81e251c

19 files changed

Lines changed: 1461 additions & 1102 deletions

.pipelines/verifyCommonProps.ps1

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ foreach ($csprojFile in $csprojFilesArray) {
4848
continue
4949
}
5050

51+
# The PowerAccent.Common project does not target WinRT, so skip it
52+
if ($csprojFile -like '*PowerAccent.Common.csproj') {
53+
continue
54+
}
55+
5156
$importExists = Test-ImportSharedCsWinRTProps -filePath $csprojFile
5257
if (!$importExists) {
5358
Write-Output "$csprojFile need to import 'Common.Dotnet.CsWinRT.props'."

PowerToys.slnx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,14 @@
802802
<Project Path="src/modules/peek/peek/peek.vcxproj" Id="a1425b53-3d61-4679-8623-e64a0d3d0a48" />
803803
</Folder>
804804
<Folder Name="/modules/PowerAccent/">
805+
<Project Path="src/modules/poweraccent/PowerAccent.Common.UnitTests/PowerAccent.Common.UnitTests.csproj">
806+
<Platform Solution="*|ARM64" Project="ARM64" />
807+
<Platform Solution="*|x64" Project="x64" />
808+
</Project>
809+
<Project Path="src/modules/poweraccent/PowerAccent.Common/PowerAccent.Common.csproj">
810+
<Platform Solution="*|ARM64" Project="ARM64" />
811+
<Platform Solution="*|x64" Project="x64" />
812+
</Project>
805813
<Project Path="src/modules/poweraccent/PowerAccent.Core/PowerAccent.Core.csproj">
806814
<Platform Solution="*|ARM64" Project="ARM64" />
807815
<Platform Solution="*|x64" Project="x64" />
@@ -1134,3 +1142,5 @@
11341142
<Project Path="src/Update/PowerToys.Update.vcxproj" Id="44ce9ae1-4390-42c5-bacc-0fd6b40aa203" />
11351143
<Project Path="tools/project_template/ModuleTemplate/ModuleTemplateCompileTest.vcxproj" Id="64a80062-4d8b-4229-8a38-dfa1d7497749" />
11361144
</Solution>
1145+
1146+
Lines changed: 302 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,302 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
7+
namespace PowerAccent.Common.UnitTests;
8+
9+
[TestClass]
10+
public sealed class CharacterMappingsTests
11+
{
12+
// Every Language enum value must appear in All exactly once. If a value is missing,
13+
// GetCharacters will silently return no characters for that language. If it appears
14+
// more than once, the second entry is dead code.
15+
[TestMethod]
16+
public void All_ContainsEveryLanguageEnumValue_ExactlyOnce()
17+
{
18+
foreach (Language lang in Enum.GetValues<Language>())
19+
{
20+
var count = CharacterMappings.All.Count(e => e.Id == lang);
21+
Assert.AreEqual(
22+
1,
23+
count,
24+
$"Language.{lang} appears {count} time(s) in CharacterMappings.All — expected exactly 1.");
25+
}
26+
}
27+
28+
/// <summary>
29+
/// The Characters dictionary for each entry in All must not contain null or empty
30+
/// mappings.
31+
/// </summary>
32+
[TestMethod]
33+
public void All_Characters_ContainsNoNullOrEmptyEntries()
34+
{
35+
foreach (var entry in CharacterMappings.All)
36+
{
37+
foreach (var kvp in entry.Characters)
38+
{
39+
var key = kvp.Key;
40+
var mappings = kvp.Value;
41+
42+
Assert.IsNotNull(
43+
mappings,
44+
$"Language.{entry.Id} has a null mappings array for key {key}.");
45+
46+
Assert.IsTrue(
47+
mappings.Length > 0,
48+
$"Language.{entry.Id} has an empty mappings array for key {key}.");
49+
50+
Assert.IsFalse(
51+
mappings.Any(c => string.IsNullOrEmpty(c)),
52+
$"Language.{entry.Id} has null or empty string(s) in its mappings array for key {key}.");
53+
}
54+
}
55+
}
56+
57+
// Every Language enum value must appear in DisplayOrder exactly once. If a value is
58+
// missing, its characters will be silently omitted from the popup. If it appears more
59+
// than once, Collect will emit its characters twice (before Distinct removes them).
60+
[TestMethod]
61+
public void DisplayOrder_ContainsEveryLanguageEnumValue_ExactlyOnce()
62+
{
63+
foreach (Language lang in Enum.GetValues<Language>())
64+
{
65+
var count = CharacterMappings.DisplayOrder.Count(l => l == lang);
66+
Assert.AreEqual(
67+
1,
68+
count,
69+
$"Language.{lang} appears {count} time(s) in CharacterMappings.DisplayOrder - expected exactly 1.");
70+
}
71+
}
72+
73+
// Every LanguageGroup enum value must appear in GroupDisplayOrder exactly once.
74+
[TestMethod]
75+
public void GroupDisplayOrder_ContainsEveryLanguageGroupValue_ExactlyOnce()
76+
{
77+
foreach (LanguageGroup group in Enum.GetValues<LanguageGroup>())
78+
{
79+
var count = CharacterMappings.GroupDisplayOrder.Count(g => g == group);
80+
Assert.AreEqual(
81+
1,
82+
count,
83+
$"LanguageGroup.{group} appears {count} time(s) in CharacterMappings.GroupDisplayOrder - expected exactly 1.");
84+
}
85+
}
86+
87+
// LanguageLookup must contain an entry for every Language enum value, derived from All.
88+
[TestMethod]
89+
public void LanguageLookup_ContainsEveryLanguageEnumValue()
90+
{
91+
foreach (Language lang in Enum.GetValues<Language>())
92+
{
93+
Assert.IsTrue(
94+
CharacterMappings.LanguageLookup.ContainsKey(lang),
95+
$"Language.{lang} is missing from CharacterMappings.LanguageLookup.");
96+
}
97+
}
98+
99+
// Every entry in All must have a non-empty Identifier. A blank identifier would
100+
// produce a malformed resource key (e.g. "QuickAccent_SelectedLanguage_") that
101+
// silently resolves to an empty string in the Settings UI.
102+
[TestMethod]
103+
public void All_EveryEntry_HasNonEmptyIdentifier()
104+
{
105+
foreach (var entry in CharacterMappings.All)
106+
{
107+
Assert.IsFalse(
108+
string.IsNullOrWhiteSpace(entry.Identifier),
109+
$"Language.{entry.Id} has a null or whitespace Identifier.");
110+
}
111+
}
112+
113+
// Every entry in All must have a non-null Characters dictionary. A null would throw
114+
// at runtime inside GetCharacters.
115+
[TestMethod]
116+
public void All_EveryEntry_HasNonNullCharacters()
117+
{
118+
foreach (var entry in CharacterMappings.All)
119+
{
120+
Assert.IsNotNull(
121+
entry.Characters,
122+
$"Language.{entry.Id} has a null Characters dictionary.");
123+
}
124+
}
125+
126+
// Every LanguageGroup enum value must be used by at least one entry in All. This
127+
// guards against a new group being added to the enum but forgotten in the data, which
128+
// would make it impossible to test or exercise that group path.
129+
[TestMethod]
130+
public void All_EveryLanguageGroupValue_IsUsedAtLeastOnce()
131+
{
132+
var usedGroups = CharacterMappings.All.Select(e => e.Group).ToHashSet();
133+
foreach (LanguageGroup group in Enum.GetValues<LanguageGroup>())
134+
{
135+
// UserDefined is reserved for future use and may not yet be populated.
136+
if (group == LanguageGroup.UserDefined)
137+
{
138+
continue;
139+
}
140+
141+
Assert.IsTrue(
142+
usedGroups.Contains(group),
143+
$"LanguageGroup.{group} is defined in the enum but no entry in CharacterMappings.All uses it.");
144+
}
145+
}
146+
147+
// GetCharacters with an empty language array must return an empty array without
148+
// throwing.
149+
[TestMethod]
150+
public void GetCharacters_EmptyLanguages_ReturnsEmpty()
151+
{
152+
var result = CharacterMappings.GetCharacters(LetterKey.VK_A, []);
153+
Assert.AreEqual(0, result.Length);
154+
}
155+
156+
// GetCharacters with all languages must return a non-empty result for a key that is
157+
// mapped in at least one language (VK_A is mapped in the majority of languages).
158+
[TestMethod]
159+
public void GetCharacters_AllLanguages_ReturnsNonEmptyForCommonKey()
160+
{
161+
var allLangs = Enum.GetValues<Language>();
162+
var result = CharacterMappings.GetCharacters(LetterKey.VK_A, allLangs);
163+
Assert.IsTrue(result.Length > 0, "Expected at least one character for VK_A across all languages.");
164+
}
165+
166+
// GetCharacters must deduplicate characters that appear in multiple languages.
167+
// If two languages both map VK_A to the same character, it should appear only once.
168+
[TestMethod]
169+
public void GetCharacters_DeduplicatesCharactersAcrossLanguages()
170+
{
171+
var allLangs = Enum.GetValues<Language>();
172+
var result = CharacterMappings.GetCharacters(LetterKey.VK_A, allLangs);
173+
var distinct = result.Distinct().ToArray();
174+
CollectionAssert.AreEquivalent(
175+
distinct,
176+
result,
177+
"GetCharacters returned duplicate characters. Results should be deduplicated.");
178+
}
179+
180+
// Calling GetCharacters twice with all languages should return the same results,
181+
// confirming the cache path is consistent.
182+
[TestMethod]
183+
public void GetCharacters_AllLanguagesCachedResult_IsConsistent()
184+
{
185+
var allLangs = Enum.GetValues<Language>();
186+
var first = CharacterMappings.GetCharacters(LetterKey.VK_E, allLangs);
187+
var second = CharacterMappings.GetCharacters(LetterKey.VK_E, allLangs);
188+
CollectionAssert.AreEqual(first, second, "Cached and non-cached results for VK_E differ.");
189+
}
190+
191+
// GetCharacters for a single language should return exactly that language's
192+
// characters for a key it maps. The test derives both the language and key from the
193+
// live data so it stays valid regardless of future mapping changes.
194+
[TestMethod]
195+
public void GetCharacters_SingleLanguage_ReturnsOnlyThatLanguagesCharacters()
196+
{
197+
var langInfo = CharacterMappings.All.First(l => l.Characters.Count > 0);
198+
var (key, expected) = langInfo.Characters.First();
199+
200+
var result = CharacterMappings.GetCharacters(key, [langInfo.Id]);
201+
202+
CollectionAssert.AreEquivalent(
203+
expected,
204+
result,
205+
$"GetCharacters for Language.{langInfo.Id} / LetterKey.{key} did not match the mapped characters.");
206+
}
207+
208+
// GetCharacters must throw KeyNotFoundException when passed a Language value that is
209+
// not in LanguageLookup (i.e. not in All). This is deliberate fail-fast behaviour:
210+
// an unknown language is a programming error, not a recoverable condition. The cast
211+
// produces a valid enum value that was never registered in All.
212+
[TestMethod]
213+
public void GetCharacters_UnknownLanguage_ThrowsKeyNotFoundException()
214+
{
215+
var unknown = (Language)(-1);
216+
Assert.ThrowsExactly<KeyNotFoundException>(
217+
() => CharacterMappings.GetCharacters(LetterKey.VK_A, [unknown]),
218+
"Expected KeyNotFoundException when a Language value absent from LanguageLookup is passed to GetCharacters.");
219+
}
220+
221+
/// <summary>
222+
/// GetCharacters must return characters sorted strictly by GroupDisplayOrder and
223+
/// then DisplayOrder, regardless of the sequence of languages passed in.
224+
/// </summary>
225+
[TestMethod]
226+
public void GetCharacters_SortsOutput_AccordingToDisplayOrder()
227+
{
228+
// Input in the wrong order.
229+
Language[] input = [Language.SPECIAL, Language.PI, Language.FR];
230+
231+
var result = CharacterMappings.GetCharacters(LetterKey.VK_A, input);
232+
233+
// Derive correct order.
234+
var expectedOrder = CharacterMappings.All
235+
.Where(lang => input.Contains(lang.Id))
236+
.OrderBy(lang => CharacterMappings.GroupDisplayOrder.ToList().IndexOf(lang.Group))
237+
.ThenBy(lang => CharacterMappings.DisplayOrder.ToList().IndexOf(lang.Id))
238+
.SelectMany(lang => lang.Characters.TryGetValue(LetterKey.VK_A, out var chars) ? chars : [])
239+
.Distinct()
240+
.ToArray();
241+
242+
CollectionAssert.AreEqual(
243+
expectedOrder,
244+
result,
245+
"GetCharacters did not return characters in the expected order based on GroupDisplayOrder and DisplayOrder.");
246+
}
247+
248+
// Collect sorts by _languageOrder[m.Id], so every entry in All must appear in
249+
// DisplayOrder. Adding to All without updating DisplayOrder will throw
250+
// KeyNotFoundException at the first GetCharacters call that exercises that language.
251+
// This test verifies the invariant directly so the failure is caught at test time
252+
// rather than at runtime.
253+
[TestMethod]
254+
public void All_EveryEntry_ExistsInDisplayOrder()
255+
{
256+
var displayOrderSet = CharacterMappings.DisplayOrder.ToHashSet();
257+
foreach (var entry in CharacterMappings.All)
258+
{
259+
Assert.IsTrue(
260+
displayOrderSet.Contains(entry.Id),
261+
$"Language.{entry.Id} is in All but missing from DisplayOrder. Add it to DisplayOrder to prevent a KeyNotFoundException at runtime.");
262+
}
263+
}
264+
265+
// GetCharacters for a key that is not mapped in a given language should return empty.
266+
// The test finds a language and an absent key from the live data so it stays valid
267+
// regardless of future mapping changes.
268+
[TestMethod]
269+
public void GetCharacters_UnmappedKey_ReturnsEmpty()
270+
{
271+
var allKeys = Enum.GetValues<LetterKey>().ToHashSet();
272+
var langInfo = CharacterMappings.All.First(l => allKeys.Except(l.Characters.Keys).Any());
273+
var absentKey = allKeys.Except(langInfo.Characters.Keys).First();
274+
275+
var result = CharacterMappings.GetCharacters(absentKey, [langInfo.Id]);
276+
277+
Assert.AreEqual(
278+
0,
279+
result.Length,
280+
$"Expected empty result for Language.{langInfo.Id} / LetterKey.{absentKey}, which has no mapping.");
281+
}
282+
283+
/// <summary>
284+
/// Spoken languages in DisplayOrder should be sorted alphabetically by their enum
285+
/// names to remain culturally neutral.
286+
/// </summary>
287+
[TestMethod]
288+
public void DisplayOrder_SpokenLanguages_AreSortedAlphabeticallyByDisplayName()
289+
{
290+
var spokenLangs = CharacterMappings.DisplayOrder
291+
.Where(lang => CharacterMappings.LanguageLookup[lang].Group == LanguageGroup.Language)
292+
.Select(lang => lang.ToString())
293+
.ToList();
294+
295+
var sorted = spokenLangs.OrderBy(l => l, StringComparer.OrdinalIgnoreCase).ToList();
296+
297+
CollectionAssert.AreEqual(
298+
sorted,
299+
spokenLangs,
300+
"Spoken languages in DisplayOrder should be sorted alphabetically by their enum names.");
301+
}
302+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using Microsoft.VisualStudio.TestTools.UnitTesting;
6+
using PowerAccent.Common;
7+
using WinRtLetterKey = PowerToys.PowerAccentKeyboardService.LetterKey;
8+
9+
namespace PowerAccent.Common.UnitTests;
10+
11+
[TestClass]
12+
public sealed class LetterKeyTests
13+
{
14+
// Verifies that the managed LetterKey enum in PowerAccent.Common stays in sync with the
15+
// WinRT LetterKey enum defined in KeyboardListener.idl. The adapter in PowerAccent.Core
16+
// casts between them via their integer values, so any divergence would silently produce
17+
// wrong character mappings at runtime.
18+
[TestMethod]
19+
public void ManagedLetterKey_MatchesWinRtLetterKey_AllNamesPresent()
20+
{
21+
foreach (WinRtLetterKey winRtValue in Enum.GetValues<WinRtLetterKey>())
22+
{
23+
var name = winRtValue.ToString();
24+
Assert.IsTrue(
25+
Enum.TryParse<LetterKey>(name, out _),
26+
$"WinRT LetterKey.{name} has no corresponding value in the managed LetterKey enum. Update PowerAccent.Common/LetterKey.cs.");
27+
}
28+
}
29+
30+
[TestMethod]
31+
public void ManagedLetterKey_MatchesWinRtLetterKey_ValuesMatch()
32+
{
33+
foreach (WinRtLetterKey winRtValue in Enum.GetValues<WinRtLetterKey>())
34+
{
35+
var name = winRtValue.ToString();
36+
if (Enum.TryParse<LetterKey>(name, out var managedValue))
37+
{
38+
Assert.AreEqual(
39+
(int)(object)winRtValue,
40+
(int)managedValue,
41+
$"LetterKey.{name} has value {(int)(object)winRtValue} in WinRT but {(int)managedValue} in the managed enum. Update PowerAccent.Common/LetterKey.cs.");
42+
}
43+
}
44+
}
45+
}

0 commit comments

Comments
 (0)