Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,16 @@ private void ExtendCaretBy (int delta)

## Testing tiers

Five test projects, mirroring Terminal.Gui's convention. **The parallel correctness projects run fully in parallel** — Terminal.Gui's `Application` lifetime is per-instance (`Application.Create()` returns an `IApplication` whose `Init`/`Begin`/`End`/`Dispose` track via `ThreadLocal<>`, not process globals). Tests in those projects must never call the static `Application.Init()` shortcut, and must never enable `ConfigurationManager` (`CM.Enable(...)`) — both reach for process-global state and would force serialization. Anything that must enable `ConfigurationManager` goes in `ConfigTests` (below), never the parallel projects.
Five test projects, mirroring Terminal.Gui's convention. **The parallel correctness projects run fully in parallel** — Terminal.Gui's `Application` lifetime is per-instance (`Application.Create()` returns an `IApplication` whose `Init`/`Begin`/`End`/`Dispose` track via `ThreadLocal<>`, not process globals). Tests in those projects must never call the static `Application.Init()` shortcut, and must never mutate process-global configuration facades (`TuiConfigurationBuilder.Shared`, `ApplyToStaticFacades()`, ted's `EditorSettings.Defaults`) — both reach for process-global state and would force serialization. Anything that must mutate those facades goes in `ConfigTests` (below), never the parallel projects.

- `Terminal.Gui.Editor.Tests` — pure, no UI, no static state. Target ≥90% coverage. Runs in `ci.yml`.
- `Terminal.Gui.Editor.IntegrationTests` — full key-input → render scenarios via `AppFixture<T>`, which boots a per-test `IApplication` from `Application.Create()`. Parallel by default. Runs in `ci.yml`.
- `Terminal.Gui.Editor.ConfigTests` — **ConfigurationManager only.** CM is process-global with one-time `[ConfigurationProperty]` discovery, so it cannot share a process with parallel tests (a `DisableParallelization` collection fixes concurrency but not cross-collection discovery order). This project's `xunit.runner.json` disables assembly **and** collection parallelization — the same quarantine Terminal.Gui uses for its own CM suite. Put any test that calls `ConfigurationManager.Enable/Load/Apply` (e.g. verifying ted's `AppSettingsScope` settings round-trip) here, and nothing else. Runs in `ci.yml`.
- `Terminal.Gui.Editor.ConfigTests` — **process-global configuration only.** Terminal.Gui 2.5's configuration facades (`TuiConfigurationBuilder` static application, ted's `EditorSettings.Defaults`) are process-global, so tests that apply configuration to them cannot share a process with parallel tests. This project's `xunit.runner.json` disables assembly **and** collection parallelization — the same quarantine Terminal.Gui used for its old CM suite. Put any test that applies configuration to process-global facades (e.g. verifying ted's `EditorSettings` round-trip) here, and nothing else. Runs in `ci.yml`.
- `Terminal.Gui.Editor.PerformanceTests` — stopwatch-based perf smoke tests. **Release only, ubuntu-latest only.** Lives in its own project and its own workflow (`.github/workflows/perf.yml`) because Windows/macOS GitHub-hosted runners are too noisy for wall-time assertions. The BenchmarkDotNet suite in `benchmarks/` runs from the same workflow.

New tests default to the parallel-by-name project. Promote to `IntegrationTests` only when an `IApplication` (driver, input injection, full layout/draw) is genuinely needed. Promote to `PerformanceTests` only when you need a wall-time assertion — and remember it won't run on Windows/macOS CI, so don't put correctness checks there.

