Fixes #3652. Setting Menus causes unexpected Exception.

Terminal.Gui/Application/Application.Keyboard.cs

@@ -9,7 +9,6 @@
 
     /// <summary>Alternative key to navigate forwards through views. Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key NextTabKey
     {
         get => _nextTabKey;
@@ -27,7 +26,6 @@
 
     /// <summary>Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key PrevTabKey
     {
         get => _prevTabKey;
@@ -45,7 +43,6 @@
 
     /// <summary>Alternative key to navigate forwards through views. Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key NextTabGroupKey
     {
         get => _nextTabGroupKey;
@@ -63,7 +60,6 @@
 
     /// <summary>Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key PrevTabGroupKey
     {
         get => _prevTabGroupKey;
@@ -81,7 +77,6 @@
 
     /// <summary>Gets or sets the key to quit the application.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key QuitKey
     {
         get => _quitKey;
@@ -299,7 +294,7 @@
     /// <param name="f">The function.</param>
     private static void AddCommand (Command command, Func<bool?> f) { CommandImplementations [command] = ctx => f (); }
 
     static Application () { AddApplicationKeyBindings (); }
 
     internal static void AddApplicationKeyBindings ()
     {

Terminal.Gui/Input/Key.cs

@@ -1,5 +1,6 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
+using System.Text.Json.Serialization;
 
 namespace Terminal.Gui;
 
@@ -450,7 +451,7 @@ public override bool Equals (object obj)
 
     /// <summary>Pretty prints the KeyEvent</summary>
     /// <returns></returns>
-    public override string ToString () { return ToString (KeyCode, (Rune)'+'); }
+    public override string ToString () { return ToString (KeyCode, Separator); }
 
     private static string GetKeyString (KeyCode key)
     {
@@ -483,7 +484,7 @@ private static string GetKeyString (KeyCode key)
     ///     The formatted string. If the key is a printable character, it will be returned as a string. Otherwise, the key
     ///     name will be returned.
     /// </returns>
-    public static string ToString (KeyCode key) { return ToString (key, (Rune)'+'); }
+    public static string ToString (KeyCode key) { return ToString (key, Separator); }
 
     /// <summary>Formats a <see cref="KeyCode"/> as a string.</summary>
     /// <param name="key">The key to format.</param>
@@ -584,7 +585,7 @@ public static bool TryParse (string text, [NotNullWhen (true)] out Key key)
         key = null;
 
         // Split the string into parts
-        string [] parts = text.Split ('+', '-');
+        string [] parts = text.Split ('+', '-', (char)Separator.Value);
 
         if (parts.Length is 0 or > 4 || parts.Any (string.IsNullOrEmpty))
         {
@@ -971,4 +972,20 @@ out parsedInt
     public static Key F24 => new (KeyCode.F24);
 
     #endregion
+
+    private static Rune _separator = new ('+');
+
+    /// <summary>Sets or gets the shortcut delimiter separator. The default is "+".</summary>
+    [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
+    public static Rune Separator
+    {
+        get => _separator;
+        set
+        {
+            if (_separator != value)
+            {
+                _separator = value == default (Rune) ? new ('+') : value;
+            }
+        }
+    }
 }

Terminal.Gui/Input/ShortcutHelper.cs

@@ -23,7 +23,7 @@ public virtual KeyCode Shortcut
     }
 
     /// <summary>The keystroke combination used in the <see cref="Shortcut"/> as string.</summary>
-    public virtual string ShortcutTag => Key.ToString (shortcut, MenuBar.ShortcutDelimiter);
+    public virtual string ShortcutTag => Key.ToString (shortcut, Key.Separator);
 
     /// <summary>Lookup for a <see cref="KeyCode"/> on range of keys.</summary>
     /// <param name="key">The source key.</param>
@@ -59,7 +59,7 @@ public static KeyCode GetShortcutFromTag (string tag, Rune delimiter = default)
         //var hasCtrl = false;
         if (delimiter == default (Rune))
         {
-            delimiter = MenuBar.ShortcutDelimiter;
+            delimiter = Key.Separator;
         }
 
         string [] keys = sCut.Split (delimiter.ToString ());

Terminal.Gui/Resources/config.json

@@ -22,6 +22,7 @@
   "Application.NextTabGroupKey": "F6",
   "Application.PrevTabGroupKey": "Shift+F6",
   "Application.QuitKey": "Esc",
+  "Key.Separator": "+",
 
   "Theme": "Default",
   "Themes": [

Terminal.Gui/Views/Menu/Menu.cs

@@ -120,7 +120,7 @@ _barItems.Children [_currentChild]
                     }
                    );
 
-        AddKeyBindings (_barItems);
+        AddKeyBindingsHotKey (_barItems);
     }
 
     public Menu ()
@@ -179,7 +179,7 @@ public Menu ()
         KeyBindings.Add (Key.Enter, Command.Accept);
     }
 
-    private void AddKeyBindings (MenuBarItem menuBarItem)
+    private void AddKeyBindingsHotKey (MenuBarItem menuBarItem)
     {
         if (menuBarItem is null || menuBarItem.Children is null)
         {
@@ -190,23 +190,30 @@ private void AddKeyBindings (MenuBarItem menuBarItem)
         {
             KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, menuItem);
 
-            if ((KeyCode)menuItem.HotKey.Value != KeyCode.Null)
+            if (menuItem.HotKey != Key.Empty)
             {
-                KeyBindings.Remove ((KeyCode)menuItem.HotKey.Value);
-                KeyBindings.Add ((KeyCode)menuItem.HotKey.Value, keyBinding);
-                KeyBindings.Remove ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask);
-                KeyBindings.Add ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey);
+                KeyBindings.Add (menuItem.HotKey, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey.WithAlt);
+                KeyBindings.Add (menuItem.HotKey.WithAlt, keyBinding);
             }
+        }
+    }
+
+    private void RemoveKeyBindingsHotKey (MenuBarItem menuBarItem)
+    {
+        if (menuBarItem is null || menuBarItem.Children is null)
+        {
+            return;
+        }
 
-            if (menuItem.Shortcut != KeyCode.Null)
+        foreach (MenuItem menuItem in menuBarItem.Children.Where (m => m is { }))
+        {
+            if (menuItem.HotKey != Key.Empty)
             {
-                keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem);
-                KeyBindings.Remove (menuItem.Shortcut);
-                KeyBindings.Add (menuItem.Shortcut, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey);
+                KeyBindings.Remove (menuItem.HotKey.WithAlt);
             }
-
-            MenuBarItem subMenu = menuBarItem.SubMenu (menuItem);
-            AddKeyBindings (subMenu);
         }
     }
 
@@ -910,6 +917,8 @@ internal bool CheckSubMenu ()
 
     protected override void Dispose (bool disposing)
     {
+        RemoveKeyBindingsHotKey (_barItems);
+
         if (Application.Current is { })
         {
             Application.Current.DrawContentComplete -= Current_DrawContentComplete;

Terminal.Gui/Views/Menu/MenuBar.cs

@@ -66,6 +66,8 @@ public class MenuBar : View, IDesignable
     /// <summary>Initializes a new instance of the <see cref="MenuBar"/>.</summary>
     public MenuBar ()
     {
+        MenuItem._menuBar = this;
+
         TabStop = TabBehavior.NoStop;
         X = 0;
         Y = 0;
@@ -122,7 +124,7 @@ public MenuBar ()
                         return true;
                     }
                    );
-        AddCommand (Command.ToggleExpandCollapse, ctx => Select ((int)ctx.KeyBinding?.Context!));
+        AddCommand (Command.ToggleExpandCollapse, ctx => Select (Menus.IndexOf (ctx.KeyBinding?.Context)));
         AddCommand (Command.Select, ctx => Run ((ctx.KeyBinding?.Context as MenuItem)?.Action));
 
         // Default key bindings for this view
@@ -172,19 +174,23 @@ public MenuBarItem [] Menus
             {
                 MenuBarItem menuBarItem = Menus [i];
 
-                if (menuBarItem?.HotKey != default (Rune))
+                if (menuBarItem?.HotKey != Key.Empty)
                 {
-                    KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.Focused, i);
-                    KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value, keyBinding);
-                    keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, i);
-                    KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value | KeyCode.AltMask, keyBinding);
+                    KeyBindings.Remove (menuBarItem!.HotKey);
+                    KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.Focused, menuBarItem);
+                    KeyBindings.Add (menuBarItem!.HotKey, keyBinding);
+                    KeyBindings.Remove (menuBarItem.HotKey.WithAlt);
+                    keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, menuBarItem);
+                    KeyBindings.Add (menuBarItem.HotKey.WithAlt, keyBinding);
                 }
 
