Skip to content

Commit 6006bcf

Browse files
YourRobotOverlordCopilottig
authored
Fixes #4885. View.GetScheme() falls back gracefully instead of throwing when SchemeName is not found (#4886)
* Fixes #4457. Implement graceful scheme fallback chain in View.GetScheme() Add SchemeManager.TryGetScheme() — a non-throwing scheme lookup that safely handles uninitialized ConfigurationManager and missing Schemes in the current theme (both cases that GetSchemesForCurrentTheme() throws for). Refactor View.GetScheme() DefaultAction to use the full fallback chain in all cases, removing the asymmetry where a SchemeName pointing to a missing scheme threw KeyNotFoundException while no SchemeName gracefully fell back: HasScheme (_scheme) -> Named scheme via TryGetScheme(SchemeName) -> SuperView.GetScheme() -> 'Base' scheme -> Hard-coded 'Base' (guaranteed last resort) Add Logging.Warning() when SchemeName is set but not found, to aid debugging. Add tests covering: TryGetScheme happy/sad paths, fallback to SuperView, fallback to Base, and full chain traversal with missing scheme at every level. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ThemeFallback UICatalog scenario demonstrating SchemeName fallback chain - Registers a custom 'CustomHighlight' scheme with TextStyle.Blink on the Default theme, making it visually distinct from built-in schemes - Shows a view with SchemeName = 'CustomHighlight' (found on Default, blinks) - Shows a view with SchemeName = 'NonExistentScheme' (never found, falls back) - Includes a theme selector: switching to a non-Default theme causes 'CustomHighlight' to also fall back, demonstrating both fallback cases live - The warning written to the debug log for missing scheme names is mentioned in the UI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Code Review/code cleanup and fix stupid UICatalog bug I introduced and didn't test... Refactor style, naming, and shortcut logic for consistency Refactored constant naming and string interpolation in ThemeFallback for clarity and consistency. Improved XML documentation formatting. Updated UICatalogRunnable shortcut key assignment to avoid conflicts and accept ignored keys. Fixed GetScheme logic in View to handle empty SchemeName correctly. Replaced var with explicit types and used collection expressions in tests. Improved lambda formatting and simplified event handler assignments. Applied project code style conventions throughout. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Tig <tig@users.noreply.github.com>
1 parent 2df822d commit 6006bcf

7 files changed

Lines changed: 488 additions & 122 deletions