**The one allowed exception:** a test that legitimately mutates a process-global (e.g. `Logging.Logger`, `Trace.EnabledCategories`, anything `static`) must opt out of cross-collection parallelism via a `[CollectionDefinition(name, DisableParallelization = true)]` + `[Collection(name)]` pair. See `tests/Terminal.Gui.Editor.IntegrationTests/HostingTests.cs` for the canonical example. Do **not** add an assembly-wide `xunit.runner.json` to make a *shared* correctness project serial — that's the wrong tool for one offending class. (The sole exception is the purpose-built `ConfigTests` project, which is CM-only and serial *by design* — that is not "serializing a shared project", it is the quarantine.)
**The one allowed exception:** a test that legitimately mutates a process-global (e.g. `Logging.Logger`, `Trace.EnabledCategories`, anything `static`) must opt out of cross-collection parallelism via a `[CollectionDefinition(name, DisableParallelization = true)]` + `[Collection(name)]` pair. See `tests/Terminal.Gui.Editor.IntegrationTests/HostingTests.cs` for the canonical example. Do **not** add an assembly-wide `xunit.runner.json` to make a *shared* correctness project serial — that's the wrong tool for one offending class. (The sole exception is the purpose-built `ConfigTests` project, which is config-facade-only and serial *by design* — that is not "serializing a shared project", it is the quarantine.)

### Performance gates

