Skip to content

Commit 2b7fd03

Browse files
committed
Fix 32+ bugs: thread safety, unicode rendering, code quality, dropdown arrows, layout crash
Core infrastructure: - InvalidationManager: finally block no longer overrides catch error recovery - NotificationStateService: snapshot list with ToList().AsReadOnly(); disposed guard - DisposableManager: set _disposed before disposal work to prevent TOCTOU race - FocusStateService: guard null-to-null ClearFocus in PopFocus - ModalStateService: cycle detection in FindDeepestModalChildInternal - WindowStateService: merge FlashWindow TOCTOU lock blocks Thread safety: - WindowLifecycleHelper: marshal background thread UI mutations via EnqueueOnUIThread - WindowLifecycleHelper: remove double-dispose in ApplyErrorState - TerminalControl: TryClose via EnqueueOnUIThread from ReadLoop - TreeControl: capture deferred events inside lock before firing - ListControl: read _items.Count inside lock in Items/StringItems setters - UnixStdinReader: add _mouseStateLock for torn-read protection - WindowPositioningManager: use SetPositionDirect to avoid re-entrant MoveWindowTo Window state: - Window.SetIsActive: set _isActive before firing Activated/Deactivated events - Window.SetIsActive: clear leaf HasFocus on deactivation Unicode rendering (Rule 12B/12C): - SliderControl/RangeSliderControl: UnicodeWidth.GetStringWidth for label widths - TabControl: MarkupParser.Parse for nav hint rendering - ListControl: UnicodeWidth for selectionIndicator width - TreeControl: use ControlRenderingHelpers.FillRect for gradient support Code quality: - FigleControl, LayoutNode, DatePickerControl, DropdownControl: StringBuilder - SequenceHelper, UnixStdinReader: named constants from ControlDefaults - Add ContinuousPressIntervalMs constant Dropdown arrows: - Use narrow ▾/▴ (U+25BE/U+25B4) instead of ▼/▲ which render as 2-wide - Add ControlDefaults for dropdown arrow characters - Render arrow separately via MarkupParser.Parse Layout crash fix: - LayoutConstraints.Constrain: defensive Math.Max to prevent MinWidth > MaxWidth - ScrollablePanelControl: clamp contentWidth to minimum 1 Tests: - DropdownControlTests: add assertions to 5 empty keyboard tests - TestWindowSystemBuilder: fix no-op configure callback (Func instead of Action) - AppRegistryTests: polling loop instead of fragile Task.Delay
1 parent b07acaa commit 2b7fd03

33 files changed

Lines changed: 306 additions & 133 deletions

SharpConsoleUI.Tests/Controls/DropdownControlTests.cs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -521,8 +521,9 @@ public void ProcessInput_Enter_TogglesDropdown()
521521
var dd = CreateFocusedDropdown("S:", "A", "B", "C");
522522
// Without a container Window, Enter won't create portal, but IsDropdownOpen should still toggle
523523
// The state is maintained internally even if portal creation fails
524-
dd.ProcessKey(Key(ConsoleKey.Enter));
525-
// Can't fully test open without Window, but we verify the control processes the key
524+
var result = dd.ProcessKey(Key(ConsoleKey.Enter));
525+
Assert.True(result);
526+
Assert.True(dd.IsDropdownOpen);
526527
}
527528

528529
[Fact]
@@ -538,32 +539,40 @@ public void ProcessInput_DownArrow_IncreasesHighlight()
538539
{
539540
var dd = CreateFocusedDropdown("S:", "A", "B", "C");
540541
dd.SelectedIndex = 0;
541-
dd.ProcessKey(Key(ConsoleKey.DownArrow));
542-
// Without open dropdown, down arrow should navigate
542+
var result = dd.ProcessKey(Key(ConsoleKey.DownArrow));
543+
// When closed, DownArrow opens the dropdown
544+
Assert.True(result);
545+
Assert.True(dd.IsDropdownOpen);
543546
}
544547

545548
[Fact]
546549
public void ProcessInput_UpArrow_DecreasesHighlight()
547550
{
548551
var dd = CreateFocusedDropdown("S:", "A", "B", "C");
549552
dd.SelectedIndex = 2;
550-
dd.ProcessKey(Key(ConsoleKey.UpArrow));
553+
// When closed, UpArrow does nothing
554+
var result = dd.ProcessKey(Key(ConsoleKey.UpArrow));
555+
Assert.False(result);
551556
}
552557