File tree

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
#nullable enable
2+
3+
namespace UICatalog.Scenarios;
4+
5+
/// <summary>
6+
/// Demonstrates the SchemeName fallback chain introduced in v2.
7+
/// When a view's <see cref="View.SchemeName"/> is not found in the active theme, the view no longer
8+
/// throws a <see cref="KeyNotFoundException"/>. Instead, it walks the fallback chain:
9+
/// <list type="number">
10+
/// <item>
11+
/// <description>Named scheme (if found in current theme)</description>
12+
/// </item>
13+
/// <item>
14+
/// <description>SuperView's scheme (recursive)</description>
15+
/// </item>
16+
/// <item>
17+
/// <description>"Base" scheme from the current theme</description>
18+
/// </item>
19+
/// <item>
20+
/// <description>Hard-coded "Base" scheme (always present)</description>
21+
/// </item>
22+
/// </list>
23+
/// </summary>
24+
[ScenarioMetadata ("Theme Fallback", "Demonstrates graceful SchemeName fallback when a named scheme is missing from the active theme.")]
25+
[ScenarioCategory ("Colors")]
26+
[ScenarioCategory ("Configuration")]
27+
public sealed class ThemeFallback : Scenario
28+
{
29+
private const string CUSTOM_SCHEME_NAME = "CustomHighlight";
30+
private const string MISSING_SCHEME_NAME = "NonExistentScheme";
31+
32+
public override void Main ()
33+
{
34+
ConfigurationManager.Enable (ConfigLocations.All);
35+
36+
using IApplication app = Application.Create ();
37+
app.Init ();
38+
39+
// Extend the Default theme with a custom scheme that has TextStyle.Blink
40+
// so it stands out visually. Other built-in themes do NOT contain this scheme,
41+
// so switching themes lets you watch the fallback chain activate in real time.
42+
SchemeManager.AddScheme (CUSTOM_SCHEME_NAME, new () { Normal = new Attribute (Color.BrightYellow, Color.Blue, TextStyle.Blink) });
43+
44+
using Window appWindow = new ();
45+
appWindow.Title = GetQuitKeyAndName ();
46+
47+
// --- Theme selector ---
48+
string [] themeLabels = ThemeManager.GetThemeNames ().Select (n => "_" + n).ToArray ();
49+
50+
OptionSelector themeSelector = new ()
51+
{
52+
Title = "_Theme",
53+
BorderStyle = LineStyle.Rounded,
54+
X = 1,
55+
Y = 1,
56+
Width = Dim.Auto (),
57+
Height = Dim.Auto (),
58+
Labels = themeLabels,
59+
Value = ThemeManager.GetThemeNames ().IndexOf (ThemeManager.Theme)
60+
};
61+
62+
themeSelector.ValueChanged += (sender, args) =>
63+
{
64+
if (sender is not OptionSelector sel)
65+
{
66+
return;
67+
}
68+
69+
string rawLabel = sel.Labels! [(int)args.NewValue!];
70+
71+
// Strip the leading underscore added for keyboard shortcut.
72+
ThemeManager.Theme = rawLabel [1..];
73+
ConfigurationManager.Apply ();
74+
75+
// Re-add the custom scheme to the newly-active theme so the
76+
// "Default" theme always demonstrates the found case.
77+
if (ThemeManager.Theme == ThemeManager.DEFAULT_THEME_NAME)
78+
{
79+
SchemeManager.AddScheme (CUSTOM_SCHEME_NAME,
80+
new () { Normal = new Attribute (Color.BrightYellow, Color.Blue, TextStyle.Blink) });
81+
}
82+
};
83+
84+
// --- Explanation ---
85+
Label intro = new ()
86+
{
87+
X = Pos.Right (themeSelector) + 1,
88+
Y = 1,
89+
Width = Dim.Fill (1),
90+
Text = $"Switch to a non-Default theme to see the fallback activate.\n"
91+
+ $" • \"{CUSTOM_SCHEME_NAME}\" is only in the Default theme.\n"
92+
+ $" • \"{MISSING_SCHEME_NAME}\" is never in any theme.\n"
93+
+ $"In both missing cases the view falls back gracefully instead of throwing."
94+
};
95+
96+
// --- View 1: scheme FOUND in the active theme ---
97+
FrameView foundFrame = new ()
98+
{
99+
Title = $"SchemeName = \"{CUSTOM_SCHEME_NAME}\"",
100+
X = Pos.Right (themeSelector) + 1,
101+
Y = Pos.Bottom (intro) + 1,
102+
Width = Dim.Fill (1),
103+
Height = 5,
104+
SchemeName = CUSTOM_SCHEME_NAME
105+
};
106+
107+
Label foundLabel = new ()
108+
{
109+
X = 1,
110+
Y = 1,
111+
Width = Dim.Fill (2),
112+
Text = $"On the Default theme this scheme exists (BrightYellow/Blue + Blink).\n"
113+
+ $"On any other theme the scheme is missing → fallback chain activates."
114+
};
115+
foundFrame.Add (foundLabel);
116+
117+
// --- View 2: scheme NEVER found — fallback always activates ---
118+
FrameView missingFrame = new ()
119+
{
120+
Title = $"SchemeName = \"{MISSING_SCHEME_NAME}\"",
121+
X = Pos.Right (themeSelector) + 1,
122+
Y = Pos.Bottom (foundFrame) + 1,
123+
Width = Dim.Fill (1),
124+
Height = 5,
125+
SchemeName = MISSING_SCHEME_NAME
126+
};
127+
128+
Label missingLabel = new ()
129+
{
130+
X = 1,
131+
Y = 1,
132+
Width = Dim.Fill (2),
133+
Text = $"This scheme does not exist in any theme.\n"
134+
+ $"The view silently falls back to its SuperView's scheme (no exception).\n"
135+
+ $"A warning is written to the debug log."
136+
};
137+
missingFrame.Add (missingLabel);
138+
139+
appWindow.Add (themeSelector, intro, foundFrame, missingFrame);
140+
141+
app.Run (appWindow);
142+
}
143+
}

