Skip to content

Commit d0a0ed9

Browse files
authored
Merge pull request #5584 from tui-cs/release/v2.4.17
2 parents 5e89ba9 + 6d9f096 commit d0a0ed9

13 files changed

Lines changed: 620 additions & 30 deletions

File tree

.github/workflows/prepare-release.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ jobs:
4848
git config user.email "github-actions[bot]@users.noreply.github.com"
4949
5050
- name: Install GitVersion
51-
uses: gittools/actions/gitversion/setup@v4.5.0
51+
uses: gittools/actions/gitversion/setup@v4.7.0
5252
with:
5353
versionSpec: '6.x'
5454

5555
- name: Determine Version
56-
uses: gittools/actions/gitversion/execute@v4.5.0
56+
uses: gittools/actions/gitversion/execute@v4.7.0
5757
with:
5858
useConfigFile: true
5959
id: gitversion

.github/workflows/publish.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ jobs:
3232
# from git history + GitVersion.yml during build. We still run the CLI here to
3333
# capture the SemVer for use in subsequent steps (push, template dispatch).
3434
- name: Install GitVersion
35-
uses: gittools/actions/gitversion/setup@v4.5.0
35+
uses: gittools/actions/gitversion/setup@v4.7.0
3636
with:
3737
versionSpec: '6.x'
3838

3939
- name: Determine Version
40-
uses: gittools/actions/gitversion/execute@v4.5.0
40+
uses: gittools/actions/gitversion/execute@v4.7.0
4141
with:
4242
useConfigFile: true
4343
id: gitversion # step id used as reference for output values

Directory.Packages.props

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.14.0" />
99
<PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" Version="4.14.0" />
1010
<PackageVersion Include="Microsoft.CodeAnalysis.Workspaces.MSBuild" Version="4.14.0" />
11-
<PackageVersion Include="Microsoft.Net.Compilers.Toolset" Version="5.3.0" />
11+
<PackageVersion Include="Microsoft.Net.Compilers.Toolset" Version="5.6.0" />
1212
<PackageVersion Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
1313
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="10.0.203" />
1414
<PackageVersion Include="Microsoft.CodeAnalysis" Version="4.14.0" />
@@ -17,13 +17,13 @@
1717
<PackageVersion Include="Microsoft.Extensions.Configuration" Version="10.0.7" />
1818
<PackageVersion Include="Microsoft.Extensions.Configuration.Binder" Version="10.0.7" />
1919
<PackageVersion Include="Microsoft.Extensions.Configuration.Json" Version="10.0.7" />
20-
<PackageVersion Include="Microsoft.Extensions.Options" Version="10.0.7" />
20+
<PackageVersion Include="Microsoft.Extensions.Options" Version="10.0.9" />
2121
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="10.0.7" />
2222
<PackageVersion Include="Microsoft.Extensions.Logging" Version="10.0.7" />
2323
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="10.0.7" />
2424
<PackageVersion Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.23.0" />
2525
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="18.7.0" />
26-
<PackageVersion Include="Spectre.Console" Version="0.57.1" />
26+
<PackageVersion Include="Spectre.Console" Version="0.57.2" />
2727
<PackageVersion Include="TestableIO.System.IO.Abstractions.TestingHelpers" Version="22.1.1" />
2828
<PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
2929
<PackageVersion Include="xunit.v3" Version="3.2.2" />
@@ -52,7 +52,7 @@
5252
<PackageVersion Include="Moq" Version="4.20.72" />
5353
<PackageVersion Include="ReportGenerator" Version="5.5.9" />
5454
<PackageVersion Include="coverlet.collector" Version="10.0.0" />
55-
<PackageVersion Include="GitVersion.MsBuild" Version="6.7.0" />
55+
<PackageVersion Include="GitVersion.MsBuild" Version="6.8.1" />
5656
</ItemGroup>
5757
<ItemGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
5858
<PackageVersion Include="Terminal.Gui" Version="2.0.0" />

Terminal.Gui/FileServices/FileSystemTreeBuilder.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,5 +72,18 @@ private IEnumerable<IFileSystemInfo> TryGetChildren (IFileSystemInfo entry)
7272
}
7373
}
7474

75-
internal static bool IsReparsePoint (IFileSystemInfo entry) => (entry.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint;
75+
internal static bool IsReparsePoint (IFileSystemInfo entry)
76+
{
77+
try
78+
{
79+
return (entry.Attributes & FileAttributes.ReparsePoint) == FileAttributes.ReparsePoint;
80+
}
81+
catch (Exception)
82+
{
83+
// Fail closed: if the reparse status cannot be read, treat the entry as a reparse point so
84+
// callers do not traverse it — an unreadable symlink/junction could otherwise recreate the
85+
// directory-cycle risk this guard exists to prevent.
86+
return true;
87+
}
88+
}
7689
}