-                if (menuBarItem?.Shortcut != KeyCode.Null)
+                if (menuBarItem?.ShortcutKey != Key.Empty)
                 {
                     // Technically this will never run because MenuBarItems don't have shortcuts
-                    KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, i);
-                    KeyBindings.Add (menuBarItem.Shortcut, keyBinding);
+                    // unless the IsTopLevel is true
+                    KeyBindings.Remove (menuBarItem.ShortcutKey);
+                    KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuBarItem);
+                    KeyBindings.Add (menuBarItem.ShortcutKey, keyBinding);
                 }
 
                 menuBarItem?.AddShortcutKeyBindings (this);
@@ -1255,21 +1261,6 @@ public bool UseKeysUpDownAsKeysLeftRight
         }
     }
 
-    private static Rune _shortcutDelimiter = new ('+');
-
-    /// <summary>Sets or gets the shortcut delimiter separator. The default is "+".</summary>
-    public static Rune ShortcutDelimiter
-    {
-        get => _shortcutDelimiter;
-        set
-        {
-            if (_shortcutDelimiter != value)
-            {
-                _shortcutDelimiter = value == default (Rune) ? new ('+') : value;
-            }
-        }
-    }
-
     /// <summary>The specifier character for the hot keys.</summary>
     public new static Rune HotKeySpecifier => (Rune)'_';
 