Expand All @@ -242,7 +242,7 @@ The full BenchmarkDotNet matrix (`Scrolling`, `EndToEndScroll`, `CaretMovement`,
When integration tests run individually but hang when run as a suite, **the cause is almost always shared mutable state, not the parallelism itself**. Do not reach for `xunit.runner.json` with `parallelizeTestCollections: false` to "fix" it — that hides the bug and slows the suite. Walk this checklist instead:

1. **Static `Application.Init()`?** Grep for `Application.Init` (without an `IApplication` receiver). It must always be `app.Init()` on an `IApplication` from `Application.Create()`. The static form is a process-global Init/Shutdown pair and serializes everything.
2. **`ConfigurationManager.Enable(...)`?** Grep for `ConfigurationManager.Enable` / `CM.Enable`. CM is a process-global config store; tests must never enable it. Enabling it from one test poisons every concurrent `Application.Create()`.
2. **Applying configuration to process-global facades?** Grep for `ApplyToStaticFacades` / `EditorSettings.Apply` / `EditorSettings.Defaults =`. Those are process-global config stores; tests outside `ConfigTests` must never mutate them. Doing so from one test poisons every concurrent `Application.Create()`.
3. **Mutating TG-read process globals?** `Logging.Logger`, `Trace.EnabledCategories`, and anything `static` on `Application`/`Terminal.Gui.*` that TG itself reads during draw or lifecycle. A test that swaps these (even with try/finally restore) will deadlock or corrupt parallel tests because TG running on another test's thread reads the half-set value.
4. **A new `View` subclass touching shared state?** Subscribing to a static event, allocating from a static cache, etc.

Expand Down
3 changes: 2 additions & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
Override per-build via -p:TerminalGuiVersion=<x>; use -p:UseLocalTerminalGui=true
to build against the ../Terminal.Gui enlistment instead (see Directory.Build.targets).
-->
<TerminalGuiVersion Condition="'$(TerminalGuiVersion)' == ''">2.4.18-develop.5</TerminalGuiVersion>
<!-- Public Terminal.Gui 2.5.0-beta.1 (TG #5630 ecosystem gate). -->
<TerminalGuiVersion Condition="'$(TerminalGuiVersion)' == ''">2.5.0-beta.1</TerminalGuiVersion>
</PropertyGroup>

<ItemGroup>
Expand Down
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ For a user-facing editor built on this library, se [clet](https:/github.com/tui-

`Editor` is a `View`. The standard TG machinery applies:

- **Commands + keybindings.** All editor actions are `Command` bindings, remappable via `KeyBindings` or ConfigurationManager.
- **Commands + keybindings.** All editor actions are `Command` bindings, remappable via `KeyBindings` or `TuiConfigurationBuilder`.
- **Themes.** Colors come from the active `Scheme`; switch themes at runtime and the editor reflows.
- **Layout.** `Pos` / `Dim` constraints; `Padding` / `Border` / `Margin` adornments (the gutter is a `View` inside `Padding`).
- **Scrollbars.** Set `ViewportSettings = ViewportSettingsFlags.HasScrollBars`.
Expand Down Expand Up @@ -100,7 +100,7 @@ For a user-facing editor built on this library, se [clet](https:/github.com/tui-

### Default keybindings

These are the *defaults*. They are `Command`-bound and remappable via TG's `KeyBindings` API or ConfigurationManager.
These are the *defaults*. They are `Command`-bound and remappable via TG's `KeyBindings` API or `TuiConfigurationBuilder`.

| Command | Default key | Notes |
|---|---|---|
Expand Down Expand Up @@ -135,7 +135,7 @@ Install the package (requires the .NET 10 SDK and Terminal.Gui):
dotnet add package Terminal.Gui.Editor
```

Drop the editor into a Terminal.Gui app. `Editor` is just a `View`, so it gets TG's layout, scheme, scrollbars, and Configuration Manager for free:
Drop the editor into a Terminal.Gui app. `Editor` is just a `View`, so it gets TG's layout, scheme, scrollbars, and `TuiConfigurationBuilder` for free:

```csharp
using Terminal.Gui.App;
Expand All @@ -146,7 +146,8 @@ using Terminal.Gui.Highlighting;
using Terminal.Gui.ViewBase;

// Pick up themes / keymaps / preferences from the user's TG config.
ConfigurationManager.Enable (ConfigLocations.All);
TuiConfigurationBuilder config = new ("MyEditor");
config.ApplyToStaticFacades ();

using IApplication app = Application.Create ();
app.Init ();
Expand Down
21 changes: 4 additions & 17 deletions examples/ted/EditorSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,15 @@
using System.Text.Json.Nodes;
using Microsoft.Extensions.Configuration;
using Terminal.Gui.App;
using Terminal.Gui.Configuration;

namespace Ted;

#pragma warning disable CS0618 // Keep legacy CM attributes until Terminal.Gui fully removes CM.

/// <summary>
/// ted's persisted editor settings. Microsoft.Extensions.Configuration is the primary read path:
/// ted's persisted editor settings. Microsoft.Extensions.Configuration is the read path:
/// startup loads <c>~/.tui/ted.config.json</c> and applies the values to these static properties
/// before <see cref="TedApp" /> is constructed. Legacy CM attributes are retained only so older
/// Terminal.Gui builds can still apply the previous <see cref="AppSettingsScope" /> format.
/// before <see cref="TedApp" /> is constructed. Terminal.Gui 2.5 removed the legacy
/// ConfigurationManager; <see cref="Apply" /> still migrates the old flat
/// <c>"EditorSettings.*"</c> and CM <c>"AppSettings"</c> shapes on read.
/// <para>
/// <see cref="Save(string)" /> writes the MEC-native shape:
/// <c>"EditorSettings": { "WordWrap": true }</c>. Other top-level keys a user may have added
Expand All @@ -26,63 +24,54 @@ internal static class EditorSettings
{
internal const string SectionName = "EditorSettings";

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool LineNumbers
{
get => Defaults.LineNumbers;
set => Defaults.LineNumbers = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool FoldIndicators
{
get => Defaults.FoldIndicators;
set => Defaults.FoldIndicators = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool WordWrap
{
get => Defaults.WordWrap;
set => Defaults.WordWrap = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool ShowTabs
{
get => Defaults.ShowTabs;
set => Defaults.ShowTabs = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static int IndentSize
{
get => Defaults.IndentSize;
set => Defaults.IndentSize = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool ConvertTabsToSpaces
{
get => Defaults.ConvertTabsToSpaces;
set => Defaults.ConvertTabsToSpaces = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool AutoIndent
{
get => Defaults.AutoIndent;
set => Defaults.AutoIndent = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool Scrollbars
{
get => Defaults.Scrollbars;
set => Defaults.Scrollbars = value;
}

[ConfigurationProperty (Scope = typeof (AppSettingsScope))]
public static bool AutoComplete
{
get => Defaults.AutoComplete;
Expand Down Expand Up @@ -283,5 +272,3 @@ internal sealed class EditorSettingsValues
public bool AutoComplete { get; set; }
}
}

#pragma warning restore CS0618
8 changes: 5 additions & 3 deletions examples/ted/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
Hosting.EnableTracing ();

// Load settings through Terminal.Gui's Microsoft.Extensions.Configuration builder
// (TuiConfigurationBuilder), applied before TedApp is constructed. Requires Terminal.Gui
// >= 2.4.15 (the TerminalGuiVersion pin); there is no ConfigurationManager fallback.
// (TuiConfigurationBuilder), applied before TedApp is constructed. Terminal.Gui 2.5
// removed the legacy ConfigurationManager; this is the only config path.
TerminalGuiConfigurationBootstrap.Apply ();

using IApplication app = Application.Create ();
Expand Down Expand Up @@ -40,7 +40,9 @@
{
// Synchronous (non-marshalled) load completes before app.Run, so the very first paint
// shows the document — no blank-buffer-then-fill flash for the common small-file case.
ted.OpenFileAsync (requestedPath).GetAwaiter ().GetResult ();
// OpenFileBlocking clears Terminal.Gui 2.5's main-loop SynchronizationContext (installed
// at Init, tui-cs/Terminal.Gui#5588) for the wait so it cannot deadlock on continuations.
ted.OpenFileBlocking (requestedPath);
}
else
{
Expand Down
40 changes: 37 additions & 3 deletions examples/ted/TedApp.FileOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,41 @@ public bool OpenFile ()
{
var filePath = ShowOpenDialog ();

return !string.IsNullOrWhiteSpace (filePath) && OpenFileAsync (filePath).GetAwaiter ().GetResult ();
return !string.IsNullOrWhiteSpace (filePath) && RunSyncBridge (() => OpenFileAsync (filePath));
}

/// <summary>
/// Synchronously loads <paramref name="filePath" /> into the editor, blocking without
/// deadlocking on Terminal.Gui's main-loop <see cref="SynchronizationContext" /> (see
/// <see cref="RunSyncBridge" />).
/// </summary>
internal bool OpenFileBlocking (string filePath)
{
return RunSyncBridge (() => OpenFileAsync (filePath));
}

/// <summary>
/// Blocks on <paramref name="operation" /> without deadlocking on Terminal.Gui's
/// <see cref="SynchronizationContext" />. Terminal.Gui 2.5 installs its main-loop context at
/// <c>Init</c> (tui-cs/Terminal.Gui#5588); awaits inside <paramref name="operation" /> would
/// otherwise post continuations to the very thread this method blocks. Clearing the ambient
/// context for the call restores the pre-2.5 behavior (continuations run on the thread pool)
/// while the synchronous prefix — including <see cref="TextDocument" /> owner-thread handoff —
/// still runs on the calling thread.
/// </summary>
private bool RunSyncBridge (Func<Task<bool>> operation)
{
SynchronizationContext? previous = SynchronizationContext.Current;
SynchronizationContext.SetSynchronizationContext (null);

try
{
return operation ().GetAwaiter ().GetResult ();
}
finally
{
SynchronizationContext.SetSynchronizationContext (previous);
}
}

/// <summary>Prompts for a file path, then asynchronously streams that file into the editor.</summary>
Expand Down Expand Up @@ -163,7 +197,7 @@ public void OpenMissingFile (string filePath)
/// <summary>Saves the editor text to the current file, or prompts for a path if the buffer is untitled.</summary>
public bool SaveFile ()
{
return CurrentFilePath is null ? SaveFileAs () : SaveFileAsync ().GetAwaiter ().GetResult ();
return CurrentFilePath is null ? SaveFileAs () : RunSyncBridge (() => SaveFileAsync ());
}

/// <summary>Asynchronously streams the editor text to the current file, or prompts for a path if untitled.</summary>
Expand Down Expand Up @@ -200,7 +234,7 @@ public bool SaveFileAs ()
{
var filePath = ShowSaveDialog ();

return !string.IsNullOrWhiteSpace (filePath) && SaveFileAsAsync (filePath).GetAwaiter ().GetResult ();
return !string.IsNullOrWhiteSpace (filePath) && RunSyncBridge (() => SaveFileAsAsync (filePath));
}

private async Task<bool> SaveFileAsAsync (bool marshalToApp, CancellationToken cancellationToken = default)
Expand Down
4 changes: 2 additions & 2 deletions examples/ted/ted.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="10.0.7" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="10.0.7" />
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="10.0.11" />
<PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="10.0.11" />
<ProjectReference Include="..\..\src\Terminal.Gui.Editor\Terminal.Gui.Editor.csproj" />
<PackageReference Include="Terminal.Gui" Version="$(TerminalGuiVersion)" />
<PackageReference Include="Serilog" Version="4.3.1" />
Expand Down
37 changes: 29 additions & 8 deletions src/Terminal.Gui.Editor/Editor.Completion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,10 @@ internal bool HandleCompletionMouse (Mouse mouse)
return false;
}

// Map the click's screen position to the Popover's content area.
// The ListView's frame within the Popover determines the hit region.
Rectangle popoverScreenFrame = _completionPopover.Frame;
// Map the click's screen position to the popup's content area. Terminal.Gui 2.5's
// Popover is a screen-filling transparent overlay; the visible popup rectangle is
// the ContentView (the ListView), positioned in screen coordinates.
Rectangle popoverScreenFrame = _completionListView.FrameToScreen ();

if (mouse.ScreenPosition.X < popoverScreenFrame.X
|| mouse.ScreenPosition.X >= popoverScreenFrame.Right
Expand Down Expand Up @@ -449,16 +450,36 @@ private void ShowCompletionPopup ()
}
};

// Accepted fires on BOTH Enter and mouse-click. Acceptance itself is driven
// explicitly — HandleCompletionKey for Enter/Tab, HandleCompletionMouse for a
// click — so this only syncs the selected index (like ValueChanged above).
// Calling AcceptCompletion here double-handled Enter and leaked a trailing newline.
_completionListView.Accepted += (_, args) =>
// Accepting fires on BOTH Enter and mouse-click. Key-driven acceptance is handled
// explicitly by HandleCompletionKey (Enter/Tab) — calling AcceptCompletion here for
// keys double-handled Enter and leaked a trailing newline — so keys only sync the
// selected index (like ValueChanged above). Mouse clicks are different in TG 2.5:
// the screen-filling Popover overlay routes popup clicks to the ListView, so the
// Editor's OnMouseEvent/HandleCompletionMouse never sees them — accept here when
// the Accept came from a mouse binding. Mark Handled so Popover does not bridge
// Command.Accept to the Editor Target (that would submit a hosting dialog).
_completionListView.Accepting += (sender, args) =>
{
if (args.Context?.Value is int idx)
{
CompletionSelectedIndex = idx;
}

if (args.Context?.Binding is not MouseBinding { MouseEvent: { Position: { } clickPosition } })
{
return;
}

// The click's Position is ListView-viewport-relative; honor scroll via Viewport.Y.
var clickedIdx = clickPosition.Y + ((ListView)sender!).Viewport.Y;

if (clickedIdx >= 0 && clickedIdx < _completionItems.Count)
{
CompletionSelectedIndex = clickedIdx;
}

AcceptCompletion ();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent completion clicks from bubbling the Accept command

When the editor is hosted where Command.Accept submits a form or closes a dialog, a completion click now has two effects: this handler inserts the completion, while the same ListView.Accepted event is automatically bridged by the 2.5 Popover to its Target (Editor). Thus selecting a suggestion can also raise/bubble Accept on the editor and unexpectedly submit its container; handle the mouse click without invoking the ListView's Command.Accept, or consume it before the popover bridge runs.

Useful? React with 👍 / 👎.

args.Handled = true;
};
}

Expand Down
Loading
Loading