Examples/UICatalog/UICatalogRunnable.cs

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ private ListView CreateCategoryList ()
641641
SuperViewRendersLineCanvas = true,
642642
Source = new ListWrapper<string> (CachedCategories)
643643
};
644+
644645
//categoryList.Border.Settings = BorderSettings.Title | BorderSettings.Tab;
645646
//categoryList.Border.TabSide = Side.Top;
646647
//categoryList.Border.Thickness = new Thickness (1, 2, 1, 1);
@@ -721,20 +722,31 @@ public static bool ShowStatusBar
721722
/// or assigned to a <see cref="MenuItem.Key"/> on a menu item. Falls back to
722723
/// <see cref="Key.F12"/> if all are taken.
723724
/// </summary>
724-
private Key GetFirstUnboundFKey ()
725+
/// <param name="ignoreKeys"></param>
726+
private Key GetFirstUnboundFKey (HashSet<Key> ignoreKeys)
725727
{
726728
Key [] fKeys =
727729
[
728-
Key.F1, Key.F2, Key.F3, Key.F4, Key.F5, Key.F6,
729-
Key.F7, Key.F8, Key.F9, Key.F10, Key.F11, Key.F12
730+
Key.F1,
731+
Key.F2,
732+
Key.F3,
733+
Key.F4,
734+
Key.F5,
735+
Key.F6,
736+
Key.F7,
737+
Key.F8,
738+
Key.F9,
739+
Key.F10,
740+
Key.F11,
741+
Key.F12
730742
];
731743

732744
// Collect keys already bound at the Application level
733-
HashSet<Key> boundKeys = [];
745+
HashSet<Key> boundKeys = ignoreKeys;
734746

735747
foreach (Key fKey in fKeys)
736748
{
737-
if (Application.KeyBindings.TryGet (fKey, out _))
749+
if (App?.Keyboard.KeyBindings.TryGet (fKey, out _) is true)
738750
{
739751
boundKeys.Add (fKey);
740752
}
@@ -753,6 +765,16 @@ private Key GetFirstUnboundFKey ()
753765
}
754766
}
755767

