Merge pull request #264 from dodexahedron/convert-tests-to-constraint…

…-model

Unit test conversion and TGD modernization round 2

.editorconfig

@@ -1,7 +1,3 @@
 root = true
 
-[*Keys.yaml]
-end_of_line = lf
 
-[*KeyMapTests.cs]
-end_of_line = lf

src/ColorSchemeManager.cs

@@ -61,7 +61,7 @@ public void Remove(NamedColorScheme toDelete)
     /// schemes.
     /// </summary>
     /// <param name="viewBeingEdited">View to find color schemes in, must be the root design (i.e. <see cref="Design.IsRoot"/>).</param>
-    /// <exception cref="ArgumentException">Thrown if passed a non root <see cref="Design"/>.</exception>
+    /// <exception cref="ArgumentException">Thrown if passed a non-root <see cref="Design"/>.</exception>
     public void FindDeclaredColorSchemes(Design viewBeingEdited)
     {
         if (!viewBeingEdited.IsRoot)

src/Design.cs

@@ -390,47 +390,54 @@ public IEnumerable<IOperation> GetExtraOperations(Point pos)
 
         yield return new DeleteViewOperation(this);
 
-        if (this.View is TabView tabView)
+        switch ( this.View )
         {
-            yield return new AddTabOperation(this, null);
-
-            if (tabView.SelectedTab != null)
+            case TabView tabView:
             {
-                yield return new RemoveTabOperation(this, tabView.SelectedTab);
-                yield return new RenameTabOperation(this, tabView.SelectedTab, null);
-                yield return new MoveTabOperation(this, tabView.SelectedTab, -1);
-                yield return new MoveTabOperation(this, tabView.SelectedTab, 1);
+                yield return new AddTabOperation(this, null);
+
+                if (tabView.SelectedTab != null)
+                {
+                    yield return new RemoveTabOperation(this, tabView.SelectedTab);
+                    yield return new RenameTabOperation(this, tabView.SelectedTab, null);
+                    yield return new MoveTabOperation(this, tabView.SelectedTab, -1);
+                    yield return new MoveTabOperation(this, tabView.SelectedTab, 1);
+                }
+
+                break;
             }
-        }
+            case MenuBar mb:
+            {
+                yield return new AddMenuOperation(this, null);
 
-        if (this.View is MenuBar mb)
-        {
-            yield return new AddMenuOperation(this, null);
+                var menu = pos.IsEmpty ? mb.GetSelectedMenuItem() : mb.ScreenToMenuBarItem(pos.X);
 
-            var menu = pos.IsEmpty ? mb.GetSelectedMenuItem() : mb.ScreenToMenuBarItem(pos.X);
+                if (menu != null)
+                {
+                    yield return new RemoveMenuOperation(this, menu);
+                    yield return new RenameMenuOperation(this, menu, null);
+                    yield return new MoveMenuOperation(this, menu, -1);
+                    yield return new MoveMenuOperation(this, menu, 1);
+                }
 
-            if (menu != null)
-            {
-                yield return new RemoveMenuOperation(this, menu);
-                yield return new RenameMenuOperation(this, menu, null);
-                yield return new MoveMenuOperation(this, menu, -1);
-                yield return new MoveMenuOperation(this, menu, 1);
+                break;
             }
-        }
+            case StatusBar sb:
+            {
+                yield return new AddStatusItemOperation(this, null);
 
-        if (this.View is StatusBar sb)
-        {
-            yield return new AddStatusItemOperation(this, null);
+                var item = sb.ScreenToMenuBarItem(pos.X);
 
-            var item = sb.ScreenToMenuBarItem(pos.X);
+                if (item != null)
+                {
+                    yield return new RemoveStatusItemOperation(this, item);
+                    yield return new RenameStatusItemOperation(this, item, null);
+                    yield return new SetShortcutOperation(this, item, null);
+                    yield return new MoveStatusItemOperation(this, item, -1);
+                    yield return new MoveStatusItemOperation(this, item, 1);
+                }
 
-            if (item != null)
-            {
-                yield return new RemoveStatusItemOperation(this, item);
-                yield return new RenameStatusItemOperation(this, item, null);
-                yield return new SetShortcutOperation(this, item, null);
-                yield return new MoveStatusItemOperation(this, item, -1);
-                yield return new MoveStatusItemOperation(this, item, 1);
+                break;
             }
         }
     }
@@ -620,7 +627,7 @@ private void RegisterCheckboxDesignTimeChanges(CheckBox cb)
     {
         // prevent space toggling the checkbox
         // (gives better typing experience e.g. "my lovely checkbox")
-        cb.ClearKeyBinding(Key.Space);
+        cb.KeyBindings.Remove(Key.Space);
         cb.MouseClick += (s, e) =>
         {
             if (e.MouseEvent.Flags.HasFlag(MouseFlags.Button1Clicked))

src/DimExtensions.cs

@@ -237,6 +237,8 @@ public static bool GetDimType(this Dim d, out DimType type, out float value, out
     {
         if (!d.GetDimType(out var type, out var val, out var offset))
         {
+            // TODO: This is currently unreachable (though a TG change could make it not so)
+            // Would it maybe be a better idea to throw an exception, so generated code isn't silently incorrect?
             // could not determine the type
             return null;
         }

src/Keys.yaml

@@ -1,28 +1,28 @@
 EditProperties: F4
 ShowContextMenu: Enter
-ViewSpecificOperations: ShiftMask, F4
+ViewSpecificOperations: Shift+F4
 EditRootProperties: F5
 ShowHelp: F1
-New: N, CtrlMask
-Open: O, CtrlMask
-Save: S, CtrlMask
-Redo: Y, CtrlMask
-Undo: Z, CtrlMask
-Delete: DeleteChar
+New: Ctrl+N
+Open: Ctrl+O
+Save: Ctrl+S
+Redo: Ctrl+Y
+Undo: Ctrl+Z
+Delete: Delete
 ToggleDragging: F3
 AddView: F2
-ToggleShowFocused: L, CtrlMask
-ToggleShowBorders: B, CtrlMask
+ToggleShowFocused: Ctrl+L
+ToggleShowBorders: Ctrl+B
 RightClick: Button3Clicked
-Copy: C, CtrlMask
-Paste: V, CtrlMask
-Rename: R, CtrlMask
-SetShortcut: T, CtrlMask
-SelectAll: A, CtrlMask
-MoveRight: CursorRight, ShiftMask
-MoveLeft: CursorLeft, ShiftMask
-MoveUp: CursorUp, ShiftMask
-MoveDown: CursorDown, ShiftMask
+Copy: Ctrl+C
+Paste: Ctrl+V
+Rename: Ctrl+R
+SetShortcut: Ctrl+T
+SelectAll: Ctrl+A
+MoveRight: Shift+CursorRight
+MoveLeft: Shift+CursorLeft
+MoveUp: Shift+CursorUp
+MoveDown: Shift+CursorDown
 ShowColorSchemes: F6
 SelectionColor:
   NormalForeground: BrightGreen

src/MenuBarExtensions.cs

@@ -1,4 +1,3 @@
-using System.Reflection;
 using Terminal.Gui;
 
 namespace TerminalGuiDesigner;
@@ -17,7 +16,7 @@ public static class MenuBarExtensions
     /// <returns>Selected <see cref="MenuItem"/> or null if none.</returns>
     public static MenuBarItem? GetSelectedMenuItem(this MenuBar menuBar)
     {
-        var selected = (int)GetNonNullPrivateFieldValue("selected", menuBar, typeof(MenuBar));
+        int selected = menuBar.GetNonNullNonPublicFieldValue<int, MenuBar>( "selected" );
 
         if (selected < 0 || selected >= menuBar.Menus.Length)
         {
@@ -36,6 +35,7 @@ public static class MenuBarExtensions
     public static MenuBarItem? ScreenToMenuBarItem(this MenuBar menuBar, int screenX)
     {
         // These might be changed in Terminal.Gui library
+        // TODO: Maybe load these from a config file, so we aren't at TG's mercy
         const int initialWhitespace = 1;
         const int afterEachItemWhitespace = 2;
 
@@ -75,29 +75,4 @@ public static class MenuBarExtensions
         // Return the last menu item that begins rendering before this X point
         return menuXLocations.Last(m => m.Key <= clientPoint.X).Value;
     }
-
-    /// <summary>
-    /// Changes the <see cref="StatusItem.Shortcut"/> even though it has no setter in Terminal.Gui.
-    /// </summary>
-    /// <param name="item"><see cref="StatusItem"/> to change <see cref="StatusItem.Shortcut"/> on.</param>
-    /// <param name="newShortcut">The new value for <see cref="StatusItem.Shortcut"/>.</param>
-    public static void SetShortcut(this StatusItem item, Key newShortcut)
-    {
-        // See: https://stackoverflow.com/a/40917899/4824531
-        const string backingFieldName = "<Shortcut>k__BackingField";
-
-        var field =
-            typeof(StatusItem).GetField(backingFieldName, BindingFlags.Instance | BindingFlags.NonPublic)
-            ?? throw new Exception($"Could not find auto backing field '{backingFieldName}'");
-
-        field.SetValue(item, newShortcut);
-    }
-
-    private static object GetNonNullPrivateFieldValue(string fieldName, object item, Type type)
-    {
-        var selectedField = type.GetField(fieldName, BindingFlags.NonPublic | BindingFlags.Instance)
-            ?? throw new Exception($"Expected private field {fieldName} was not present on {type.Name}");
-        return selectedField.GetValue(item)
-            ?? throw new Exception($"Private field {fieldName} was unexpectedly null on {type.Name}");
-    }
 }

src/MenuTracker.cs

@@ -1,4 +1,6 @@
-using Terminal.Gui;
+using System.Collections.Concurrent;
+using System.Diagnostics.CodeAnalysis;
+using Terminal.Gui;
 
 namespace TerminalGuiDesigner;
 
@@ -8,7 +10,7 @@ namespace TerminalGuiDesigner;
 /// </summary>
 public class MenuTracker
 {
-    private readonly HashSet<MenuBar> bars = new();
+    private readonly ConcurrentBag<MenuBar> bars = new( );
 
     private MenuTracker()
     {
@@ -58,9 +60,10 @@ public void Register(MenuBar mb)
     /// <param name="item">The item whose parent you want to find.</param>
     /// <param name="hostBar">The <see cref="MenuBar"/> that owns <paramref name="item"/> or.
     /// null if not found or parent not registered (see <see cref="Register(MenuBar)"/>).</param>
-    /// <returns>The immediate parent of <paramref name="item"/>.  May be a top level menu (e.g. File, View)
-    /// or a sub-menu parent (e.g. View=>Windows).</returns>
-    public MenuBarItem? GetParent(MenuItem item, out MenuBar? hostBar)
+    /// <returns>The immediate parent of <paramref name="item"/>.</returns>
+    /// <remarks>Result may be a top level menu (e.g. File, View)
+    /// or a sub-menu parent (e.g. View=>Windows).</remarks>
+    public MenuBarItem? GetParent( MenuItem item, out MenuBar? hostBar )
     {
         foreach (var bar in this.bars)
         {
@@ -80,6 +83,20 @@ public void Register(MenuBar mb)
         return null;
     }
 
+    public bool TryGetParent(MenuItem item, [NotNullWhen(true)]out MenuBar? hostBar, [NotNullWhen(true)] out MenuBarItem? parentItem)
+    {
+        var parentCandidate = GetParent( item, out hostBar );
+        if ( parentCandidate is null )
+        {
+            hostBar = null;
+            parentItem = null;
+            return false;
+        }
+
+        parentItem = parentCandidate;
+        return true;
+    }
+
     /// <summary>
     /// Iterates all menus (e.g. 'File F9', 'View' etc) of a MenuBar and
     /// identifies any entries that have empty sub-menus (MenuBarItem).
@@ -120,7 +137,7 @@ public Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus()
     /// <param name="added">The result of the conversion (same text, same index etc but
     /// <see cref="MenuItem"/> instead of <see cref="MenuBarItem"/>).</param>
     /// <returns><see langword="true"/> if conversion was possible (menu was empty and belonged to tracked menu).</returns>
-    public bool ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem bar, out MenuItem? added)
+    internal static bool ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem bar, out MenuItem? added)
     {
         added = null;
 
@@ -130,9 +147,7 @@ public bool ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem bar, out MenuItem
             return false;
         }
 
-        var parent = MenuTracker.Instance.GetParent(bar, out _);
-
-        if (parent == null)
+        if ( !Instance.TryGetParent( bar, out _, out MenuBarItem? parent ) )
         {
             return false;
         }
@@ -166,7 +181,7 @@ private Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus(MenuBar bar, MenuBar
         foreach (var c in mbi.Children.OfType<MenuBarItem>())
         {
             this.ConvertEmptyMenus(bar, c);
-            if (this.ConvertMenuBarItemToRegularItemIfEmpty(c, out var added))
+            if ( ConvertMenuBarItemToRegularItemIfEmpty( c, out var added))
             {
                 if (added != null)
                 {

src/Operations/MenuOperations/MenuItemOperation.cs

@@ -16,11 +16,9 @@ protected MenuItemOperation(MenuItem operateOn)
     {
         // if taking a new line add an extra menu item
         // menuItem.Parent doesn't work for root menu items
-        var parent = MenuTracker.Instance.GetParent(operateOn, out var bar);
-
-        if (parent == null || bar == null)
+        if ( !MenuTracker.Instance.TryGetParent( operateOn, out MenuBar? bar, out MenuBarItem? parent ) )
         {
-            this.IsImpossible = true;
+            IsImpossible = true;
             return;
         }
 

src/Operations/MenuOperations/MoveMenuItemLeftOperation.cs

@@ -66,9 +66,7 @@ protected override bool DoImpl()
             return false;
         }
 
-        var parentsParent = MenuTracker.Instance.GetParent(this.Parent, out var bar);
-
-        if (parentsParent == null)
+        if ( !MenuTracker.Instance.TryGetParent( Parent, out _, out MenuBarItem? parentsParent ) )
         {
             return false;
         }
@@ -83,7 +81,7 @@ protected override bool DoImpl()
         if (new RemoveMenuItemOperation(this.OperateOn).Do())
         {
             // We are the parent but parents children don't contain
-            // us.  Thats bad. TODO: log this
+            // us.  That's bad. TODO: log this
             if (parentsIdx == -1)
             {
                 return false;

src/Operations/MenuOperations/RemoveMenuItemOperation.cs

@@ -77,15 +77,13 @@ public override void Undo()
         {
             foreach (var converted in this.convertedMenuBars)
             {
-                var grandparent = MenuTracker.Instance.GetParent(converted.Value, out _);
-                if (grandparent != null)
+                if(MenuTracker.Instance.TryGetParent(converted.Value,out _, out MenuBarItem? grandparent))
                 {
-                    var popIdx = Array.IndexOf(grandparent.Children, converted.Value);
-                    var newParents = grandparent.Children.ToList<MenuItem>();
-                    newParents.RemoveAt(popIdx);
-                    newParents.Insert(popIdx, converted.Key);
-
-                    grandparent.Children = newParents.ToArray();
+                    int replacementIndex = Array.IndexOf(grandparent.Children, converted.Value);
+                    if(replacementIndex >=0 && replacementIndex < grandparent.Children.Length)
+                    {
+                        grandparent.Children[ replacementIndex ] = converted.Key;
+                    }
                 }
             }
         }

src/Operations/StatusBarOperations/AddStatusItemOperation.cs

@@ -18,7 +18,7 @@ public AddStatusItemOperation(Design design, string? name)
                   (d) => d.Items,
                   (d, v) => d.Items = v,
                   (v) => v.Title.ToString() ?? Operation.Unnamed,
-                  (d, name) => { return new StatusItem(Key.Null, name, null); },
+                  (d, name) => { return new StatusItem(KeyCode.Null, name, null); },
                   design,
                   name)
         {

src/Operations/StatusBarOperations/SetShortcutOperation.cs

@@ -1,4 +1,4 @@
-using Terminal.Gui;
+using Terminal.Gui;
 using TerminalGuiDesigner.Operations.Generics;
 using TerminalGuiDesigner.UI.Windows;
 
@@ -39,13 +39,13 @@ public override void Redo()
                 return;
             }
 
-            this.OperateOn.SetShortcut(this.shortcut.Value);
+            this.OperateOn.Shortcut = this.shortcut;
         }
 
         /// <inheritdoc/>
         public override void Undo()
         {
-            this.OperateOn.SetShortcut(this.originalShortcut);
+            this.OperateOn.Shortcut = this.originalShortcut;
         }
 
         /// <inheritdoc/>
@@ -56,7 +56,7 @@ protected override bool DoImpl()
                 this.shortcut = Modals.GetShortcut();
             }
 
-            this.OperateOn.SetShortcut(this.shortcut.Value);
+            this.OperateOn.Shortcut = this.shortcut;
             return true;
         }
     }