553558
[Fact]
554559
public void ProcessInput_Home_JumpsToFirst()
555560
{
556561
var dd = CreateFocusedDropdown("S:", "A", "B", "C");
557562
dd.SelectedIndex = 2;
558-
dd.ProcessKey(Key(ConsoleKey.Home));
563+
// When closed, Home does nothing
564+
var result = dd.ProcessKey(Key(ConsoleKey.Home));
565+
Assert.False(result);
559566
}
560567

561568
[Fact]
562569
public void ProcessInput_End_JumpsToLast()
563570
{
564571
var dd = CreateFocusedDropdown("S:", "A", "B", "C");
565572
dd.SelectedIndex = 0;
566-
dd.ProcessKey(Key(ConsoleKey.End));
573+
// When closed, End does nothing
574+
var result = dd.ProcessKey(Key(ConsoleKey.End));
575+
Assert.False(result);
567576
}
568577

569578
[Fact]
@@ -572,8 +581,9 @@ public void ProcessInput_Disabled_IgnoresInput()
572581
var dd = CreateFocusedDropdown("S:", "A", "B");
573582
dd.IsEnabled = false;
574583
dd.SelectedIndex = 0;
575-
dd.ProcessKey(Key(ConsoleKey.DownArrow));
576-
// Should not change because disabled
584+
var result = dd.ProcessKey(Key(ConsoleKey.DownArrow));
585+
Assert.False(result);
586+
Assert.Equal(0, dd.SelectedIndex);
577587
}
578588

579589
#endregion