768+
// Exclude keys used by StatusBar items
769+
if (_statusBar is { })
770+
{
771+
foreach (View view in _statusBar.SubViews)
772+
{
773+
var shortcut = (Shortcut)view;
774+
boundKeys.Add (shortcut.Key);
775+
}
776+
}
777+
756778
foreach (Key fKey in fKeys)
757779
{
758780
if (!boundKeys.Contains (fKey))
@@ -773,7 +795,10 @@ private StatusBar CreateStatusBar ()
773795

774796
_shVersion = new Shortcut { Title = "Version Info", CanFocus = false };
775797

776-
Shortcut statusBarShortcut = new () { Key = GetFirstUnboundFKey (), Title = "Show/Hide Status Bar", CanFocus = false, Action = () => ShowStatusBar = !ShowStatusBar };
798+
Shortcut statusBarShortcut = new ()
799+
{
800+
Key = GetFirstUnboundFKey ([]), Title = "Show/Hide Status Bar", CanFocus = false, Action = () => ShowStatusBar = !ShowStatusBar
801+
};
777802

778803
_force16ColorsShortcutCb = new CheckBox
779804
{
@@ -786,7 +811,7 @@ private StatusBar CreateStatusBar ()
786811
CommandView = _force16ColorsShortcutCb,
787812
HelpText = "",
788813
BindKeyToApplication = true,
789-
Key = Key.F7,
814+
Key = GetFirstUnboundFKey ([statusBarShortcut.Key]),
790815
Action = () =>
791816
{
792817
Driver.Force16Colors = !Driver.Force16Colors;

Terminal.Gui/Configuration/SchemeManager.cs

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,67 @@ public static Scheme GetScheme (Schemes schemeName)
136136
/// </summary>
137137
/// <param name="schemeName"></param>
138138
/// <returns></returns>
139-
/// <exception cref="ArgumentException"></exception>
139+
/// <exception cref="KeyNotFoundException">If <paramref name="schemeName"/> is not found in the current theme.</exception>
140140
public static Scheme GetScheme (string schemeName) { return GetSchemesForCurrentTheme ()! [schemeName]!; }
141141

142+
/// <summary>
143+
/// Attempts to get the <see cref="Scheme"/> for the specified name without throwing.
144+
/// Returns <see langword="false"/> and sets <paramref name="scheme"/> to <see langword="null"/> if the scheme is
145+
/// not found, or if the configuration is not in a state where schemes can be resolved.
146+
/// </summary>
147+
/// <param name="schemeName">The name of the scheme to retrieve.</param>
148+
/// <param name="scheme">
149+
/// When this method returns <see langword="true"/>, contains the resolved <see cref="Scheme"/>; otherwise
150+
/// <see langword="null"/>.
151+
/// </param>
152+
/// <returns><see langword="true"/> if the scheme was found; otherwise <see langword="false"/>.</returns>
153+
public static bool TryGetScheme (string schemeName, [NotNullWhen (true)] out Scheme? scheme)
154+
{
155+
lock (_schemesLock)
156+
{
157+
Dictionary<string, Scheme?> schemes;
158+
159+
if (!ConfigurationManager.IsInitialized ())
160+
{
161+
// Module initializer / unit-test path — fall back to hard-coded defaults.
162+
ImmutableSortedDictionary<string, Scheme?>? hardCoded = GetHardCodedSchemes ();
163+
164+
if (hardCoded is null)
165+
{
166+
scheme = null;
167+
168+
return false;
169+
}
170+
171+
schemes = hardCoded.ToDictionary (StringComparer.InvariantCultureIgnoreCase);
172+
}
173+
else
174+
{
175+
// Avoid GetSchemesForCurrentTheme() — it throws if the Schemes property is absent.
176+
if (ThemeManager.GetCurrentTheme () ["Schemes"].PropertyValue
177+
is not Dictionary<string, Scheme?> themeSchemes)
178+
{
179+
scheme = null;
180+
181+
return false;
182+
}
183+
184+
schemes = themeSchemes;
185+
}
186+
187+
if (schemes.TryGetValue (schemeName, out Scheme? s) && s is not null)
188+
{
189+
scheme = s;
190+
191+
return true;
192+
}
193+
194+
scheme = null;
195+
196+
return false;
197+
}
198+
}
199+
142200
/// <summary>
143201
/// Gets the name of the specified <see cref="Schemes"/>. Will throw an exception if <paramref name="schemeName"/>
144202
/// is not a built-in Scheme.

Terminal.Gui/ViewBase/View.Drawing.Scheme.cs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,20 +134,47 @@ public Scheme GetScheme ()
134134

135135
Scheme DefaultAction ()
136136
{
137-
if (!HasScheme && !string.IsNullOrEmpty (SchemeName))
137+
if (HasScheme)
138138
{
139-
return SchemeManager.GetScheme (SchemeName);
139+
return _scheme!;
140140
}
141141

142-
if (!HasScheme)
142+
if (string.IsNullOrEmpty (SchemeName))
143143
{
144-
return SuperView?.GetScheme () ?? SchemeManager.GetScheme (Schemes.Base);
144+
return ResolveFallbackScheme ();
145145
}
146146

147-
return _scheme!;
147+
if (SchemeManager.TryGetScheme (SchemeName, out Scheme? namedScheme))
148+
{
149+
return namedScheme;
150+
}
151+
152+
Logging.Warning ($"SchemeName '{SchemeName}' not found in current theme. Falling back.");
153+
154+
return ResolveFallbackScheme ();
148155
}
149156
}
150157

158+
/// <summary>
159+
/// Resolves a scheme using the fallback chain when no explicit scheme or valid <see cref="SchemeName"/> is
160+
/// available: <see cref="SuperView"/>'s scheme → "Base" in the current theme → hard-coded "Base".
161+
/// </summary>
162+
private Scheme ResolveFallbackScheme ()
163+
{
164+
if (SuperView is { })
165+
{
166+
return SuperView.GetScheme ();
167+
}
168+
169+
if (SchemeManager.TryGetScheme ("Base", out Scheme? baseScheme))
170+
{
171+
return baseScheme;
172+
}
173+
174+
// Last resort: hard-coded defaults are always available regardless of configuration state.
175+
return SchemeManager.GetHardCodedSchemes ()! ["Base"]!;
176+
}
177+
151178
/// <summary>
152179
/// Called when the <see cref="Scheme"/> for the <see cref="View"/> is being retrieved. Subclasses can return
153180
/// true to stop further processing and optionally set <paramref name="scheme"/> to a different value.

Tests/UnitTestsParallelizable/Configuration/SchemeManagerTests.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,36 @@ public void GetScheme_Throws_On_Invalid_String ()
7777
Assert.Throws<KeyNotFoundException> (() => SchemeManager.GetScheme ("NotAScheme"));
7878
}
7979

80+
// Copilot
81+
82+
[Fact]
83+
public void TryGetScheme_ExistingScheme_ReturnsTrueAndScheme ()
84+
{
85+
bool found = SchemeManager.TryGetScheme ("Base", out Scheme? scheme);
86+
87+
Assert.True (found);
88+
Assert.NotNull (scheme);
89+
}
90+
91+
[Fact]
92+
public void TryGetScheme_MissingScheme_ReturnsFalseAndNull ()
93+
{
94+
bool found = SchemeManager.TryGetScheme ("DoesNotExist", out Scheme? scheme);
95+
96+
Assert.False (found);
97+
Assert.Null (scheme);
98+
}
99+
100+
[Fact]
101+
public void TryGetScheme_AllBuiltInSchemes_ReturnsTrue ()
102+
{
103+
foreach (string name in SchemeManager.GetSchemeNames ())
104+
{
105+
bool found = SchemeManager.TryGetScheme (name, out Scheme? scheme);
106+
107+
Assert.True (found, $"Expected TryGetScheme to return true for built-in scheme '{name}'");
108+
Assert.NotNull (scheme);
109+
}
110+
}
111+
80112
}

0 commit comments

Comments
 (0)