@@ -1321,6 +1312,10 @@ private bool Select (int index)
         {
             OpenMenu ();
         }
+        else if (Menus [index].IsTopLevel)
+        {
+            Run (Menus [index].Action);
+        }
         else
         {
             Activate (index);
@@ -1766,4 +1761,12 @@ public bool EnableForDesign<TContext> (ref readonly TContext context) where TCon
         ];
         return true;
     }
+
+    /// <inheritdoc />
+    protected override void Dispose (bool disposing)
+    {
+        MenuItem._menuBar = null;
+
+        base.Dispose (disposing);
+    }
 }

Terminal.Gui/Views/Menu/MenuBarItem.cs

@@ -2,7 +2,7 @@ namespace Terminal.Gui;
 
 /// <summary>
 ///     <see cref="MenuBarItem"/> is a menu item on  <see cref="MenuBar"/>. MenuBarItems do not support
-///     <see cref="MenuItem.Shortcut"/>.
+///     <see cref="MenuItem.ShortcutKey"/>.
 /// </summary>
 public class MenuBarItem : MenuItem
 {
@@ -100,11 +100,9 @@ internal void AddShortcutKeyBindings (MenuBar menuBar)
         {
             // For MenuBar only add shortcuts for submenus
 
-            if (menuItem.Shortcut != KeyCode.Null)
+            if (menuItem.ShortcutKey != Key.Empty)
             {
-                KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem);
-                menuBar.KeyBindings.Remove (menuItem.Shortcut);
-                menuBar.KeyBindings.Add (menuItem.Shortcut, keyBinding);
+                menuItem.UpdateShortcutKeyBinding (Key.Empty);
             }
 
             SubMenu (menuItem)?.AddShortcutKeyBindings (menuBar);
@@ -176,4 +174,75 @@ private void SetTitle (string title)
         title ??= string.Empty;
         Title = title;
     }
+
+    /// <summary>
+    /// Add a <see cref="MenuBarItem"/> dynamically into the <see cref="MenuBar"/><c>.Menus</c>.
+    /// </summary>
+    /// <param name="menuItem"></param>
+    public void AddMenuBarItem (MenuItem menuItem = null)
+    {
+        if (menuItem is null)
+        {
+            MenuBarItem [] menus = _menuBar.Menus;
+            Array.Resize (ref menus, menus.Length + 1);
+            menus [^1] = this;
+            _menuBar.Menus = menus;
+        }
+        else
+        {
+            MenuItem [] childrens = Children ?? [];
+            Array.Resize (ref childrens, childrens.Length + 1);
+            childrens [^1] = menuItem;
+            Children = childrens;
+        }
+    }
+
+    /// <inheritdoc />
+    public override void RemoveMenuItem ()
+    {
+        if (Children is { })
+        {
+            foreach (MenuItem menuItem in Children)
+            {
+                if (menuItem.ShortcutKey != Key.Empty)
+                {
+                    // Remove an existent ShortcutKey
+                    _menuBar?.KeyBindings.Remove (menuItem.ShortcutKey);
+                }
+            }
+        }
+
+        if (ShortcutKey != Key.Empty)
+        {
+            // Remove an existent ShortcutKey
+            _menuBar?.KeyBindings.Remove (ShortcutKey);
+        }
+
+        var index = _menuBar!.Menus.IndexOf (this);
+        if (index > -1)
+        {
+            if (_menuBar!.Menus [index].HotKey != Key.Empty)
+            {
+                // Remove an existent HotKey
+                _menuBar?.KeyBindings.Remove (HotKey.WithAlt);
+            }
+
+            _menuBar!.Menus [index] = null;
+        }
+
+        var i = 0;
+
+        foreach (MenuBarItem m in _menuBar.Menus)
+        {
+            if (m != null)
+            {
+                _menuBar.Menus [i] = m;
+                i++;
+            }
+        }
+
+        MenuBarItem [] menus = _menuBar.Menus;
+        Array.Resize (ref menus, menus.Length - 1);
+        _menuBar.Menus = menus;
+    }
 }