Terminal.Gui/ViewBase/View.Layout.cs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,14 @@ public Rectangle Frame
6767
return;
6868
}
6969

70-
// BUGBUG: We set the internal fields here to avoid recursion. However, this means that
71-
// BUGBUG: other logic in the property setters does not get executed. Specifically:
72-
// BUGBUG: - Reset TextFormatter
73-
// BUGBUG: - SetLayoutNeeded (not an issue as we explicitly call Layout below)
74-
// BUGBUG: - If we add property change events for X/Y/Width/Height they will not be invoked
70+
// We set the internal fields directly (not via the X/Y/Width/Height setters) to avoid recursion.
71+
// This is intentional and the two side effects of the setters are accounted for elsewhere (#5498):
72+
// - TextFormatter constraints: recomputed by the Layout () -> SetRelativeLayout call below
73+
// (SetTextFormatterSize + FinalizeTextFormatterConstraints), so no reset is needed here.
74+
// - Width/Height (and X/Y) change events: deliberately NOT raised here. Those events observe
75+
// declarative Dim/Pos assignment; this path overwrites them with Absolute values as bookkeeping
76+
// to keep state consistent with an imperatively-set Frame. Use FrameChanged (raised by SetFrame
77+
// above) to observe resolved-size changes from any cause.
7578
// If Frame gets set, set all Pos/Dim to Absolute values.
7679
_x = _frame!.Value.X;
7780
_y = _frame!.Value.Y;
@@ -152,6 +155,14 @@ protected virtual void OnFrameChanged (in Rectangle frame) { }
152155
/// Raised when the <see cref="Frame"/> changes. This event is raised after the <see cref="Frame"/> has been
153156
/// updated.
154157
/// </summary>
158+
/// <remarks>
159+
/// This is the canonical event for observing resolved-size and position changes. It fires whenever the
160+
/// absolute <see cref="Frame"/> changes for any reason: assigning <see cref="X"/>, <see cref="Y"/>,
161+
/// <see cref="Width"/>, or <see cref="Height"/>; setting <see cref="Frame"/> directly; or a layout pass
162+
/// resolving a relative <see cref="Dim"/>/<see cref="Pos"/> (e.g. <see cref="DimFill"/>). By contrast, the
163+
/// <see cref="WidthChanged"/>/<see cref="HeightChanged"/> events observe assignment of the declarative
164+
/// <see cref="Width"/>/<see cref="Height"/> <see cref="Dim"/> only.
165+
/// </remarks>
155166
public event EventHandler<EventArgs<Rectangle>>? FrameChanged;
156167

157168
/// <summary>Gets the <see cref="Frame"/> with a screen-relative location.</summary>
@@ -368,6 +379,13 @@ public Pos Y
368379
/// allowing customization or cancellation of the change. The <see cref="HeightChanging"/> event
369380
/// is raised before the change, and <see cref="HeightChanged"/> is raised after.
370381
/// </para>
382+
/// <para>
383+
/// <see cref="HeightChanging"/>/<see cref="HeightChanged"/> observe assignment of this declarative
384+
/// <see cref="Dim"/> only. They are deliberately not raised when a layout pass resolves the view's size
385+
/// or when <see cref="Frame"/> is set directly, because those paths change the resolved
386+
/// <see cref="Frame"/> rather than the declarative <see cref="Height"/>. To observe resolved-size
387+
/// changes from any cause, subscribe to <see cref="FrameChanged"/>.
388+
/// </para>
371389
/// <para>The default value is <c>Dim.Absolute (0)</c>.</para>
372390
/// </remarks>
373391
/// <seealso cref="HeightChanging"/>
@@ -462,6 +480,13 @@ protected virtual void OnHeightChanged (ValueChangedEventArgs<Dim> args) { }
462480
/// allowing customization or cancellation of the change. The <see cref="WidthChanging"/> event
463481
/// is raised before the change, and <see cref="WidthChanged"/> is raised after.
464482
/// </para>
483+
/// <para>
484+
/// <see cref="WidthChanging"/>/<see cref="WidthChanged"/> observe assignment of this declarative
485+
/// <see cref="Dim"/> only. They are deliberately not raised when a layout pass resolves the view's size
486+
/// or when <see cref="Frame"/> is set directly, because those paths change the resolved
487+
/// <see cref="Frame"/> rather than the declarative <see cref="Width"/>. To observe resolved-size
488+
/// changes from any cause, subscribe to <see cref="FrameChanged"/>.
489+
/// </para>
465490
/// <para>The default value is <c>Dim.Absolute (0)</c>.</para>
466491
/// </remarks>
467492
/// <seealso cref="WidthChanging"/>
@@ -686,11 +711,14 @@ public bool SetRelativeLayout (Size superviewContentSize)
686711
_suppressNeedsDrawAfterLayout = false;
687712
}
688713

