diff --git a/Schema.Editor.Test/EditorShortcutTests.cs b/Schema.Editor.Test/EditorShortcutTests.cs new file mode 100644 index 0000000..29dc98f --- /dev/null +++ b/Schema.Editor.Test/EditorShortcutTests.cs @@ -0,0 +1,178 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Schema.Editor.Test; + +using System; +using System.Collections.Generic; +using System.Linq; + +/// +/// The shortcut table: that a chord selects the command it is meant to, and that the menu's label +/// for a command is the chord that actually fires it. +/// +/// +/// +/// The chords used to be written twice - an if/else if chain for dispatch, and +/// literal strings such as "Ctrl+Shift+S" beside the menu items - with nothing holding the two +/// together. is the test that pins the join: +/// it reads the label the File and Edit menus now draw and checks it against the chord the +/// dispatcher matches, so rebinding a shortcut in one place and advertising the other cannot pass. +/// +/// +/// Frameless, like the rest of this suite. EditorShortcuts takes the pressed key as a name +/// rather than an ImGuiKey precisely so the table can be exercised without a rasterizer; +/// that the editor feeds it ImGui's real key state is covered by +/// Schema.Editor.UITests/ShortcutTests, which presses the chords into a live frame. +/// +/// +[TestClass] +public sealed class EditorShortcutTests +{ + private EditorShortcuts shortcuts = null!; + + [TestInitialize] + public void BuildShortcuts() => shortcuts = new EditorShortcuts(); + + /// + /// Presses a chord: the named key, with the modifiers the chord carries. + /// + private string? Press(string key, bool ctrl = true, bool shift = false, bool alt = false) => + shortcuts.FindCommand(ctrl, shift, alt, pressed => string.Equals(pressed, key, StringComparison.OrdinalIgnoreCase)); + + /// + /// Every command the menus label, against the chord the menu is expected to show for it. + /// + private static IEnumerable<(string CommandId, string Label)> LabelledCommands => + [ + (EditorShortcuts.New, "Ctrl+N"), + (EditorShortcuts.Open, "Ctrl+O"), + (EditorShortcuts.Save, "Ctrl+S"), + (EditorShortcuts.SaveAs, "Ctrl+Shift+S"), + (EditorShortcuts.Undo, "Ctrl+Z"), + (EditorShortcuts.Redo, "Ctrl+Y"), + ]; + + /// + /// The label the menu draws is the chord that fires the command, not a string that merely + /// resembles it. This is the duplication the shortcut table exists to remove. + /// + [TestMethod] + public void MenuLabelIsTheChordThatFiresTheCommand() + { + foreach ((string commandId, string label) in LabelledCommands) + { + Assert.AreEqual(label, shortcuts.Label(commandId), $"The menu label for {commandId} is not the chord bound to it."); + } + } + + /// + /// And the chord the label names is the one that actually selects that command, so the two + /// cannot drift apart in the other direction either. + /// + [TestMethod] + public void TheChordOnTheLabelSelectsThatCommand() + { + foreach ((string commandId, _) in LabelledCommands) + { + string label = shortcuts.Label(commandId); + string[] notes = label.Split('+'); + string key = notes[^1]; + + string? selected = Press( + key, + ctrl: notes.Contains("Ctrl", StringComparer.OrdinalIgnoreCase), + shift: notes.Contains("Shift", StringComparer.OrdinalIgnoreCase), + alt: notes.Contains("Alt", StringComparer.OrdinalIgnoreCase)); + + Assert.AreEqual(commandId, selected, $"Pressing {label}, the chord the menu shows for {commandId}, did not select it."); + } + } + + /// + /// Save As shares its key with Save, so the Shift has to be what tells them apart. Under the old + /// chain this held only because the Save As arm was written first. + /// + [TestMethod] + public void ShiftDistinguishesSaveAsFromSave() + { + Assert.AreEqual(EditorShortcuts.Save, Press("S")); + Assert.AreEqual(EditorShortcuts.SaveAs, Press("S", shift: true)); + } + + /// + /// Undo and Redo share Z the same way. + /// + [TestMethod] + public void ShiftDistinguishesRedoFromUndo() + { + Assert.AreEqual(EditorShortcuts.Undo, Press("Z")); + Assert.AreEqual(EditorShortcuts.RedoAlternate, Press("Z", shift: true)); + } + + /// + /// Ctrl+Y redoes as well, which is why Redo has a second binding rather than a second chord. + /// + [TestMethod] + public void CtrlYIsTheOtherRedo() + { + Assert.AreEqual(EditorShortcuts.Redo, Press("Y")); + Assert.AreEqual("Ctrl+Y", shortcuts.Label(EditorShortcuts.Redo)); + } + + /// + /// A chord has to match in every modifier, not only the ones a branch thought to test. + /// + /// + /// This is the one deliberate behaviour change. The old chain asked ctrl && + /// IsKeyPressed(N) and said nothing about Shift or Alt, so Ctrl+Shift+N and Ctrl+Alt+N both + /// started a new document. Neither is a chord the editor advertises, and neither fires now. + /// + [TestMethod] + public void AnExtraModifierIsNotTheSameChord() + { + Assert.AreEqual(EditorShortcuts.New, Press("N")); + Assert.IsNull(Press("N", shift: true), "Ctrl+Shift+N is not a chord the editor binds."); + Assert.IsNull(Press("N", alt: true), "Ctrl+Alt+N is not a chord the editor binds."); + Assert.IsNull(Press("O", shift: true), "Ctrl+Shift+O is not a chord the editor binds."); + } + + /// + /// A key with no Ctrl is ordinary typing, not a shortcut. + /// + [TestMethod] + public void AKeyWithoutItsModifierSelectsNothing() + { + Assert.IsNull(Press("N", ctrl: false)); + Assert.IsNull(Press("S", ctrl: false)); + } + + /// + /// A key no shortcut is built around never selects anything, whatever is held with it. + /// + [TestMethod] + public void AnUnboundKeySelectsNothing() + { + Assert.IsNull(Press("Q")); + Assert.IsNull(Press("Q", shift: true)); + } + + /// + /// No two commands answer to the same chord, or which one fires would come down to the order + /// they happen to be registered in - the ambiguity the else if chain had to be read + /// carefully to rule out. + /// + [TestMethod] + public void NoChordFiresTwoCommands() + { + string[] commandIds = + [ + EditorShortcuts.New, EditorShortcuts.Open, EditorShortcuts.Save, EditorShortcuts.SaveAs, + EditorShortcuts.Undo, EditorShortcuts.Redo, EditorShortcuts.RedoAlternate, + ]; + + List chords = [.. commandIds.Select(shortcuts.Label)]; + + CollectionAssert.AllItemsAreUnique(chords, $"Two commands share a chord: {string.Join(", ", chords)}"); + Assert.IsFalse(chords.Any(string.IsNullOrEmpty), "A command reached the menu with no chord bound to it."); + } +} diff --git a/Schema.Editor/EditorShortcuts.cs b/Schema.Editor/EditorShortcuts.cs new file mode 100644 index 0000000..981a097 --- /dev/null +++ b/Schema.Editor/EditorShortcuts.cs @@ -0,0 +1,186 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Schema.Editor; + +using System; +using System.Collections.Generic; +using System.Linq; + +using ktsu.Keybinding.Core.Contracts; +using ktsu.Keybinding.Core.Models; +using ktsu.Keybinding.Core.Services; + +/// +/// The editor's keyboard shortcuts, held once so that dispatch and the menu labels cannot disagree. +/// +/// +/// +/// The chords used to be written down twice: once as an if/else if chain over ImGui's +/// key state, and again as literal strings beside the menu items ("Ctrl+Shift+S" and friends). +/// Nothing tied the two together, so a chord could be rebound in one place and go on being +/// advertised as the other. is now the only place a chord is written, and +/// renders the menu's text from the same binding that fires the command. +/// +/// +/// The matching itself is ktsu.Keybinding.Core's, which this project already referenced and +/// did not use. Its equality is insensitive to the order the modifiers are +/// written in and to case, and renders a chord in the same spelling +/// the menu used to hard-code - so adopting it leaves the menu reading exactly as it did. +/// +/// +/// Nothing here is persisted. is built over an in-memory +/// and , so no repository is involved and +/// the editor never reads or writes a keybinding file. +/// +/// +/// One behaviour did change, deliberately. The old chain tested the modifiers it cared about and +/// ignored the rest, so Ctrl+Shift+N started a new document just as Ctrl+N did, and Ctrl+Alt+O +/// opened one. A chord now has to match exactly, so those no longer fire. The chords that were +/// always meant to be distinguished - Ctrl+S against Ctrl+Shift+S, Ctrl+Z against Ctrl+Shift+Z - +/// still are, and no longer depend on the order the else if arms happen to be written in. +/// +/// +internal sealed class EditorShortcuts +{ + /// Start a new document. + internal const string New = "editor.file.new"; + + /// Open an existing document. + internal const string Open = "editor.file.open"; + + /// Write the open document to its current path. + internal const string Save = "editor.file.save"; + + /// Ask where to write a copy of the open document. + internal const string SaveAs = "editor.file.save-as"; + + /// Undo the last edit. + internal const string Undo = "editor.edit.undo"; + + /// Redo the last undone edit. + internal const string Redo = "editor.edit.redo"; + + /// + /// Redo, under the other chord that has always done it. Kept as its own command because a + /// binding holds one chord, and Ctrl+Y is the one the Edit menu advertises. + /// + internal const string RedoAlternate = "editor.edit.redo-alternate"; + + private const string ProfileId = "default"; + + private static readonly string[] ModifierNotes = ["CTRL", "SHIFT", "ALT", "META"]; + + /// + /// Every shortcut the editor has, and the only place each chord is written. + /// + private static readonly (string CommandId, string Name, string Chord)[] Bindings = + [ + (New, "New", "Ctrl+N"), + (Open, "Open", "Ctrl+O"), + (Save, "Save", "Ctrl+S"), + (SaveAs, "Save As", "Ctrl+Shift+S"), + (Undo, "Undo", "Ctrl+Z"), + (Redo, "Redo", "Ctrl+Y"), + (RedoAlternate, "Redo", "Ctrl+Shift+Z"), + ]; + + private readonly KeybindingService keybindings; + + /// + /// The keys a chord can be built around, each named as ktsu.Keybinding.Core spells it. + /// Derived from so that adding a shortcut needs no second edit here. + /// + private readonly string[] primaryKeys; + + /// + /// Registers the editor's commands and binds each to its chord. + /// + internal EditorShortcuts() + { + CommandRegistry commands = new(); + ProfileManager profiles = new(); + keybindings = new KeybindingService(commands, profiles); + + keybindings.CreateProfile(ProfileId, "Default", "The editor's built-in shortcuts"); + keybindings.SetActiveProfile(ProfileId); + + foreach ((string commandId, string name, string chord) in Bindings) + { + commands.RegisterCommand(new Command(commandId, name, name, "Editor")); + keybindings.BindChord(commandId, Chord.Parse(chord)); + } + + primaryKeys = [.. Bindings + .Select(binding => Chord.Parse(binding.Chord)) + .SelectMany(chord => chord.Notes) + .Select(note => note.ToString()) + .Where(note => !ModifierNotes.Contains(note, StringComparer.OrdinalIgnoreCase)) + .Distinct(StringComparer.OrdinalIgnoreCase)]; + } + + /// + /// The text the menu shows beside the item that names. + /// + /// The command whose chord to render. + /// The chord's spelling, or an empty string if the command has no chord bound. + internal string Label(string commandId) => keybindings.GetChord(commandId)?.ToString() ?? string.Empty; + + /// + /// The command whose chord is being pressed, if any. + /// + /// Whether Control is held. + /// Whether Shift is held. + /// Whether Alt is held. + /// + /// Answers whether the named key was pressed on this frame. Named rather than typed as an ImGui + /// key so that this can be exercised without a frame to press keys into. + /// + /// The command's id, or if no bound chord matches. + /// + /// The modifiers are taken as given and the pressed key is looked for among the keys the + /// bindings actually use, so a chord is only ever built for a key some shortcut wants. Matching + /// the result is left to , which is + /// what makes the match exact in the modifiers rather than merely in the ones a branch thought + /// to test. + /// + internal string? FindCommand(bool ctrl, bool shift, bool alt, Func isKeyPressed) + { + Ensure.NotNull(isKeyPressed); + + foreach (string key in primaryKeys) + { + if (!isKeyPressed(key)) + { + continue; + } + + List notes = []; + + if (ctrl) + { + notes.Add(new Note("CTRL")); + } + + if (shift) + { + notes.Add(new Note("SHIFT")); + } + + if (alt) + { + notes.Add(new Note("ALT")); + } + + notes.Add(new Note(key)); + + string? commandId = keybindings.FindCommandByChord(new Chord(notes)); + + if (commandId is not null) + { + return commandId; + } + } + + return null; + } +} diff --git a/Schema.Editor/SchemaEditor.cs b/Schema.Editor/SchemaEditor.cs index a634ebe..78610eb 100644 --- a/Schema.Editor/SchemaEditor.cs +++ b/Schema.Editor/SchemaEditor.cs @@ -37,6 +37,12 @@ public partial class SchemaEditor private ImGuiWidgets.DividerContainer DividerContainerCols { get; init; } internal IUndoRedoService UndoRedo { get; } + + /// + /// The keyboard shortcuts, read both to dispatch a chord and to label the menu items it fires. + /// + internal EditorShortcuts Shortcuts { get; } = new(); + internal Popups Popups { get; } private TreeSchema TreeSchema { get; init; } private CodeGeneratorPanel CodeGeneratorPanel { get; init; } @@ -192,42 +198,43 @@ private void ProcessKeyboardShortcuts() return; } - bool ctrl = io.KeyCtrl; - bool shift = io.KeyShift; - - if (ctrl && ImGui.IsKeyPressed(ImGuiKey.Z, false)) + switch (Shortcuts.FindCommand(io.KeyCtrl, io.KeyShift, io.KeyAlt, WasKeyPressed)) { - if (shift) - { - Redo(); - } - else - { + case EditorShortcuts.New: + New(); + break; + case EditorShortcuts.Open: + Open(); + break; + case EditorShortcuts.Save: + Save(); + break; + case EditorShortcuts.SaveAs: + SaveAs(); + break; + case EditorShortcuts.Undo: Undo(); - } - } - else if (ctrl && ImGui.IsKeyPressed(ImGuiKey.Y, false)) - { - Redo(); - } - else if (ctrl && shift && ImGui.IsKeyPressed(ImGuiKey.S, false)) - { - SaveAs(); - } - else if (ctrl && ImGui.IsKeyPressed(ImGuiKey.S, false)) - { - Save(); - } - else if (ctrl && ImGui.IsKeyPressed(ImGuiKey.N, false)) - { - New(); - } - else if (ctrl && ImGui.IsKeyPressed(ImGuiKey.O, false)) - { - Open(); + break; + case EditorShortcuts.Redo: + case EditorShortcuts.RedoAlternate: + Redo(); + break; + default: + break; } } + /// + /// Whether the key ktsu.Keybinding.Core names was pressed on this frame. + /// + /// + /// The library spells the keys a chord is built from ("N", "S") exactly as ImGui names them in + /// , so the two meet at a parse. A key the enum does not know counts as + /// not pressed, rather than throwing on every frame. + /// + private static bool WasKeyPressed(string key) => + Enum.TryParse(key, ignoreCase: true, out ImGuiKey imGuiKey) && ImGui.IsKeyPressed(imGuiKey, false); + internal void OnRender(float dt) { // Stashed for the parameterless tab content delegates (the Class Graph needs the frame delta). @@ -328,12 +335,12 @@ private void ShowFileMenu() return; } - if (MenuItem("New", "Ctrl+N")) + if (MenuItem("New", Shortcuts.Label(EditorShortcuts.New))) { New(); } - if (MenuItem("Open", "Ctrl+O")) + if (MenuItem("Open", Shortcuts.Label(EditorShortcuts.Open))) { Open(); } @@ -342,14 +349,14 @@ private void ShowFileMenu() ImGui.Separator(); - if (MenuItem("Save", "Ctrl+S", CurrentSchema is not null)) + if (MenuItem("Save", Shortcuts.Label(EditorShortcuts.Save), CurrentSchema is not null)) { Save(); } // Always available while a schema is open: without it there is no way to save a copy // somewhere else once the schema has a path. - if (MenuItem("Save As...", "Ctrl+Shift+S", CurrentSchema is not null)) + if (MenuItem("Save As...", Shortcuts.Label(EditorShortcuts.SaveAs), CurrentSchema is not null)) { SaveAs(); } @@ -383,12 +390,12 @@ private void ShowEditMenu() return; } - if (MenuItem("Undo", "Ctrl+Z", UndoRedo.CanUndo)) + if (MenuItem("Undo", Shortcuts.Label(EditorShortcuts.Undo), UndoRedo.CanUndo)) { Undo(); } - if (MenuItem("Redo", "Ctrl+Y", UndoRedo.CanRedo)) + if (MenuItem("Redo", Shortcuts.Label(EditorShortcuts.Redo), UndoRedo.CanRedo)) { Redo(); }