SharpConsoleUI.Tests/Infrastructure/TestWindowSystemBuilder.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ private static ConsoleWindowSystem WithInputHandlers(ConsoleWindowSystem system)
3636
/// <summary>
3737
/// Creates a test window system with diagnostics enabled and a mock console driver.
3838
/// </summary>
39-
/// <param name="configure">Optional configuration callback.</param>
39+
/// <param name="configure">Optional configuration callback that returns a modified options record.</param>
4040
/// <returns>A configured ConsoleWindowSystem instance ready for testing.</returns>
41-
public static ConsoleWindowSystem CreateTestSystem(Action<ConsoleWindowSystemOptions>? configure = null)
41+
public static ConsoleWindowSystem CreateTestSystem(Func<ConsoleWindowSystemOptions, ConsoleWindowSystemOptions>? configure = null)
4242
{
4343
// Disable status bars by default in tests to isolate core rendering logic
4444
var statusBarOptions = new Configuration.StatusBarOptions(
@@ -58,13 +58,10 @@ public static ConsoleWindowSystem CreateTestSystem(Action<ConsoleWindowSystemOpt
5858
StatusBarOptions: statusBarOptions
5959
);
6060

61-
// Allow caller to customize options
61+
// Allow caller to customize options via record 'with' expressions
6262
if (configure != null)
6363
{
64-
// Create a modified options instance
65-
var customOptions = options;
66-
configure(customOptions);
67-
options = customOptions;
64+
options = configure(options);
6865
}
6966

7067
// Create mock console driver

SharpConsoleUI.Tests/Registry/AppRegistryTests.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,17 @@ public async Task LazyFlush_TimerFires_PersistsData()
9696
using var reg = new AppRegistry(config);
9797
reg.OpenSection("App").SetInt("X", 77);
9898

99-
// Wait 10x the interval — generous margin for CI environments under load
100-
await Task.Delay(500);
99+
// Poll until the lazy flush timer persists data, with a generous timeout for CI
100+
var timeout = TimeSpan.FromSeconds(2);
101+
var start = DateTime.UtcNow;
102+
System.Text.Json.Nodes.JsonNode? loaded = null;
103+
while (DateTime.UtcNow - start < timeout)
104+
{
105+
loaded = storage.Load();
106+
if (loaded != null) break;
107+
await Task.Delay(50);
108+
}
101109

102-
var loaded = storage.Load();
103110
Assert.NotNull(loaded);
104111
}
105112

SharpConsoleUI/Configuration/ControlDefaults.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ public static class ControlDefaults
7373
/// </summary>
7474
public const int DefaultDebounceMs = 300;
7575

76+
/// <summary>
77+
/// Interval for continuous mouse button press polling in milliseconds (default: 100ms)
78+
/// </summary>
79+
public const int ContinuousPressIntervalMs = 100;
80+
7681
/// <summary>
7782
/// Cursor blink rate for text inputs in milliseconds (default: 500ms)
7883
/// </summary>
@@ -448,6 +453,32 @@ public static class ControlDefaults
448453
/// </summary>
449454
public const string DatePickerDropdownIndicator = "\u25BC";
450455

456+
// Dropdown defaults
457+
458+
/// <summary>
459+
/// Arrow indicator for a closed dropdown (points down).
460+
/// Uses small triangle (U+25BE) which is reliably 1-column wide across terminals.
461+
/// </summary>
462+
public const string DropdownClosedArrow = "\u25BE";
463+
464+
/// <summary>
465+
/// Arrow indicator for an open dropdown (points up).
466+
/// Uses small triangle (U+25B4) which is reliably 1-column wide across terminals.
467+
/// </summary>
468+
public const string DropdownOpenArrow = "\u25B4";
469+
470+
/// <summary>
471+
/// Scroll-up indicator for dropdown portal.
472+
/// Uses small triangle (U+25B4) which is reliably 1-column wide across terminals.
473+
/// </summary>
474+
public const string DropdownScrollUpArrow = "\u25B4";
475+
476+
/// <summary>
477+
/// Scroll-down indicator for dropdown portal.
478+
/// Uses small triangle (U+25BE) which is reliably 1-column wide across terminals.
479+
/// </summary>
480+
public const string DropdownScrollDownArrow = "\u25BE";
481+
451482
// TimePicker defaults
452483

453484
/// <summary>

SharpConsoleUI/Controls/DatePickerControl/DatePickerControl.Rendering.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,16 +185,18 @@ internal void PaintCalendarInternal(CharacterBuffer buffer, LayoutRect bounds, L
185185
if (paintY >= clipRect.Y && paintY < clipRect.Bottom)
186186
{
187187
string monthName = _displayMonth.ToString("MMMM yyyy", _culture);
188-
string header = $" {ControlDefaults.CalendarPrevMonthArrow} {monthName}";
189-
int headerDisplayLen = Parsing.MarkupParser.StripLength(header);
188+
var headerBuilder = new System.Text.StringBuilder();
189+
headerBuilder.Append(" ").Append(ControlDefaults.CalendarPrevMonthArrow).Append(" ").Append(monthName);
190+
int headerDisplayLen = Parsing.MarkupParser.StripLength(headerBuilder.ToString());
190191
int arrowRightPos = innerWidth - 3;
191192
int padding = Math.Max(0, arrowRightPos - headerDisplayLen);
192-
header += new string(' ', padding) + ControlDefaults.CalendarNextMonthArrow + " ";
193+
headerBuilder.Append(' ', padding).Append(ControlDefaults.CalendarNextMonthArrow).Append(" ");
193194

194195
// Ensure total width
195-
int totalLen = Parsing.MarkupParser.StripLength(header);
196+
int totalLen = Parsing.MarkupParser.StripLength(headerBuilder.ToString());
196197
if (totalLen < innerWidth)
197-
header += new string(' ', innerWidth - totalLen);
198+
headerBuilder.Append(' ', innerWidth - totalLen);
199+
string header = headerBuilder.ToString();
198200

199201
var headerCells = Parsing.MarkupParser.Parse(header, headerColor, bg);
200202
buffer.WriteCellsClipped(startX, paintY, headerCells, clipRect);

SharpConsoleUI/Controls/DropdownControl/DropdownControl.Portal.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// License: MIT
77
// -----------------------------------------------------------------------
88

9+
using SharpConsoleUI.Configuration;
910
using SharpConsoleUI.Drivers;
1011
using SharpConsoleUI.Helpers;
1112
using SharpConsoleUI.Layout;
@@ -236,11 +237,13 @@ internal void PaintDropdownListInternal(CharacterBuffer buffer, LayoutRect clipR
236237
{
237238
if (paintY >= clipRect.Y && paintY < clipRect.Bottom)
238239
{
239-
string scrollIndicator = (dropdownScroll > 0 ? "▲" : " ");
240+
var scrollBuilder = new System.Text.StringBuilder();
241+
scrollBuilder.Append(dropdownScroll > 0 ? ControlDefaults.DropdownScrollUpArrow : " ");
240242
int scrollPadding = dropdownWidth - 2;
241243
if (scrollPadding > 0)
242-
scrollIndicator += new string(' ', scrollPadding);
243-
scrollIndicator += (dropdownScroll + itemsToShow < items.Count ? "▼" : " ");
244+
scrollBuilder.Append(' ', scrollPadding);
245+
scrollBuilder.Append(dropdownScroll + itemsToShow < items.Count ? ControlDefaults.DropdownScrollDownArrow : " ");
246+
string scrollIndicator = scrollBuilder.ToString();
244247

245248
var scrollCells = Parsing.MarkupParser.Parse(scrollIndicator, foregroundColor, backgroundColor);
246249
buffer.WriteCellsClipped(startX, paintY, scrollCells, clipRect);

SharpConsoleUI/Controls/DropdownControl/DropdownControl.Rendering.cs

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// License: MIT
77
// -----------------------------------------------------------------------
88

9+
using SharpConsoleUI.Configuration;
910
using SharpConsoleUI.Drivers;
1011
using SharpConsoleUI.Helpers;
1112
using SharpConsoleUI.Layout;
@@ -22,7 +23,7 @@ public override LayoutSize MeasureDOM(LayoutConstraints constraints)
2223
int dropdownWidth = calculateHeaderWidth(constraints.MaxWidth - Margin.Left - Margin.Right);
2324

2425
// Sane minimum: prompt + arrow + space for at least a few chars
25-
string arrow = "▼";
26+
string arrow = ControlDefaults.DropdownClosedArrow;
2627
int minWidth = Parsing.MarkupParser.StripLength($"{_prompt} {arrow}") + 3;
2728
dropdownWidth = Math.Max(dropdownWidth, minWidth);
2829

@@ -80,7 +81,7 @@ public override void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutR
8081
int dropdownWidth = calculateHeaderWidth(targetWidth);
8182

8283
// Sane minimum: prompt + arrow + space for at least a few chars
83-
string arrowMin = "▼";
84+
string arrowMin = ControlDefaults.DropdownClosedArrow;
8485
int minWidth = Parsing.MarkupParser.StripLength($"{_prompt} {arrowMin}") + 3;
8586
dropdownWidth = Math.Min(Math.Max(dropdownWidth, minWidth), targetWidth);
8687

@@ -114,17 +115,16 @@ public override void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutR
114115

115116
// Render header: arrow flush-right, padding between text and arrow
116117
string selectedText = selectedIdx >= 0 && selectedIdx < paintSnapshot.Count ? paintSnapshot[selectedIdx].Text : "(None)";
117-
string arrow = _isDropdownOpen && _opensUpward ? "▲" : "▼";
118-
string suffix = $" {arrow}";
119-
int suffixLen = Parsing.MarkupParser.StripLength(suffix);
120-
int maxSelectedTextLength = dropdownWidth - promptLength - 1 - suffixLen; // 1 = space after prompt
118+
string arrow = _isDropdownOpen && _opensUpward ? ControlDefaults.DropdownOpenArrow : ControlDefaults.DropdownClosedArrow;
119+
int arrowDisplayWidth = Parsing.MarkupParser.StripLength(arrow);
120+
// Reserve: space + arrow
121+
int suffixReserved = 1 + arrowDisplayWidth;
122+
int maxSelectedTextLength = dropdownWidth - promptLength - 1 - suffixReserved; // 1 = space after prompt
121123
if (maxSelectedTextLength > 0 && Parsing.MarkupParser.StripLength(selectedText) > maxSelectedTextLength)
122124
selectedText = TextTruncationHelper.Truncate(selectedText, maxSelectedTextLength);
123125

124126
string prefix = $"{_prompt} {selectedText}";
125127
int prefixLen = Parsing.MarkupParser.StripLength(prefix);
126-
int paddingNeeded = Math.Max(0, dropdownWidth - prefixLen - suffixLen);
127-
string headerContent = prefix + new string(' ', paddingNeeded) + suffix;
128128

129129
int paintY = startY;
130130

@@ -136,10 +136,28 @@ public override void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutR
136136
if (alignOffset > 0)
137137
ControlRenderingHelpers.FillRect(buffer, new LayoutRect(startX, paintY, alignOffset, 1), foregroundColor, windowBackground, preserveBg);
138138

139-
var cells = Parsing.MarkupParser.Parse(headerContent, foregroundColor, backgroundColor);
140-
buffer.WriteCellsClipped(startX + alignOffset, paintY, cells, clipRect);
139+
int writeX = startX + alignOffset;
141140

142-
int rightFillStart = startX + alignOffset + dropdownWidth;
141+
// Paint prefix (prompt + selected text)
142+
var prefixCells = Parsing.MarkupParser.Parse(prefix, foregroundColor, backgroundColor);
143+
buffer.WriteCellsClipped(writeX, paintY, prefixCells, clipRect);
144+
writeX += prefixCells.Count;
145+
146+
// Paint padding between text and arrow
147+
int paddingNeeded = Math.Max(0, dropdownWidth - prefixLen - suffixReserved);
148+
for (int p = 0; p < paddingNeeded + 1; p++) // +1 for space before arrow
149+
{
150+
if (writeX >= clipRect.X && writeX < clipRect.Right)
151+
buffer.SetNarrowCell(writeX, paintY, ' ', foregroundColor, backgroundColor);
152+
writeX++;
153+
}
154+
155+
// Paint arrow via Parse (handles wide chars with continuation cells)
156+
var arrowCells = Parsing.MarkupParser.Parse(arrow, foregroundColor, backgroundColor);
157+
buffer.WriteCellsClipped(writeX, paintY, arrowCells, clipRect);
158+
writeX += arrowCells.Count;
159+
160+
int rightFillStart = writeX;
143161
int rightFillWidth = bounds.Right - rightFillStart - Margin.Right;
144162
if (rightFillWidth > 0)
145163
ControlRenderingHelpers.FillRect(buffer, new LayoutRect(rightFillStart, paintY, rightFillWidth, 1), foregroundColor, windowBackground, preserveBg);

SharpConsoleUI/Controls/DropdownControl/DropdownControl.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// License: MIT
77
// -----------------------------------------------------------------------
88

9+
using SharpConsoleUI.Configuration;
910
using SharpConsoleUI.Core;
1011
using SharpConsoleUI.Drivers;
1112
using SharpConsoleUI.Events;
@@ -643,7 +644,7 @@ private int calculateHeaderWidth(int? availableWidth)
643644
}
644645

645646
// Build header string with longest item and measure exact display width
646-
string arrow = "▼"; // use wider of the two arrows for stable measurement
647+
string arrow = ControlDefaults.DropdownClosedArrow;
647648
string header = $"{_prompt} {longestText} {arrow}";
648649
return Parsing.MarkupParser.StripLength(header);
649650
}

SharpConsoleUI/Controls/FigleControl.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,11 +287,12 @@ private static FigletFont LoadEmbeddedFont(string fileName)
287287
private static FigletFont CreateFallbackFont()
288288
{
289289
// Create a trivial 1-height font where each character is just itself
290-
var fontData = "flf2a$ 1 1 1 0 0\n";
290+
var sb = new System.Text.StringBuilder("flf2a$ 1 1 1 0 0\n");
291291
for (int c = 32; c <= 126; c++)
292292
{
293-
fontData += (char)c + "@@\n";
293+
sb.Append((char)c).Append("@@\n");
294294
}
295+
var fontData = sb.ToString();
295296
using var stream = new MemoryStream(System.Text.Encoding.UTF8.GetBytes(fontData));
296297
return FigletFont.Load(stream);
297298
}

SharpConsoleUI/Controls/ListControl/ListControl.Rendering.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ public override void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutR
447447
string iconMarkup = $"[{iconColor.ToMarkup()}]{iconText}[/] ";
448448
int iconVisibleLength = GetCachedTextLength(iconText) + 1;
449449
itemContent = selectionIndicator + iconMarkup + lineText;
450-
int visibleTextLength = selectionIndicator.Length + iconVisibleLength + GetCachedTextLength(lineText);
450+
int visibleTextLength = UnicodeWidth.GetStringWidth(selectionIndicator) + iconVisibleLength + GetCachedTextLength(lineText);
451451
int paddingNeeded = Math.Max(0, listWidth - visibleTextLength);
452452
if (paddingNeeded > 0) itemContent += new string(' ', paddingNeeded);
453453
}
@@ -461,7 +461,7 @@ public override void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutR
461461
indent = new string(' ', iconWidth);
462462
}
463463
itemContent = selectionIndicator + indent + lineText;
464-
int visibleTextLength = selectionIndicator.Length + indent.Length + GetCachedTextLength(lineText);
464+
int visibleTextLength = UnicodeWidth.GetStringWidth(selectionIndicator) + indent.Length + GetCachedTextLength(lineText);
465465
int paddingNeeded = Math.Max(0, listWidth - visibleTextLength);
466466
if (paddingNeeded > 0) itemContent += new string(' ', paddingNeeded);
467467
}

0 commit comments

Comments
 (0)