689-
// BUGBUG: We set the internal fields here to avoid recursion. However, this means that
690-
// BUGBUG: other logic in the property setters does not get executed. Specifically:
691-
// BUGBUG: - Reset TextFormatter
692-
// BUGBUG: - SetLayoutNeeded (not an issue as we explicitly call Layout below)
693-
// BUGBUG: - If we add property change events for X/Y/Width/Height they will not be invoked
714+
// We update the internal fields directly (not via the X/Y/Width/Height setters) to avoid recursion.
715+
// This is intentional and the two side effects of the setters are accounted for here (#5498):
716+
// - TextFormatter constraints: already recomputed by SetTextFormatterSize () above and finalized by
717+
// FinalizeTextFormatterConstraints () below, so no reset is needed here.
718+
// - Width/Height (and X/Y) change events: deliberately NOT raised. A layout pass resolves the Frame
719+
// without changing the declarative Dim/Pos (e.g. Dim.Fill () stays Dim.Fill ()); these assignments
720+
// only collapse an already-Absolute dim onto its resolved value. FrameChanged (raised by SetFrame
721+
// above) is the event for resolved-size changes.
694722
if (_x is PosAbsolute)
695723
{
696724
_x = Frame.X;

Terminal.Gui/Views/CharMap/CharMap.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ public CharMap ()
103103
KeyBindings.Add (Key.PageDown, Command.PageDown);
104104
KeyBindings.Add (Key.Home, Command.Start);
105105
KeyBindings.Add (Key.End, Command.End);
106-
KeyBindings.Add (PopoverMenu.DefaultKey, Command.Context);
106+
// ReplaceCommands, not Add: PopoverMenu.DefaultKey is configurable and may collide with a key
107+
// that is already bound (e.g. Ctrl+P); the user's context-menu key wins.
108+
KeyBindings.ReplaceCommands (PopoverMenu.DefaultKey, Command.Context);
107109

108110
MouseBindings.ReplaceCommands (MouseFlags.LeftButtonClicked, Command.Activate);
109111
MouseBindings.Add (MouseFlags.LeftButtonDoubleClicked, Command.Accept);

Terminal.Gui/Views/FileDialogs/FileDialog.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,10 @@ internal FileDialog (IFileSystem? fileSystem)
231231
// by default, Runnable doesn't bind to Command.Context, so
232232
// we can take advantage of the CommandNotBound event to handle it
233233
_tableView.CommandNotBound += TableViewHandleCommandNotBound;
234-
_tableView.KeyBindings.Add (PopoverMenu.DefaultKey, Command.Context);
234+
235+
// ReplaceCommands, not Add: PopoverMenu.DefaultKey is configurable and may collide with a key
236+
// TableView already binds (e.g. Ctrl+P -> Command.Up); the user's context-menu key wins.
237+
_tableView.KeyBindings.ReplaceCommands (PopoverMenu.DefaultKey, Command.Context);
235238
_tableView.MouseBindings.Add (MouseFlags.RightButtonClicked, Command.Context);
236239

237240
_tbPath.TextChanged += (_, _) => PathChanged ();

Terminal.Gui/Views/FileDialogs/FileDialogState.cs

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ namespace Terminal.Gui.Views;
44

55
internal class FileDialogState
66
{
7+
// AttributesToSkip defaults to Hidden | System; clear it so hidden/system entries (e.g. dotfiles on Unix)
8+
// remain visible, matching the behavior of the GetDirectories ()/GetFileSystemInfos () calls this replaced.
9+
private static readonly EnumerationOptions _ignoreInaccessibleEnumerationOptions = new () { IgnoreInaccessible = true, AttributesToSkip = FileAttributes.None };
10+
711
public FileDialogState (IDirectoryInfo dir, FileDialog parent)
812
{
913
Parent = parent;
@@ -60,17 +64,31 @@ private void AddReadableChildren (List<FileSystemInfoStats> children, IDirectory
6064
{
6165
try
6266
{
63-
IEnumerable<IFileSystemInfo> entries;
64-
65-
// if directories only
66-
if (Parent.OpenMode == OpenMode.Directory)
67-
{
68-
entries = dir.GetDirectories ();
69-
}
70-
else
67+
foreach (IFileSystemInfo entry in EnumerateReadableEntries (dir))
7168
{
72-
entries = dir.GetFileSystemInfos ();
69+
AddReadableChild (children, entry);
7370
}
71+
}
72+
catch (NotSupportedException)
73+
{
74+
// Some IFileSystem implementations (e.g. System.IO.Abstractions.TestingHelpers MockFileSystem)
75+
// do not support clearing EnumerationOptions.AttributesToSkip; fall back to the eager listing
76+
// methods, which never skip hidden/system entries.
77+
AddReadableChildrenEagerly (children, dir);
78+
}
79+
catch (Exception)
80+
{
81+
// Access permission exceptions, missing directories, etc.
82+
}
83+
}
84+
85+
private void AddReadableChildrenEagerly (List<FileSystemInfoStats> children, IDirectoryInfo dir)
86+
{
87+
try
88+
{
89+
IEnumerable<IFileSystemInfo> entries = Parent.OpenMode == OpenMode.Directory
90+
? dir.GetDirectories ()
91+
: dir.GetFileSystemInfos ();
7492

7593
foreach (IFileSystemInfo entry in entries)
7694
{
@@ -83,6 +101,17 @@ private void AddReadableChildren (List<FileSystemInfoStats> children, IDirectory
83101
}
84102
}
85103

104+
private IEnumerable<IFileSystemInfo> EnumerateReadableEntries (IDirectoryInfo dir)
105+
{
106+
// if directories only
107+
if (Parent.OpenMode == OpenMode.Directory)
108+
{
109+
return dir.EnumerateDirectories ("*", _ignoreInaccessibleEnumerationOptions);
110+
}
111+
112+
return dir.EnumerateFileSystemInfos ("*", _ignoreInaccessibleEnumerationOptions);
113+
}
114+
86115
private void AddReadableChild (List<FileSystemInfoStats> children, IFileSystemInfo entry)
87116
{
88117
try
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Claude - Opus 4.8
2+
3+
using System.IO.Abstractions.TestingHelpers;
4+
using Terminal.Gui.Configuration;
5+
6+
namespace ConfigurationTests;
7+
8+
/// <summary>
9+
/// Verifies that views binding <see cref="PopoverMenu.DefaultKey"/> to <see cref="Command.Context"/>
10+
/// do not throw when the configured key collides with a key the view already binds
11+
/// (e.g. Ctrl+P, which <see cref="TableView"/> binds to <see cref="Command.Up"/> by default).
12+
/// </summary>
13+
[Collection ("StaticSettingsTests")]
14+
public class PopoverMenuDefaultKeyCollisionTests
15+
{
16+
[Fact]
17+
public void FileDialog_Ctor_DoesNotThrow_WhenDefaultKeyCollidesWithTableViewBinding ()
18+
{
19+
PopoverMenuSettings original = PopoverMenuSettings.Defaults;
20+
21+
try
22+
{
23+
// TableView binds Ctrl+P -> Command.Up by default; the context-menu key must win without throwing.
24+
PopoverMenuSettings.Defaults = new () { DefaultKey = Key.P.WithCtrl };
25+
26+
MockFileSystem fs = new ();
27+
fs.AddDirectory ("/testdir");
28+
29+
using FileDialog fd = new (fs);
30+
31+
Assert.NotNull (fd);
32+
}
33+
finally
34+
{
35+
PopoverMenuSettings.Defaults = original;
36+
}
37+
}
38+
39+
[Fact]
40+
public void CharMap_Ctor_DoesNotThrow_WhenDefaultKeyCollidesWithExistingBinding ()
41+
{
42+
PopoverMenuSettings original = PopoverMenuSettings.Defaults;
43+
44+
try
45+
{
46+
// CharMap binds Key.End -> Command.End before binding the context-menu key.
47+
PopoverMenuSettings.Defaults = new () { DefaultKey = Key.End };
48+
49+
using CharMap charMap = new ();
50+
51+
Assert.NotNull (charMap);
52+
}
53+
finally
54+
{
55+
PopoverMenuSettings.Defaults = original;
56+
}
57+
}
58+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
namespace ConfigurationTests;
2+
3+
[CollectionDefinition ("StaticSettingsTests", DisableParallelization = true)]
4+
public class StaticSettingsTestCollection
5+
{
6+
// Marker collection for tests that mutate process-wide static settings facades
7+
// (e.g. DriverSettings.Defaults, PopoverMenuSettings.Defaults). Without
8+
// DisableParallelization, these tests race views constructed in parallel tests —
9+
// e.g. temporarily making PopoverMenu.DefaultKey read Ctrl+P while a FileDialog
10+
// is being constructed elsewhere.
11+
}

0 commit comments

Comments
 (0)