From bbbd73ead4d64d87d4688ebf314b3cbfc342c135 Mon Sep 17 00:00:00 2001 From: matt-edmondson Date: Sat, 26 Sep 2026 10:27:03 +0000 Subject: [PATCH] Normalize modifier aliases in the Note constructors [patch] Only Chord.Parse mapped Control to Ctrl and Win/Windows/Cmd/Command to Meta, so chords built through KeybindingService.ParseChord, new Chord(...) or a stored profile kept the alias, displayed identically, and never matched the canonical chord. Canonicalize in both Note constructors so every path agrees, and drop the now-redundant mapping in Chord.Parse. Fixes #107 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CZB6C9rAp2hA5WQDcw2NDp --- .../ModifierAliasNormalizationTests.cs | 109 ++++++++++++++++++ Keybinding/Models/MusicalTypes.cs | 29 +++-- 2 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 Keybinding.Test/ModifierAliasNormalizationTests.cs diff --git a/Keybinding.Test/ModifierAliasNormalizationTests.cs b/Keybinding.Test/ModifierAliasNormalizationTests.cs new file mode 100644 index 0000000..832f06e --- /dev/null +++ b/Keybinding.Test/ModifierAliasNormalizationTests.cs @@ -0,0 +1,109 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Keybinding.Test; + +using ktsu.Keybinding.Core; +using ktsu.Keybinding.Core.Models; +using ktsu.Keybinding.Core.Services; + +[TestClass] +public class ModifierAliasNormalizationTests +{ + private string _testDataDirectory = null!; + + [TestInitialize] + public void Setup() + { + _testDataDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(_testDataDirectory); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(_testDataDirectory)) + { + Directory.Delete(_testDataDirectory, recursive: true); + } + } + + private static KeybindingService CreateService() + { + CommandRegistry registry = new(); + registry.RegisterCommand(new Command("save", "Save")); + ProfileManager profiles = new(); + profiles.CreateProfile("p", "Profile"); + profiles.SetActiveProfile("p"); + return new KeybindingService(registry, profiles); + } + + [TestMethod] + [DataRow("Control", "Ctrl")] + [DataRow("Win", "Meta")] + [DataRow("Windows", "Meta")] + [DataRow("Cmd", "Meta")] + [DataRow("Command", "Meta")] + public void EveryConstructionPath_NormalizesModifierAliases(string alias, string canonical) + { + ArgumentNullException.ThrowIfNull(alias); + Chord expected = Chord.Parse($"{canonical}+S"); + KeybindingService service = CreateService(); + + Chord[] built = + [ + service.ParseChord($"{alias}+S"), + Chord.Parse($"{alias}+S"), + new Chord([new Note(alias), new Note("S")]), + new Chord([new Note(NoteName.Create(alias.ToUpperInvariant())), new Note("S")]), + ]; + + foreach (Chord chord in built) + { + Assert.AreEqual(expected, chord, $"{chord} should equal {expected}"); + Assert.AreEqual(expected.GetHashCode(), chord.GetHashCode(), $"{chord} should hash like {expected}"); + } + } + + [TestMethod] + public void ParseChord_AliasAndCanonicalName_CollapseToOneNote() + { + Chord chord = CreateService().ParseChord("Ctrl+Control"); + + Assert.AreEqual(1, chord.Notes.Count); + Assert.AreEqual("Ctrl", chord.ToString()); + } + + [TestMethod] + public void BindChord_WithAliasSpelling_IsFoundByCanonicalChord() + { + KeybindingService service = CreateService(); + Assert.IsTrue(service.BindChord("save", service.ParseChord("Control+S"))); + + Assert.AreEqual("save", service.FindCommandByChord(Chord.Parse("Ctrl+S"))); + } + + [TestMethod] + public async Task StoredProfileWithAliasSpelling_MatchesAfterReload() + { + string json = """ + [ + { + "id": "p", + "name": "Profile", + "chords": { + "save": { "notes": ["CONTROL", "S"] }, + "find": { "notes": ["CMD", "F"] } + } + } + ] + """; + await File.WriteAllTextAsync(Path.Combine(_testDataDirectory, Constants.Files.ProfilesFileName), json).ConfigureAwait(false); + + JsonKeybindingRepository repository = new(_testDataDirectory); + Profile? profile = await repository.LoadProfileAsync("p").ConfigureAwait(false); + + Assert.IsNotNull(profile); + Assert.AreEqual(Chord.Parse("Ctrl+S"), profile.GetChord("save")); + Assert.AreEqual(Chord.Parse("Meta+F"), profile.GetChord("find")); + } +} diff --git a/Keybinding/Models/MusicalTypes.cs b/Keybinding/Models/MusicalTypes.cs index 8549fb9..c7af056 100644 --- a/Keybinding/Models/MusicalTypes.cs +++ b/Keybinding/Models/MusicalTypes.cs @@ -18,7 +18,8 @@ public sealed class Note : IEquatable public Note(NoteName key) { Ensure.NotNull(key); - Key = key; + string canonical = CanonicalizeKey(key.ToString()); + Key = canonical == key.ToString() ? key : NoteName.Create(canonical); } /// @@ -33,9 +34,22 @@ public Note(string key) throw new ArgumentException("Key cannot be null or whitespace", nameof(key)); } - Key = NoteName.Create(key.Trim().ToUpperInvariant()); + Key = NoteName.Create(CanonicalizeKey(key.Trim().ToUpperInvariant())); } + /// + /// Maps a modifier alias to its canonical key name, so that every way of building a note + /// (parsing, constructing directly, or loading a stored profile) compares and hashes the same. + /// + /// An uppercase key name + /// The canonical key name + private static string CanonicalizeKey(string key) => key switch + { + "CONTROL" => "CTRL", + "WIN" or "WINDOWS" or "CMD" or "COMMAND" => "META", + _ => key + }; + /// /// Gets the key that this note represents /// @@ -274,15 +288,8 @@ public static Chord Parse(string value) foreach (string part in parts) { - // Normalize modifier key names for consistency - string normalizedPart = part.ToUpperInvariant() switch - { - "CONTROL" => "CTRL", - "WIN" or "WINDOWS" or "CMD" or "COMMAND" => "META", - _ => part.ToUpperInvariant() - }; - - notes.Add(new Note(normalizedPart)); + // The Note constructor normalizes modifier aliases such as "Control" and "Cmd" + notes.Add(new Note(part)); } return new Chord(notes);