Skip to content

Commit

Permalink
Fixes gui-cs#3652. Setting Menus causes unexpected Exception. (gui-cs…
Browse files Browse the repository at this point in the history 
…#3653)

* Moving ShortcutDelimiter from MenuBar to Key.

* Rename to ShortcutKey and change type to Key.

* Improving add and remove menu items dynamically.

* Code cleanup.

* Fix status bar shortcuts issues.

* Fix build error.

* Change HotKey type to Key.

* Change HotKey.setter to private.

* Fix warnings.

* Fix some bugs.

* Rename ShortcutDelimiter to Separator.

* Add Separator property into the Configuration Manager.

* Change XML doc for Separator.

* Replace KeyEvent with Key.

* Add unit test preventing the Key.Separator is never Null ('\0).
  • Loading branch information
@BDisp
BDisp authored Aug 13, 2024
1 parent a893169 commit a661fce
Show file tree
Hide file tree
Showing 17 changed files with 802 additions and 599 deletions.
5 changes: 0 additions & 5 deletions Terminal.Gui/Application/Application.Keyboard.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ public static partial class Application // Keyboard handling

/// <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;
Expand All @@ -27,7 +26,6 @@ public static Key NextTabKey

/// <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;
Expand All @@ -45,7 +43,6 @@ public static Key PrevTabKey

/// <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;
Expand All @@ -63,7 +60,6 @@ public static Key NextTabGroupKey

/// <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;
Expand All @@ -81,7 +77,6 @@ public static Key PrevTabGroupKey

/// <summary>Gets or sets the key to quit the application.</summary>
[SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
[JsonConverter (typeof (KeyJsonConverter))]
public static Key QuitKey
{
get => _quitKey;
Expand Down
25 changes: 21 additions & 4 deletions Terminal.Gui/Input/Key.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Text.Json.Serialization;

namespace Terminal.Gui;

Expand Down Expand Up @@ -448,9 +449,9 @@ public override bool Equals (object obj)

#region String conversion

/// <summary>Pretty prints the KeyEvent</summary>
/// <summary>Pretty prints the Key.</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)
{
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -971,4 +972,20 @@ out parsedInt
public static Key F24 => new (KeyCode.F24);

#endregion

private static Rune _separator = new ('+');

/// <summary>Gets or sets the separator character used when parsing and printing Keys. E.g. Ctrl+A. The default is '+'.</summary>
[SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
public static Rune Separator
{
get => _separator;
set
{
if (_separator != value)
{
_separator = value == default (Rune) ? new ('+') : value;
}
}
}
}
4 changes: 2 additions & 2 deletions Terminal.Gui/Input/ShortcutHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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 ());
Expand Down
1 change: 1 addition & 0 deletions Terminal.Gui/Resources/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"Application.NextTabGroupKey": "F6",
"Application.PrevTabGroupKey": "Shift+F6",
"Application.QuitKey": "Esc",
"Key.Separator": "+",

"Theme": "Default",
"Themes": [
Expand Down
37 changes: 23 additions & 14 deletions Terminal.Gui/Views/Menu/Menu.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ _barItems.Children [_currentChild]
}
);

AddKeyBindings (_barItems);
AddKeyBindingsHotKey (_barItems);
}

public Menu ()
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -910,6 +917,8 @@ internal bool CheckSubMenu ()

protected override void Dispose (bool disposing)
{
RemoveKeyBindingsHotKey (_barItems);

if (Application.Current is { })
{
Application.Current.DrawContentComplete -= Current_DrawContentComplete;
Expand Down
51 changes: 27 additions & 24 deletions Terminal.Gui/Views/Menu/MenuBar.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)'_';

Expand Down Expand Up @@ -1321,6 +1312,10 @@ private bool Select (int index)
{
OpenMenu ();
}
else if (Menus [index].IsTopLevel)
{
Run (Menus [index].Action);
}
else
{
Activate (index);
Expand Down Expand Up @@ -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);
}
}
79 changes: 74 additions & 5 deletions Terminal.Gui/Views/Menu/MenuBarItem.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
Loading

0 comments on commit a661fce

Please sign in to comment.