From 6b18c525d54f13346aad70c46809aeff458cedf6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 14:27:16 +0000 Subject: [PATCH 01/32] Initial plan From 087d4fe918335257dcbd7f998127375652c83923 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 14:43:06 +0000 Subject: [PATCH 02/32] feat: add in-editor completion/autocomplete popup (IEditorCompletionProvider, CompletionItem, Editor.Completion.cs, WordCompletionProvider in ted) Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/dddd5eaf-c7d7-40bb-866d-9961401a391b Co-authored-by: tig <585482+tig@users.noreply.github.com> --- examples/ted/TedApp.cs | 2 + examples/ted/WordCompletionProvider.cs | 74 +++++ .../Completion/CompletionItem.cs | 30 ++ .../Completion/IEditorCompletionProvider.cs | 37 +++ src/Terminal.Gui.Editor/Editor.Commands.cs | 24 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 291 ++++++++++++++++++ src/Terminal.Gui.Editor/Editor.Keyboard.cs | 18 ++ .../EditorCompletionTests.cs | 186 +++++++++++ 8 files changed, 658 insertions(+), 4 deletions(-) create mode 100644 examples/ted/WordCompletionProvider.cs create mode 100644 src/Terminal.Gui.Editor/Completion/CompletionItem.cs create mode 100644 src/Terminal.Gui.Editor/Completion/IEditorCompletionProvider.cs create mode 100644 src/Terminal.Gui.Editor/Editor.Completion.cs create mode 100644 tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs diff --git a/examples/ted/TedApp.cs b/examples/ted/TedApp.cs index 1c6e97a..e54a6a8 100644 --- a/examples/ted/TedApp.cs +++ b/examples/ted/TedApp.cs @@ -6,6 +6,7 @@ using Terminal.Gui.Document.Folding; using Terminal.Gui.Drawing; using Terminal.Gui.Editor; +using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; using Terminal.Gui.Resources; using Terminal.Gui.Text.Indentation; @@ -40,6 +41,7 @@ public TedApp (bool readOnly = false) WordWrap = EditorSettings.WordWrap, ShowTabs = EditorSettings.ShowTabs, ReadOnly = readOnly, + CompletionProvider = new WordCompletionProvider (), ViewportSettings = ViewportSettingsFlags.HasScrollBars }; diff --git a/examples/ted/WordCompletionProvider.cs b/examples/ted/WordCompletionProvider.cs new file mode 100644 index 0000000..d3e8476 --- /dev/null +++ b/examples/ted/WordCompletionProvider.cs @@ -0,0 +1,74 @@ +using Terminal.Gui.Document; +using Terminal.Gui.Editor.Completion; +using Terminal.Gui.Input; + +namespace Ted; + +/// +/// A trivial word-completion provider for the ted demo. Scans the document for unique +/// word tokens (letters, digits, underscores) and offers them as suggestions when the prefix +/// matches. Triggered by Ctrl+Space. +/// +internal sealed class WordCompletionProvider : IEditorCompletionProvider +{ + /// + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + if (string.IsNullOrEmpty (prefix)) + { + return []; + } + + var text = document.Text; + HashSet seen = new (StringComparer.OrdinalIgnoreCase); + List results = []; + + // Walk the document text for word tokens. + var i = 0; + + while (i < text.Length) + { + if (!IsWordChar (text[i])) + { + i++; + + continue; + } + + var start = i; + + while (i < text.Length && IsWordChar (text[i])) + { + i++; + } + + var word = text.Substring (start, i - start); + + // Skip the exact prefix and short tokens. + if (word.Length <= prefix.Length || string.Equals (word, prefix, StringComparison.Ordinal)) + { + continue; + } + + if (word.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) && seen.Add (word)) + { + results.Add (new CompletionItem { Label = word }); + } + } + + results.Sort ((a, b) => string.Compare (a.Label, b.Label, StringComparison.OrdinalIgnoreCase)); + + return results; + } + + /// + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } + + private static bool IsWordChar (char ch) + { + return char.IsLetterOrDigit (ch) || ch == '_'; + } +} diff --git a/src/Terminal.Gui.Editor/Completion/CompletionItem.cs b/src/Terminal.Gui.Editor/Completion/CompletionItem.cs new file mode 100644 index 0000000..d97c01c --- /dev/null +++ b/src/Terminal.Gui.Editor/Completion/CompletionItem.cs @@ -0,0 +1,30 @@ +namespace Terminal.Gui.Editor.Completion; + +/// +/// A single completion suggestion returned by an . +/// Modelled after the LSP CompletionItem shape — label + insertText + optional detail — +/// but kept minimal for the terminal context. +/// +public sealed class CompletionItem +{ + /// + /// The display text shown in the completion popup. This is the primary string the user sees + /// when filtering suggestions. + /// + public required string Label { get; init; } + + /// + /// The text inserted into the document when this item is accepted. When , + /// is inserted instead. + /// + public string? InsertText { get; init; } + + /// + /// An optional secondary description shown alongside the label (e.g. a type signature or + /// source module). + /// + public string? Detail { get; init; } + + /// The text that will actually be inserted: ?? . + internal string TextToInsert => InsertText ?? Label; +} diff --git a/src/Terminal.Gui.Editor/Completion/IEditorCompletionProvider.cs b/src/Terminal.Gui.Editor/Completion/IEditorCompletionProvider.cs new file mode 100644 index 0000000..0eb5896 --- /dev/null +++ b/src/Terminal.Gui.Editor/Completion/IEditorCompletionProvider.cs @@ -0,0 +1,37 @@ +using Terminal.Gui.Document; +using Terminal.Gui.Input; + +namespace Terminal.Gui.Editor.Completion; + +/// +/// Provides completion suggestions for a document at a given caret position. Consumers implement +/// this interface and assign an instance to to enable +/// in-editor completion. +/// +/// +/// The provider is queried synchronously on every triggering keystroke. Keep +/// fast — pre-index if necessary. +/// +public interface IEditorCompletionProvider +{ + /// + /// Returns completion items for the current caret position, or an empty list when no + /// suggestions are available. The is the word fragment + /// immediately before the caret that the editor extracted from the document. + /// + /// The document being edited. + /// Current caret offset in the document. + /// + /// The word fragment before the caret (letters/digits/underscores back to the nearest + /// non-word character or line start). Empty when the caret follows whitespace or punctuation. + /// + /// An ordered list of suggestions. The first item is pre-selected in the popup. + IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix); + + /// + /// Returns when the given key should trigger the completion popup + /// (e.g. Ctrl+Space). The editor calls this before normal key dispatch; if the + /// provider claims the key, the popup opens (or re-filters). + /// + bool ShouldTrigger (Key key); +} diff --git a/src/Terminal.Gui.Editor/Editor.Commands.cs b/src/Terminal.Gui.Editor/Editor.Commands.cs index 732a34f..6bd1a50 100644 --- a/src/Terminal.Gui.Editor/Editor.Commands.cs +++ b/src/Terminal.Gui.Editor/Editor.Commands.cs @@ -170,10 +170,26 @@ private void CreateCommandsAndBindings () return true; }); - // Editing — selection-aware (multi-caret aware) - AddCommand (Command.NewLine, MultiCaretNewLine); - AddCommand (Command.DeleteCharLeft, MultiCaretDeleteLeft); - AddCommand (Command.DeleteCharRight, MultiCaretDeleteRight); + // Editing — selection-aware (multi-caret aware), with completion notification. + AddCommand (Command.NewLine, () => + { + DismissCompletion (); + + return MultiCaretNewLine (); + }); + AddCommand (Command.DeleteCharLeft, () => + { + bool? result = MultiCaretDeleteLeft (); + NotifyCompletionAfterInsert (); + + return result; + }); + AddCommand (Command.DeleteCharRight, () => + { + DismissCompletion (); + + return MultiCaretDeleteRight (); + }); // History AddCommand (Command.Undo, () => diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs new file mode 100644 index 0000000..279e0bb --- /dev/null +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -0,0 +1,291 @@ +using System.Drawing; +using Terminal.Gui.Editor.Completion; +using Terminal.Gui.Input; +using Terminal.Gui.ViewBase; +using Terminal.Gui.Views; + +namespace Terminal.Gui.Editor; + +public partial class Editor +{ + private IReadOnlyList _completionItems = []; + private int _completionPrefixStart; + private PopoverMenu? _completionPopup; + private int _completionSelectedIndex; + + /// + /// Gets or sets the completion provider that supplies suggestions for the in-editor + /// autocomplete popup. Set to to disable completion. + /// + public IEditorCompletionProvider? CompletionProvider { get; set; } + + /// Whether the completion session is currently active (items are available). + public bool IsCompletionActive => _completionItems.Count > 0; + + /// + /// Extracts the word-prefix immediately before the caret (letters, digits, underscores) + /// for use as the completion filter string. Returns empty when the caret follows + /// whitespace or punctuation. + /// + internal string GetCompletionPrefix () + { + return GetCompletionPrefix (out _); + } + + /// + /// Extracts the word-prefix immediately before the caret and also returns the document + /// offset where the prefix starts. + /// + internal string GetCompletionPrefix (out int prefixStart) + { + if (_document is null) + { + prefixStart = 0; + + return string.Empty; + } + + var offset = CaretOffset; + var start = offset; + + while (start > 0) + { + var ch = _document.GetCharAt (start - 1); + + if (!char.IsLetterOrDigit (ch) && ch != '_') + { + break; + } + + start--; + } + + prefixStart = start; + + return start < offset ? _document.GetText (start, offset - start) : string.Empty; + } + + /// + /// Called before normal key dispatch. Returns when the completion + /// popup consumed the key (navigation / accept / dismiss / trigger keys). + /// + internal bool HandleCompletionKey (Key key) + { + if (CompletionProvider is null) + { + return false; + } + + // An active popup gets first crack at navigation keys. + if (IsCompletionActive) + { + if (key == Key.Esc) + { + DismissCompletion (); + + return true; + } + + if (key == Key.Enter || key == Key.Tab) + { + AcceptCompletion (); + + return true; + } + + if (key == Key.CursorUp) + { + SelectCompletionItem ((_completionSelectedIndex - 1 + _completionItems.Count) % _completionItems.Count); + + return true; + } + + if (key == Key.CursorDown) + { + SelectCompletionItem ((_completionSelectedIndex + 1) % _completionItems.Count); + + return true; + } + } + + // Check provider-specific triggers (e.g. Ctrl+Space). + if (CompletionProvider.ShouldTrigger (key)) + { + ShowCompletion (); + + return true; + } + + return false; + } + + /// + /// Called after a character is inserted into the document. Refreshes or opens the + /// completion popup if a provider is active. + /// + internal void NotifyCompletionAfterInsert () + { + if (CompletionProvider is null) + { + return; + } + + var prefix = GetCompletionPrefix (out var prefixStart); + + if (prefix.Length == 0) + { + DismissCompletion (); + + return; + } + + IReadOnlyList items = + CompletionProvider.GetCompletions (_document!, CaretOffset, prefix); + + if (items.Count == 0) + { + DismissCompletion (); + + return; + } + + _completionPrefixStart = prefixStart; + _completionItems = items; + _completionSelectedIndex = 0; + ShowCompletionPopup (); + } + + /// Opens the completion popup, querying the provider for items. + internal void ShowCompletion () + { + if (CompletionProvider is null || _document is null) + { + return; + } + + var prefix = GetCompletionPrefix (out var prefixStart); + + IReadOnlyList items = + CompletionProvider.GetCompletions (_document, CaretOffset, prefix); + + if (items.Count == 0) + { + DismissCompletion (); + + return; + } + + _completionPrefixStart = prefixStart; + _completionItems = items; + _completionSelectedIndex = 0; + ShowCompletionPopup (); + } + + /// Hides the completion popup if it is visible. + internal void DismissCompletion () + { + if (_completionPopup is not null) + { + _completionPopup.Visible = false; + } + + _completionItems = []; + } + + /// + /// Accepts the currently selected completion item: replaces the prefix with the + /// item's insert text inside a single undo group. + /// + internal void AcceptCompletion () + { + if (!IsCompletionActive || _document is null || _completionItems.Count == 0) + { + return; + } + + if (_completionSelectedIndex < 0 || _completionSelectedIndex >= _completionItems.Count) + { + DismissCompletion (); + + return; + } + + CompletionItem selected = _completionItems[_completionSelectedIndex]; + var insertText = selected.TextToInsert; + var replaceLength = CaretOffset - _completionPrefixStart; + + DismissCompletion (); + + // Single undo step for the replacement. + using (_document.RunUpdate ()) + { + if (replaceLength > 0) + { + _document.Replace (_completionPrefixStart, replaceLength, insertText); + } + else + { + _document.Insert (CaretOffset, insertText); + } + } + } + + private void ShowCompletionPopup () + { + Point caretScreen = GetCaretScreenPosition (); + + // Build menu items from the completion items. + var menuItems = new MenuItem[_completionItems.Count]; + + for (var i = 0; i < _completionItems.Count; i++) + { + CompletionItem item = _completionItems[i]; + var label = i == _completionSelectedIndex ? $"> {item.Label}" : $" {item.Label}"; + menuItems[i] = new MenuItem { Title = label }; + } + + // Dispose previous popup if any — create fresh each time so the Root is rebuilt. + if (_completionPopup is not null) + { + _completionPopup.Visible = false; + } + + _completionPopup = new PopoverMenu (menuItems) + { + Target = new WeakReference (this) + }; + + // Position the popup just below the caret. + _completionPopup.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); + } + + private void SelectCompletionItem (int index) + { + if (_completionItems.Count == 0) + { + return; + } + + _completionSelectedIndex = index; + ShowCompletionPopup (); + } + + /// + /// Computes the screen position of the caret (for popup anchoring). + /// + private Point GetCaretScreenPosition () + { + if (_document is null) + { + return Point.Empty; + } + + Rectangle viewport = Viewport; + var caretLine = GetCaretVisibleLineIndex (); + var caretCol = GetCaretColumn (); + var row = caretLine - viewport.Y; + var col = caretCol - viewport.X; + + return ViewportToScreen (new Point (col, row)); + } +} diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 7f48404..b88e89c 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -13,6 +13,12 @@ public partial class Editor /// protected override bool OnKeyDownNotHandled (Key key) { + // Completion popup gets first priority for navigation / accept / dismiss / trigger keys. + if (HandleCompletionKey (key)) + { + return true; + } + if (key == Key.Esc && HasMultipleCarets) { ClearAdditionalCarets (); @@ -20,6 +26,15 @@ protected override bool OnKeyDownNotHandled (Key key) return true; } + // Esc dismisses an active completion (handled above); when no popup is active, let it + // fall through to multi-caret clear or default handling. + if (key == Key.Esc && IsCompletionActive) + { + DismissCompletion (); + + return true; + } + if (key.IsCtrl || key.IsAlt) { return false; @@ -52,6 +67,9 @@ protected override bool OnKeyDownNotHandled (Key key) _document!.Insert (CaretOffset, rune.ToString ()); } + // After inserting a character, notify the completion system so it can open / filter. + NotifyCompletionAfterInsert (); + return true; } } diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs new file mode 100644 index 0000000..f1d854a --- /dev/null +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -0,0 +1,186 @@ +// CoPilot - gpt-4.1 + +using Terminal.Gui.Document; +using Terminal.Gui.Editor; +using Terminal.Gui.Editor.Completion; +using Terminal.Gui.Input; +using Xunit; + +namespace Terminal.Gui.Editor.Tests; + +/// +/// Tests for completion logic — prefix extraction, provider querying, +/// accept/dismiss, and single-undo-step guarantee. No needed. +/// +public class EditorCompletionTests +{ + [Fact] + public void GetCompletionPrefix_Returns_Empty_On_Empty_Document () + { + Editor editor = new (); + + var prefix = editor.GetCompletionPrefix (); + + Assert.Equal (string.Empty, prefix); + } + + [Fact] + public void GetCompletionPrefix_Extracts_WordBefore_Caret () + { + Editor editor = new () { Document = new TextDocument ("hello world") }; + editor.CaretOffset = 5; // after "hello" + + var prefix = editor.GetCompletionPrefix (out var start); + + Assert.Equal ("hello", prefix); + Assert.Equal (0, start); + } + + [Fact] + public void GetCompletionPrefix_Stops_At_Punctuation () + { + Editor editor = new () { Document = new TextDocument ("foo.bar") }; + editor.CaretOffset = 7; // after "bar" + + var prefix = editor.GetCompletionPrefix (out var start); + + Assert.Equal ("bar", prefix); + Assert.Equal (4, start); + } + + [Fact] + public void GetCompletionPrefix_Returns_Empty_AfterWhitespace () + { + Editor editor = new () { Document = new TextDocument ("hello ") }; + editor.CaretOffset = 6; // after space + + var prefix = editor.GetCompletionPrefix (); + + Assert.Equal (string.Empty, prefix); + } + + [Fact] + public void GetCompletionPrefix_Includes_Underscores_And_Digits () + { + Editor editor = new () { Document = new TextDocument ("my_var2 ") }; + editor.CaretOffset = 7; // after "my_var2" + + var prefix = editor.GetCompletionPrefix (); + + Assert.Equal ("my_var2", prefix); + } + + [Fact] + public void CompletionProvider_Default_Is_Null () + { + Editor editor = new (); + + Assert.Null (editor.CompletionProvider); + Assert.False (editor.IsCompletionActive); + } + + [Fact] + public void AcceptCompletion_Replaces_Prefix_In_SingleUndoStep () + { + Editor editor = new () + { + Document = new TextDocument ("hel"), + CompletionProvider = new StubCompletionProvider ("hello_world") + }; + editor.CaretOffset = 3; // after "hel" + + // NotifyCompletionAfterInsert sets up the items and prefix state. + editor.NotifyCompletionAfterInsert (); + + // Accept the completion — uses the prefix state set by Notify. + editor.AcceptCompletion (); + Assert.Equal ("hello_world", editor.Document!.Text); + + // Single undo step should restore the original text. + editor.Document!.UndoStack.Undo (); + Assert.Equal ("hel", editor.Document!.Text); + } + + [Fact] + public void DismissCompletion_Clears_State () + { + Editor editor = new () + { + Document = new TextDocument ("hel"), + CompletionProvider = new StubCompletionProvider ("hello") + }; + editor.CaretOffset = 3; + + editor.NotifyCompletionAfterInsert (); + editor.DismissCompletion (); + + // After dismiss, AcceptCompletion should be a no-op. + editor.AcceptCompletion (); + Assert.Equal ("hel", editor.Document!.Text); + } + + [Fact] + public void ShowCompletion_NoOp_Without_Provider () + { + Editor editor = new () { Document = new TextDocument ("hel") }; + editor.CaretOffset = 3; + + editor.ShowCompletion (); + Assert.False (editor.IsCompletionActive); + } + + [Fact] + public void ShowCompletion_NoOp_When_Provider_Returns_Empty () + { + Editor editor = new () + { + Document = new TextDocument ("hel"), + CompletionProvider = new EmptyCompletionProvider () + }; + editor.CaretOffset = 3; + + editor.ShowCompletion (); + Assert.False (editor.IsCompletionActive); + } + + [Fact] + public void HandleCompletionKey_Returns_False_Without_Provider () + { + Editor editor = new (); + + Assert.False (editor.HandleCompletionKey (Key.Esc)); + } + + /// Stub provider that always returns a single hard-coded item. + private sealed class StubCompletionProvider : IEditorCompletionProvider + { + private readonly string _word; + + public StubCompletionProvider (string word) { _word = word; } + + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + if (string.IsNullOrEmpty (prefix)) + { + return []; + } + + return _word.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) + ? [new CompletionItem { Label = _word }] + : []; + } + + public bool ShouldTrigger (Key key) { return key == Key.Space.WithCtrl; } + } + + /// Provider that always returns an empty list. + private sealed class EmptyCompletionProvider : IEditorCompletionProvider + { + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + return []; + } + + public bool ShouldTrigger (Key key) { return false; } + } +} From d42c176e73d628d8ca2a19f76bf12e84c4c0f9b3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 14:48:22 +0000 Subject: [PATCH 03/32] fix: dispose PopoverMenu before recreating; fix DEC-009 reference in decisions.md; apply jb cleanup Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/dddd5eaf-c7d7-40bb-866d-9961401a391b Co-authored-by: tig <585482+tig@users.noreply.github.com> --- examples/ted/TedApp.cs | 1 - specs/completion/spec.md | 89 +++++++++++++++++++ specs/decisions.md | 16 +++- specs/public-api.md | 25 +++++- src/Terminal.Gui.Editor/Editor.Commands.cs | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 7 +- .../EditorCompletionTests.cs | 1 - 7 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 specs/completion/spec.md diff --git a/examples/ted/TedApp.cs b/examples/ted/TedApp.cs index e54a6a8..a2ebf13 100644 --- a/examples/ted/TedApp.cs +++ b/examples/ted/TedApp.cs @@ -6,7 +6,6 @@ using Terminal.Gui.Document.Folding; using Terminal.Gui.Drawing; using Terminal.Gui.Editor; -using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; using Terminal.Gui.Resources; using Terminal.Gui.Text.Indentation; diff --git a/specs/completion/spec.md b/specs/completion/spec.md new file mode 100644 index 0000000..33338eb --- /dev/null +++ b/specs/completion/spec.md @@ -0,0 +1,89 @@ +# Completion Spec + +**Status**: Implemented +**Date**: 2026-05-17 +**Resolves**: OPEN-002, `specs/textview-parity-gap/spec.md` Gap 1 + +--- + +## Summary + +In-editor autocomplete popup for `Editor`, providing caret-anchored, filter-as-you-type completion +suggestions. Consumers implement `IEditorCompletionProvider` and assign it to `Editor.CompletionProvider`. +The popup renders via TG-native `PopoverMenu`, keys are intercepted ahead of the editor, and accepted +suggestions apply as a single undo step. + +## Public API + +```csharp +namespace Terminal.Gui.Editor.Completion; + +public sealed class CompletionItem +{ + public required string Label { get; init; } + public string? InsertText { get; init; } // defaults to Label + public string? Detail { get; init; } +} + +public interface IEditorCompletionProvider +{ + IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix); + bool ShouldTrigger (Key key); +} +``` + +On `Editor`: + +```csharp +public IEditorCompletionProvider? CompletionProvider { get; set; } +public bool IsCompletionActive { get; } +``` + +## Behavior + +### Opening the popup + +1. **Explicit trigger**: Provider's `ShouldTrigger(key)` returns `true` (e.g. `Ctrl+Space`). +2. **Filter-as-you-type**: After each character insert, the editor extracts the word-prefix before the caret + (letters/digits/underscores) and queries the provider. If items are returned, the popup opens or updates. + +### Popup interaction + +| Key | Action | +|-----|--------| +| `↑` / `↓` | Move selection | +| `Enter` / `Tab` | Accept selected item | +| `Esc` | Dismiss | +| Any printable char | Inserted into document, popup re-filters | +| `Backspace` | Deletes char, popup re-filters (dismissed when prefix becomes empty) | + +### Accepting a completion + +The word-prefix (`_completionPrefixStart` to `CaretOffset`) is replaced by `CompletionItem.TextToInsert` +inside a single `Document.RunUpdate()` scope, so one `Ctrl+Z` undoes the entire replacement. + +### Dismissing + +Pressing `Esc`, pressing `Enter` on NewLine command, typing a non-word character that empties the prefix, +or the provider returning zero items — all dismiss the popup. + +## Positioning + +The popup is anchored at the caret's screen position via `ViewportToScreen`, placed one row below the caret. +Uses `PopoverMenu.MakeVisible(screenPosition)`. + +## ted demo + +`WordCompletionProvider` scans the document for unique word tokens and offers those starting with the +current prefix. Triggered by `Ctrl+Space`. Wired via `Editor.CompletionProvider = new WordCompletionProvider()`. + +## Testing + +- **Unit tests** (`EditorCompletionTests`): prefix extraction, accept/dismiss, single-undo-step, no-op cases. +- **Integration tests**: popup rendering requires `IApplication`; covered by ted integration tests. + +## Design decisions + +- **DEC-009**: Fresh LSP-flavored provider, not TG's `IAutocomplete` (see `specs/decisions.md`). +- **PopoverMenu**: TG-native vehicle (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). +- **Synchronous provider**: `GetCompletions` is synchronous; providers should pre-index for speed. diff --git a/specs/decisions.md b/specs/decisions.md index 79b601f..1e58463 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -56,6 +56,16 @@ Decisions are recorded here when an open question from the plan is resolved. Eac --- +### DEC-009: Completion item shape & provider interface + +**Decision**: Use a fresh LSP-flavored `IEditorCompletionProvider` interface and `CompletionItem` type (`Terminal.Gui.Editor.Completion` namespace), **not** Terminal.Gui's existing `IAutocomplete` / `PopupAutocomplete`. + +**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses TG-native `PopoverMenu` anchored at the caret, consistent with how `Editor` already handles its context menu. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. + +**Date**: 2026-05-17 + +--- + ## Open ### OPEN-001: Independent `Terminal.Gui.Editor` NuGet from day one @@ -66,11 +76,9 @@ Decisions are recorded here when an open question from the plan is resolved. Eac --- -### OPEN-002: Completion item shape - -**Question**: Reuse Terminal.Gui's `IAutocomplete`-style types vs. a fresh LSP-flavored `IEditorCompletionProvider`? +### OPEN-002: Completion item shape → DEC-009 -**Affected features**: Post-MLP. +Resolved — see DEC-009. --- diff --git a/specs/public-api.md b/specs/public-api.md index 00c7ed3..2373dfa 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -57,8 +57,9 @@ public class Editor : View // --- Search --- public ISearchStrategy? SearchStrategy { get; set; } // find-and-replace (needs search + rendering-pipeline ✅) - // --- Completion (post-MLP) --- - public IEditorCompletionProvider? CompletionProvider { get; set; } // post-MLP + // --- Completion --- + public IEditorCompletionProvider? CompletionProvider { get; set; } // completion ✅ + public bool IsCompletionActive { get; } // completion ✅ } ``` @@ -103,6 +104,25 @@ public interface IOverlayRenderer } ``` +## Completion Types (completion — landed) + +```csharp +namespace Terminal.Gui.Editor.Completion; + +public sealed class CompletionItem +{ + public required string Label { get; init; } + public string? InsertText { get; init; } + public string? Detail { get; init; } +} + +public interface IEditorCompletionProvider +{ + IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix); + bool ShouldTrigger (Key key); +} +``` + ## Change Log | Date | Change | Feature | @@ -114,3 +134,4 @@ public interface IOverlayRenderer | 2026-05-11 | ReadOnly property landed on Editor | read-only | | 2026-05-12 | `ISearchStrategy?` `SearchStrategy { get; set; }` landed on Editor; string-based FindNext/FindPrevious/ReplaceNext/ReplaceAll overloads retained as convenience wrappers | find-and-replace | | 2026-05-16 | Vertical multi-caret keybindings (`Ctrl+Alt+CursorUp/Down`, `Alt+Drag`) added via `Editor.DefaultKeyBindings`; no new public Editor API (R8) | vertical-multi-caret | +| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `PopoverMenu`-based popup; DEC-009 resolves OPEN-002 | completion | diff --git a/src/Terminal.Gui.Editor/Editor.Commands.cs b/src/Terminal.Gui.Editor/Editor.Commands.cs index 6bd1a50..419c0d2 100644 --- a/src/Terminal.Gui.Editor/Editor.Commands.cs +++ b/src/Terminal.Gui.Editor/Editor.Commands.cs @@ -179,7 +179,7 @@ private void CreateCommandsAndBindings () }); AddCommand (Command.DeleteCharLeft, () => { - bool? result = MultiCaretDeleteLeft (); + var result = MultiCaretDeleteLeft (); NotifyCompletionAfterInsert (); return result; diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 279e0bb..d4025a0 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -9,8 +9,8 @@ namespace Terminal.Gui.Editor; public partial class Editor { private IReadOnlyList _completionItems = []; - private int _completionPrefixStart; private PopoverMenu? _completionPopup; + private int _completionPrefixStart; private int _completionSelectedIndex; /// @@ -235,7 +235,7 @@ private void ShowCompletionPopup () Point caretScreen = GetCaretScreenPosition (); // Build menu items from the completion items. - var menuItems = new MenuItem[_completionItems.Count]; + MenuItem[] menuItems = new MenuItem[_completionItems.Count]; for (var i = 0; i < _completionItems.Count; i++) { @@ -244,10 +244,11 @@ private void ShowCompletionPopup () menuItems[i] = new MenuItem { Title = label }; } - // Dispose previous popup if any — create fresh each time so the Root is rebuilt. + // Hide and dispose previous popup if any — create fresh each time so the Root is rebuilt. if (_completionPopup is not null) { _completionPopup.Visible = false; + _completionPopup.Dispose (); } _completionPopup = new PopoverMenu (menuItems) diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index f1d854a..6af2bdb 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -1,7 +1,6 @@ // CoPilot - gpt-4.1 using Terminal.Gui.Document; -using Terminal.Gui.Editor; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; using Xunit; From 449334d2cc4b34339d640d28cad93ae292480f8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 15:47:17 +0000 Subject: [PATCH 04/32] fix: disable autocomplete by default in ted; add Auto Complete checkbox in Settings dialog Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/cfa1ac33-d4c6-44f9-a99b-c6368a9297bd Co-authored-by: tig <585482+tig@users.noreply.github.com> --- examples/ted/EditorSettings.cs | 7 ++++++- examples/ted/EditorSettingsDialog.cs | 17 +++++++++++++++-- examples/ted/TedApp.cs | 3 ++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/examples/ted/EditorSettings.cs b/examples/ted/EditorSettings.cs index 91e57e3..21ff9cb 100644 --- a/examples/ted/EditorSettings.cs +++ b/examples/ted/EditorSettings.cs @@ -29,6 +29,9 @@ internal static class EditorSettings [ConfigurationProperty (Scope = typeof (TedSettingsScope))] public static bool AutoIndent { get; set; } = true; + [ConfigurationProperty (Scope = typeof (TedSettingsScope))] + public static bool AutoComplete { get; set; } + /// /// Loads settings from the config file at . /// Called once at startup before constructing . @@ -56,6 +59,7 @@ internal static void Load (string path) IndentSize = ReadInt (text, "EditorSettings.IndentSize", IndentSize); ConvertTabsToSpaces = ReadBool (text, "EditorSettings.ConvertTabsToSpaces", ConvertTabsToSpaces); AutoIndent = ReadBool (text, "EditorSettings.AutoIndent", AutoIndent); + AutoComplete = ReadBool (text, "EditorSettings.AutoComplete", AutoComplete); } catch (Exception ex) { @@ -82,7 +86,8 @@ internal static void Save (string path) ["EditorSettings.ShowTabs"] = ToJson (ShowTabs), ["EditorSettings.IndentSize"] = IndentSize.ToString (), ["EditorSettings.ConvertTabsToSpaces"] = ToJson (ConvertTabsToSpaces), - ["EditorSettings.AutoIndent"] = ToJson (AutoIndent) + ["EditorSettings.AutoIndent"] = ToJson (AutoIndent), + ["EditorSettings.AutoComplete"] = ToJson (AutoComplete) }; List toInsert = []; diff --git a/examples/ted/EditorSettingsDialog.cs b/examples/ted/EditorSettingsDialog.cs index 5ecdfe2..d09fa63 100644 --- a/examples/ted/EditorSettingsDialog.cs +++ b/examples/ted/EditorSettingsDialog.cs @@ -7,6 +7,7 @@ namespace Ted; internal sealed class EditorSettingsDialog : Dialog { + private readonly CheckBox _autoCompleteCheck; private readonly CheckBox _autoIndentCheck; private readonly CheckBox _convertTabsCheck; private readonly NumericUpDown _indentSize; @@ -15,7 +16,7 @@ internal EditorSettingsDialog (Editor editor) { Title = "Settings"; Width = Dim.Percent (60); - Height = 16; + Height = 18; View tabSettingsTab = new () { @@ -55,11 +56,20 @@ internal EditorSettingsDialog (Editor editor) Value = editor.IndentationStrategy is not null ? CheckState.Checked : CheckState.UnChecked }; + _autoCompleteCheck = new CheckBox + { + X = 1, + Y = 7, + Title = "Auto _Complete (Ctrl+Space)", + Value = editor.CompletionProvider is not null ? CheckState.Checked : CheckState.UnChecked + }; + tabSettingsTab.Add ( new Label { X = 1, Y = 1, Text = "_Indent size:" }, _indentSize, _convertTabsCheck, - _autoIndentCheck); + _autoIndentCheck, + _autoCompleteCheck); View configTab = new () { @@ -115,5 +125,8 @@ internal void ApplyTo (Editor editor) editor.IndentationStrategy = _autoIndentCheck.Value == CheckState.Checked ? new DefaultIndentationStrategy () : null; + editor.CompletionProvider = _autoCompleteCheck.Value == CheckState.Checked + ? new WordCompletionProvider () + : null; } } diff --git a/examples/ted/TedApp.cs b/examples/ted/TedApp.cs index a2ebf13..4f941d7 100644 --- a/examples/ted/TedApp.cs +++ b/examples/ted/TedApp.cs @@ -40,7 +40,7 @@ public TedApp (bool readOnly = false) WordWrap = EditorSettings.WordWrap, ShowTabs = EditorSettings.ShowTabs, ReadOnly = readOnly, - CompletionProvider = new WordCompletionProvider (), + CompletionProvider = EditorSettings.AutoComplete ? new WordCompletionProvider () : null, ViewportSettings = ViewportSettingsFlags.HasScrollBars }; @@ -402,6 +402,7 @@ private void SaveViewSettings () EditorSettings.IndentSize = Editor.IndentationSize; EditorSettings.ConvertTabsToSpaces = Editor.ConvertTabsToSpaces; EditorSettings.AutoIndent = Editor.IndentationStrategy is not null; + EditorSettings.AutoComplete = Editor.CompletionProvider is not null; EditorSettings.Save (); } From 8f4d77dd6b3349f6350033af635f68993cdf08b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 16:16:59 +0000 Subject: [PATCH 05/32] fix: move Auto Complete checkbox from Tab Settings to Config tab in Settings dialog Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/e681778e-94f5-49ce-a7fc-8316bcd4a183 Co-authored-by: tig <585482+tig@users.noreply.github.com> --- examples/ted/EditorSettingsDialog.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/examples/ted/EditorSettingsDialog.cs b/examples/ted/EditorSettingsDialog.cs index d09fa63..c92eeba 100644 --- a/examples/ted/EditorSettingsDialog.cs +++ b/examples/ted/EditorSettingsDialog.cs @@ -56,21 +56,20 @@ internal EditorSettingsDialog (Editor editor) Value = editor.IndentationStrategy is not null ? CheckState.Checked : CheckState.UnChecked }; + tabSettingsTab.Add ( + new Label { X = 1, Y = 1, Text = "_Indent size:" }, + _indentSize, + _convertTabsCheck, + _autoIndentCheck); + _autoCompleteCheck = new CheckBox { X = 1, - Y = 7, + Y = 1, Title = "Auto _Complete (Ctrl+Space)", Value = editor.CompletionProvider is not null ? CheckState.Checked : CheckState.UnChecked }; - tabSettingsTab.Add ( - new Label { X = 1, Y = 1, Text = "_Indent size:" }, - _indentSize, - _convertTabsCheck, - _autoIndentCheck, - _autoCompleteCheck); - View configTab = new () { Title = "_Config", @@ -78,7 +77,7 @@ internal EditorSettingsDialog (Editor editor) Height = Dim.Fill () }; - configTab.Add (new Label { X = 1, Y = 1, Text = "No settings yet." }); + configTab.Add (_autoCompleteCheck); Tabs tabs = new () { From 45a1e09e8a34767b9c112270d1ff6495d78c0e60 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 16:26:24 +0000 Subject: [PATCH 06/32] =?UTF-8?q?fix:=20CLAUDE.md=20compliance=20=E2=80=94?= =?UTF-8?q?=20remove=20dead=20Esc=20handler,=20fix=20Allman=20braces=20in?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/979aa238-ec63-48d4-8db2-5a3cac276f01 Co-authored-by: tig <585482+tig@users.noreply.github.com> --- src/Terminal.Gui.Editor/Editor.Keyboard.cs | 9 --------- .../EditorCompletionTests.cs | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index b88e89c..c74873c 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -26,15 +26,6 @@ protected override bool OnKeyDownNotHandled (Key key) return true; } - // Esc dismisses an active completion (handled above); when no popup is active, let it - // fall through to multi-caret clear or default handling. - if (key == Key.Esc && IsCompletionActive) - { - DismissCompletion (); - - return true; - } - if (key.IsCtrl || key.IsAlt) { return false; diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 6af2bdb..bc38b8a 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -155,7 +155,10 @@ private sealed class StubCompletionProvider : IEditorCompletionProvider { private readonly string _word; - public StubCompletionProvider (string word) { _word = word; } + public StubCompletionProvider (string word) + { + _word = word; + } public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) { @@ -169,7 +172,10 @@ public IReadOnlyList GetCompletions (TextDocument document, int : []; } - public bool ShouldTrigger (Key key) { return key == Key.Space.WithCtrl; } + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } } /// Provider that always returns an empty list. @@ -180,6 +186,9 @@ public IReadOnlyList GetCompletions (TextDocument document, int return []; } - public bool ShouldTrigger (Key key) { return false; } + public bool ShouldTrigger (Key key) + { + return false; + } } } From dc8fcbd6b9922e69d4c349bf5a004955f2eff0c0 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 17 May 2026 13:02:38 -0400 Subject: [PATCH 07/32] code cleanup --- examples/ted/Properties/launchSettings.json | 8 ++++++++ src/Terminal.Gui.Editor/Editor.Completion.cs | 8 ++------ 2 files changed, 10 insertions(+), 6 deletions(-) create mode 100644 examples/ted/Properties/launchSettings.json diff --git a/examples/ted/Properties/launchSettings.json b/examples/ted/Properties/launchSettings.json new file mode 100644 index 0000000..da0e068 --- /dev/null +++ b/examples/ted/Properties/launchSettings.json @@ -0,0 +1,8 @@ +{ + "profiles": { + "ted": { + "commandName": "Project", + "workingDirectory": "../.." + } + } +} \ No newline at end of file diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index d4025a0..4d801b7 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -184,10 +184,7 @@ internal void ShowCompletion () /// Hides the completion popup if it is visible. internal void DismissCompletion () { - if (_completionPopup is not null) - { - _completionPopup.Visible = false; - } + _completionPopup?.Visible = false; _completionItems = []; } @@ -240,8 +237,7 @@ private void ShowCompletionPopup () for (var i = 0; i < _completionItems.Count; i++) { CompletionItem item = _completionItems[i]; - var label = i == _completionSelectedIndex ? $"> {item.Label}" : $" {item.Label}"; - menuItems[i] = new MenuItem { Title = label }; + menuItems[i] = new MenuItem { Title = item.Label }; } // Hide and dispose previous popup if any — create fresh each time so the Root is rebuilt. From 499d0d531d58da1472dfc75455766e0f06a2c903 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 17:24:16 +0000 Subject: [PATCH 08/32] refactor: replace PopoverMenu with Popover for completion popup Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/d9914184-4fc8-474d-87ec-a09c7cda74bb Co-authored-by: tig <585482+tig@users.noreply.github.com> --- CLAUDE.md | 2 +- specs/completion/spec.md | 8 +-- specs/decisions.md | 2 +- specs/public-api.md | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 64 +++++++++++++++----- 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cff4f5f..b61db24 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -233,7 +233,7 @@ Don't accidentally do these — they were considered and rejected: - Source/API compatibility with `Terminal.Gui.TextView`. `Editor` ships beside it, not as a replacement. - RTL bidi or rich text shaping beyond grapheme width. - Pixel/proportional font fidelity. -- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `PopoverMenu` for completion). +- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `Popover` + `ListView` for completion). ## Open decisions diff --git a/specs/completion/spec.md b/specs/completion/spec.md index 33338eb..4ea7c5b 100644 --- a/specs/completion/spec.md +++ b/specs/completion/spec.md @@ -10,8 +10,8 @@ In-editor autocomplete popup for `Editor`, providing caret-anchored, filter-as-you-type completion suggestions. Consumers implement `IEditorCompletionProvider` and assign it to `Editor.CompletionProvider`. -The popup renders via TG-native `PopoverMenu`, keys are intercepted ahead of the editor, and accepted -suggestions apply as a single undo step. +The popup renders via a TG-native `Popover` (a `ListView` inside a `Popover`), +keys are intercepted ahead of the editor, and accepted suggestions apply as a single undo step. ## Public API @@ -70,7 +70,7 @@ or the provider returning zero items — all dismiss the popup. ## Positioning The popup is anchored at the caret's screen position via `ViewportToScreen`, placed one row below the caret. -Uses `PopoverMenu.MakeVisible(screenPosition)`. +Uses `Popover.MakeVisible(screenPosition)`. ## ted demo @@ -85,5 +85,5 @@ current prefix. Triggered by `Ctrl+Space`. Wired via `Editor.CompletionProvider ## Design decisions - **DEC-009**: Fresh LSP-flavored provider, not TG's `IAutocomplete` (see `specs/decisions.md`). -- **PopoverMenu**: TG-native vehicle (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). +- **Popover + ListView**: TG-native `Popover` (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). - **Synchronous provider**: `GetCompletions` is synchronous; providers should pre-index for speed. diff --git a/specs/decisions.md b/specs/decisions.md index 1e58463..310a7c6 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -60,7 +60,7 @@ Decisions are recorded here when an open question from the plan is resolved. Eac **Decision**: Use a fresh LSP-flavored `IEditorCompletionProvider` interface and `CompletionItem` type (`Terminal.Gui.Editor.Completion` namespace), **not** Terminal.Gui's existing `IAutocomplete` / `PopupAutocomplete`. -**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses TG-native `PopoverMenu` anchored at the caret, consistent with how `Editor` already handles its context menu. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. +**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a TG-native `Popover` anchored at the caret, consistent with how `DropDownList` uses `Popover` for its list display. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. **Date**: 2026-05-17 diff --git a/specs/public-api.md b/specs/public-api.md index 2373dfa..9e4b503 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -134,4 +134,4 @@ public interface IEditorCompletionProvider | 2026-05-11 | ReadOnly property landed on Editor | read-only | | 2026-05-12 | `ISearchStrategy?` `SearchStrategy { get; set; }` landed on Editor; string-based FindNext/FindPrevious/ReplaceNext/ReplaceAll overloads retained as convenience wrappers | find-and-replace | | 2026-05-16 | Vertical multi-caret keybindings (`Ctrl+Alt+CursorUp/Down`, `Alt+Drag`) added via `Editor.DefaultKeyBindings`; no new public Editor API (R8) | vertical-multi-caret | -| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `PopoverMenu`-based popup; DEC-009 resolves OPEN-002 | completion | +| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `Popover`-based popup; DEC-009 resolves OPEN-002 | completion | diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 4d801b7..fa5f907 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -1,4 +1,6 @@ +using System.Collections.ObjectModel; using System.Drawing; +using Terminal.Gui.App; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; using Terminal.Gui.ViewBase; @@ -9,7 +11,8 @@ namespace Terminal.Gui.Editor; public partial class Editor { private IReadOnlyList _completionItems = []; - private PopoverMenu? _completionPopup; + private ListView? _completionListView; + private Popover? _completionPopover; private int _completionPrefixStart; private int _completionSelectedIndex; @@ -184,7 +187,10 @@ internal void ShowCompletion () /// Hides the completion popup if it is visible. internal void DismissCompletion () { - _completionPopup?.Visible = false; + if (_completionPopover is not null) + { + _completionPopover.Visible = false; + } _completionItems = []; } @@ -229,31 +235,50 @@ internal void AcceptCompletion () private void ShowCompletionPopup () { + if (_completionItems.Count == 0) + { + return; + } + Point caretScreen = GetCaretScreenPosition (); - // Build menu items from the completion items. - MenuItem[] menuItems = new MenuItem[_completionItems.Count]; + // Build the label list for the ListView. + ObservableCollection labels = new (_completionItems.Select (i => i.Label)); - for (var i = 0; i < _completionItems.Count; i++) + // Dispose previous popover if any — create fresh each time so the list is rebuilt. + if (_completionPopover is not null) { - CompletionItem item = _completionItems[i]; - menuItems[i] = new MenuItem { Title = item.Label }; + _completionPopover.Visible = false; + _completionPopover.Dispose (); } - // Hide and dispose previous popup if any — create fresh each time so the Root is rebuilt. - if (_completionPopup is not null) + // Cap visible height at 10 items to avoid oversized popups. + var visibleCount = Math.Min (_completionItems.Count, 10); + + _completionListView = new ListView { - _completionPopup.Visible = false; - _completionPopup.Dispose (); - } + Source = new ListWrapper (labels), + Width = _completionItems.Max (i => i.Label.Length) + 2, + Height = visibleCount + }; + _completionListView.SelectedItem = _completionSelectedIndex; - _completionPopup = new PopoverMenu (menuItems) + _completionPopover = new Popover (_completionListView) { - Target = new WeakReference (this) + Target = new WeakReference (this), + ResultExtractor = lv => + { + if (lv.SelectedItem is not { } index || index < 0 || index >= _completionItems.Count) + { + return null; + } + + return _completionItems[index]; + } }; // Position the popup just below the caret. - _completionPopup.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); + _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); } private void SelectCompletionItem (int index) @@ -264,6 +289,15 @@ private void SelectCompletionItem (int index) } _completionSelectedIndex = index; + + // If the popover and list are already visible, just update the selection in place. + if (_completionListView is not null && _completionPopover is { Visible: true }) + { + _completionListView.SelectedItem = index; + + return; + } + ShowCompletionPopup (); } From 47d3782f51f9d28d6d4d571346009a7486beac1e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 17:42:09 +0000 Subject: [PATCH 09/32] refactor: replace Popover with DropDownList for completion popup DropDownList internally uses Popover for its dropdown, matching TG's native widget pattern. Updated specs, CLAUDE.md, and decisions.md to reflect the change. Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/5ca893a9-cb57-417f-9ed0-eb9ef6a3bb89 Co-authored-by: tig <585482+tig@users.noreply.github.com> --- CLAUDE.md | 2 +- specs/completion/spec.md | 6 +- specs/decisions.md | 2 +- specs/public-api.md | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 78 ++++++++++---------- 5 files changed, 44 insertions(+), 46 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b61db24..4762546 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -233,7 +233,7 @@ Don't accidentally do these — they were considered and rejected: - Source/API compatibility with `Terminal.Gui.TextView`. `Editor` ships beside it, not as a replacement. - RTL bidi or rich text shaping beyond grapheme width. - Pixel/proportional font fidelity. -- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `Popover` + `ListView` for completion). +- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `DropDownList` + `Popover` for completion). ## Open decisions diff --git a/specs/completion/spec.md b/specs/completion/spec.md index 4ea7c5b..faf590d 100644 --- a/specs/completion/spec.md +++ b/specs/completion/spec.md @@ -10,7 +10,7 @@ In-editor autocomplete popup for `Editor`, providing caret-anchored, filter-as-you-type completion suggestions. Consumers implement `IEditorCompletionProvider` and assign it to `Editor.CompletionProvider`. -The popup renders via a TG-native `Popover` (a `ListView` inside a `Popover`), +The popup renders via a TG-native `DropDownList` (which internally uses `Popover` for its dropdown), keys are intercepted ahead of the editor, and accepted suggestions apply as a single undo step. ## Public API @@ -70,7 +70,7 @@ or the provider returning zero items — all dismiss the popup. ## Positioning The popup is anchored at the caret's screen position via `ViewportToScreen`, placed one row below the caret. -Uses `Popover.MakeVisible(screenPosition)`. +Uses `DropDownList` added to the editor's `SuperView`, positioned via `ScreenToViewport` conversion. ## ted demo @@ -85,5 +85,5 @@ current prefix. Triggered by `Ctrl+Space`. Wired via `Editor.CompletionProvider ## Design decisions - **DEC-009**: Fresh LSP-flavored provider, not TG's `IAutocomplete` (see `specs/decisions.md`). -- **Popover + ListView**: TG-native `Popover` (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). +- **DropDownList + Popover**: TG-native `DropDownList` (which uses `Popover` internally) positioned at the caret (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). - **Synchronous provider**: `GetCompletions` is synchronous; providers should pre-index for speed. diff --git a/specs/decisions.md b/specs/decisions.md index 310a7c6..b40d4cd 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -60,7 +60,7 @@ Decisions are recorded here when an open question from the plan is resolved. Eac **Decision**: Use a fresh LSP-flavored `IEditorCompletionProvider` interface and `CompletionItem` type (`Terminal.Gui.Editor.Completion` namespace), **not** Terminal.Gui's existing `IAutocomplete` / `PopupAutocomplete`. -**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a TG-native `Popover` anchored at the caret, consistent with how `DropDownList` uses `Popover` for its list display. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. +**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a TG-native `DropDownList` (which internally uses `Popover`) positioned at the caret. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. **Date**: 2026-05-17 diff --git a/specs/public-api.md b/specs/public-api.md index 9e4b503..0e94c96 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -134,4 +134,4 @@ public interface IEditorCompletionProvider | 2026-05-11 | ReadOnly property landed on Editor | read-only | | 2026-05-12 | `ISearchStrategy?` `SearchStrategy { get; set; }` landed on Editor; string-based FindNext/FindPrevious/ReplaceNext/ReplaceAll overloads retained as convenience wrappers | find-and-replace | | 2026-05-16 | Vertical multi-caret keybindings (`Ctrl+Alt+CursorUp/Down`, `Alt+Drag`) added via `Editor.DefaultKeyBindings`; no new public Editor API (R8) | vertical-multi-caret | -| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `Popover`-based popup; DEC-009 resolves OPEN-002 | completion | +| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `DropDownList`-based popup; DEC-009 resolves OPEN-002 | completion | diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index fa5f907..33877d8 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -1,18 +1,15 @@ using System.Collections.ObjectModel; using System.Drawing; -using Terminal.Gui.App; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; -using Terminal.Gui.ViewBase; using Terminal.Gui.Views; namespace Terminal.Gui.Editor; public partial class Editor { + private DropDownList? _completionDropDown; private IReadOnlyList _completionItems = []; - private ListView? _completionListView; - private Popover? _completionPopover; private int _completionPrefixStart; private int _completionSelectedIndex; @@ -187,9 +184,11 @@ internal void ShowCompletion () /// Hides the completion popup if it is visible. internal void DismissCompletion () { - if (_completionPopover is not null) + if (_completionDropDown is not null) { - _completionPopover.Visible = false; + SuperView?.Remove (_completionDropDown); + _completionDropDown.Dispose (); + _completionDropDown = null; } _completionItems = []; @@ -235,50 +234,54 @@ internal void AcceptCompletion () private void ShowCompletionPopup () { - if (_completionItems.Count == 0) + if (_completionItems.Count == 0 || SuperView is null) { return; } - Point caretScreen = GetCaretScreenPosition (); + // Dispose previous dropdown if any — create fresh each time so the list is rebuilt. + if (_completionDropDown is not null) + { + SuperView.Remove (_completionDropDown); + _completionDropDown.Dispose (); + _completionDropDown = null; + } - // Build the label list for the ListView. - ObservableCollection labels = new (_completionItems.Select (i => i.Label)); + // Build the label list and measure max width in a single pass. + var maxLen = 0; + ObservableCollection labels = []; - // Dispose previous popover if any — create fresh each time so the list is rebuilt. - if (_completionPopover is not null) + foreach (CompletionItem item in _completionItems) { - _completionPopover.Visible = false; - _completionPopover.Dispose (); + labels.Add (item.Label); + maxLen = Math.Max (maxLen, item.Label.Length); } - // Cap visible height at 10 items to avoid oversized popups. - var visibleCount = Math.Min (_completionItems.Count, 10); + // Compute the caret position in SuperView coordinates for placement. + Point caretScreen = GetCaretScreenPosition (); + Point inSuperView = SuperView.ScreenToViewport (caretScreen); - _completionListView = new ListView + _completionDropDown = new DropDownList { Source = new ListWrapper (labels), - Width = _completionItems.Max (i => i.Label.Length) + 2, - Height = visibleCount + ReadOnly = true, + Width = maxLen + 4, + X = inSuperView.X, + Y = inSuperView.Y + 1 }; - _completionListView.SelectedItem = _completionSelectedIndex; - _completionPopover = new Popover (_completionListView) + // Pre-select the first item (bounds-checked). + if (labels.Count > 0 && _completionSelectedIndex >= 0 && _completionSelectedIndex < labels.Count) { - Target = new WeakReference (this), - ResultExtractor = lv => - { - if (lv.SelectedItem is not { } index || index < 0 || index >= _completionItems.Count) - { - return null; - } + _completionDropDown.Text = labels[_completionSelectedIndex]; + } - return _completionItems[index]; - } - }; + SuperView.Add (_completionDropDown); + SuperView.Layout (); - // Position the popup just below the caret. - _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); + // Open the dropdown list immediately via the Toggle command. + _completionDropDown.SetFocus (); + _completionDropDown.InvokeCommand (Command.Toggle); } private void SelectCompletionItem (int index) @@ -290,15 +293,10 @@ private void SelectCompletionItem (int index) _completionSelectedIndex = index; - // If the popover and list are already visible, just update the selection in place. - if (_completionListView is not null && _completionPopover is { Visible: true }) + if (_completionDropDown is not null) { - _completionListView.SelectedItem = index; - - return; + _completionDropDown.Text = _completionItems[index].Label; } - - ShowCompletionPopup (); } /// From d34ba7c9d3bc31fd38ebc31b1805538cc76df2b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 19:12:06 +0000 Subject: [PATCH 10/32] refactor: replace DropDownList with Popover for completion popup Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/38a7a3c3-eec0-44ed-8ef8-38d47127b31c Co-authored-by: tig <585482+tig@users.noreply.github.com> --- CLAUDE.md | 2 +- specs/completion/spec.md | 6 +- specs/decisions.md | 2 +- specs/public-api.md | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 86 +++++++++++--------- 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4762546..041e452 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -233,7 +233,7 @@ Don't accidentally do these — they were considered and rejected: - Source/API compatibility with `Terminal.Gui.TextView`. `Editor` ships beside it, not as a replacement. - RTL bidi or rich text shaping beyond grapheme width. - Pixel/proportional font fidelity. -- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `DropDownList` + `Popover` for completion). +- Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `Popover` for completion). ## Open decisions diff --git a/specs/completion/spec.md b/specs/completion/spec.md index faf590d..955631b 100644 --- a/specs/completion/spec.md +++ b/specs/completion/spec.md @@ -10,7 +10,7 @@ In-editor autocomplete popup for `Editor`, providing caret-anchored, filter-as-you-type completion suggestions. Consumers implement `IEditorCompletionProvider` and assign it to `Editor.CompletionProvider`. -The popup renders via a TG-native `DropDownList` (which internally uses `Popover` for its dropdown), +The popup renders via a `Popover`, keys are intercepted ahead of the editor, and accepted suggestions apply as a single undo step. ## Public API @@ -70,7 +70,7 @@ or the provider returning zero items — all dismiss the popup. ## Positioning The popup is anchored at the caret's screen position via `ViewportToScreen`, placed one row below the caret. -Uses `DropDownList` added to the editor's `SuperView`, positioned via `ScreenToViewport` conversion. +Uses `Popover` positioned at the caret via `MakeVisible`. ## ted demo @@ -85,5 +85,5 @@ current prefix. Triggered by `Ctrl+Space`. Wired via `Editor.CompletionProvider ## Design decisions - **DEC-009**: Fresh LSP-flavored provider, not TG's `IAutocomplete` (see `specs/decisions.md`). -- **DropDownList + Popover**: TG-native `DropDownList` (which uses `Popover` internally) positioned at the caret (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). +- **Popover**: `Popover` positioned at the caret (explicit non-goal: AvaloniaEdit `CodeCompletion/` lift). - **Synchronous provider**: `GetCompletions` is synchronous; providers should pre-index for speed. diff --git a/specs/decisions.md b/specs/decisions.md index b40d4cd..890f871 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -60,7 +60,7 @@ Decisions are recorded here when an open question from the plan is resolved. Eac **Decision**: Use a fresh LSP-flavored `IEditorCompletionProvider` interface and `CompletionItem` type (`Terminal.Gui.Editor.Completion` namespace), **not** Terminal.Gui's existing `IAutocomplete` / `PopupAutocomplete`. -**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a TG-native `DropDownList` (which internally uses `Popover`) positioned at the caret. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. +**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a `Popover` positioned at the caret. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. **Date**: 2026-05-17 diff --git a/specs/public-api.md b/specs/public-api.md index 8a7c280..71d1e5d 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -139,5 +139,5 @@ public interface IEditorCompletionProvider | 2026-05-11 | ReadOnly property landed on Editor | read-only | | 2026-05-12 | `ISearchStrategy?` `SearchStrategy { get; set; }` landed on Editor; string-based FindNext/FindPrevious/ReplaceNext/ReplaceAll overloads retained as convenience wrappers | find-and-replace | | 2026-05-16 | Vertical multi-caret keybindings (`Ctrl+Alt+CursorUp/Down`, `Alt+Drag`) added via `Editor.DefaultKeyBindings`; no new public Editor API (R8) | vertical-multi-caret | -| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `DropDownList`-based popup; DEC-009 resolves OPEN-002 | completion | +| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `Popover`-based popup; DEC-009 resolves OPEN-002 | completion | | 2026-05-17 | `Editor` implements `IDesignable`; `EnableForDesign()` seeds C# sample code with syntax highlighting and line numbers | design-time | diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 33877d8..87a0b99 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -1,15 +1,18 @@ using System.Collections.ObjectModel; using System.Drawing; +using Terminal.Gui.App; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; +using Terminal.Gui.ViewBase; using Terminal.Gui.Views; namespace Terminal.Gui.Editor; public partial class Editor { - private DropDownList? _completionDropDown; private IReadOnlyList _completionItems = []; + private ListView? _completionListView; + private Popover? _completionPopover; private int _completionPrefixStart; private int _completionSelectedIndex; @@ -184,11 +187,12 @@ internal void ShowCompletion () /// Hides the completion popup if it is visible. internal void DismissCompletion () { - if (_completionDropDown is not null) + if (_completionPopover is not null) { - SuperView?.Remove (_completionDropDown); - _completionDropDown.Dispose (); - _completionDropDown = null; + _completionPopover.Visible = false; + _completionPopover.Dispose (); + _completionPopover = null; + _completionListView = null; } _completionItems = []; @@ -234,54 +238,53 @@ internal void AcceptCompletion () private void ShowCompletionPopup () { - if (_completionItems.Count == 0 || SuperView is null) + if (_completionItems.Count == 0) { return; } - // Dispose previous dropdown if any — create fresh each time so the list is rebuilt. - if (_completionDropDown is not null) + // Dispose previous popover if any — create fresh each time so the list is rebuilt. + if (_completionPopover is not null) { - SuperView.Remove (_completionDropDown); - _completionDropDown.Dispose (); - _completionDropDown = null; + _completionPopover.Visible = false; + _completionPopover.Dispose (); + _completionPopover = null; + _completionListView = null; } - // Build the label list and measure max width in a single pass. - var maxLen = 0; - ObservableCollection labels = []; - - foreach (CompletionItem item in _completionItems) - { - labels.Add (item.Label); - maxLen = Math.Max (maxLen, item.Label.Length); - } + // Build the label list for the ListView. + ObservableCollection labels = new (_completionItems.Select (i => i.Label)); - // Compute the caret position in SuperView coordinates for placement. - Point caretScreen = GetCaretScreenPosition (); - Point inSuperView = SuperView.ScreenToViewport (caretScreen); + // Cap visible height at 10 items to avoid oversized popups. + var visibleCount = Math.Min (_completionItems.Count, 10); - _completionDropDown = new DropDownList + _completionListView = new ListView { Source = new ListWrapper (labels), - ReadOnly = true, - Width = maxLen + 4, - X = inSuperView.X, - Y = inSuperView.Y + 1 + Width = _completionItems.Max (i => i.Label.Length) + 2, + Height = visibleCount }; + _completionListView.SelectedItem = _completionSelectedIndex; + + IReadOnlyList capturedItems = _completionItems; - // Pre-select the first item (bounds-checked). - if (labels.Count > 0 && _completionSelectedIndex >= 0 && _completionSelectedIndex < labels.Count) + _completionPopover = new Popover (_completionListView) { - _completionDropDown.Text = labels[_completionSelectedIndex]; - } + Target = new WeakReference (this), + ResultExtractor = lv => + { + if (lv.SelectedItem is not { } idx || idx < 0 || idx >= capturedItems.Count) + { + return null; + } - SuperView.Add (_completionDropDown); - SuperView.Layout (); + return capturedItems[idx]; + } + }; - // Open the dropdown list immediately via the Toggle command. - _completionDropDown.SetFocus (); - _completionDropDown.InvokeCommand (Command.Toggle); + // Position the popup just below the caret. + Point caretScreen = GetCaretScreenPosition (); + _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); } private void SelectCompletionItem (int index) @@ -293,10 +296,15 @@ private void SelectCompletionItem (int index) _completionSelectedIndex = index; - if (_completionDropDown is not null) + // If the popover and list are already visible, just update the selection in place. + if (_completionListView is not null && _completionPopover is { Visible: true }) { - _completionDropDown.Text = _completionItems[index].Label; + _completionListView.SelectedItem = index; + + return; } + + ShowCompletionPopup (); } /// From 828c17fecc8bf85c87364f7fe50277fb60083614 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 19:37:48 +0000 Subject: [PATCH 11/32] test: add failing tests for completion keyboard capture, Enter/Tab routing, and provider-null dismissal Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/0bcde36b-f73e-4ece-8fb4-b38cec93cb0b Co-authored-by: tig <585482+tig@users.noreply.github.com> --- .../EditorCompletionIntegrationTests.cs | 153 ++++++++++++++++++ .../EditorCompletionTests.cs | 119 ++++++++++++++ 2 files changed, 272 insertions(+) create mode 100644 tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs new file mode 100644 index 0000000..1ff7df7 --- /dev/null +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs @@ -0,0 +1,153 @@ +// CoPilot - gpt-4.1 + +using Terminal.Gui.Document; +using Terminal.Gui.Editor.Completion; +using Terminal.Gui.Editor.IntegrationTests.Testing; +using Terminal.Gui.Input; +using Terminal.Gui.Testing; +using Xunit; + +namespace Terminal.Gui.Editor.IntegrationTests; + +/// +/// Integration tests for in-editor completion. Uses to exercise +/// the full key-dispatch pipeline, including interaction. +/// +public class EditorCompletionIntegrationTests +{ + private static readonly InputInjectionOptions Direct = new () { Mode = InputInjectionMode.Direct }; + + /// + /// Typing a character while the completion popup is open must insert the character + /// into the document (not be swallowed by the Popover) and update the completion list. + /// + [Fact] + public async Task Typing_Char_While_Completion_Active_Inserts_Into_Document () + { + await using AppFixture fx = new (() => new ("using unsafe uint")); + Editor editor = fx.Top.Editor; + editor.SetFocus (); + + // Set up a completion provider that matches words from the document. + editor.CompletionProvider = new TestWordCompletionProvider (); + + // Type "u" to open completion. + fx.Injector.InjectKey (Key.U, Direct); + Assert.Equal ("u", editor.Document!.GetText (0, 1)); + Assert.True (editor.IsCompletionActive, "Completion should be active after typing 'u'"); + + // Type "s" — this must go to the Editor, not be captured by the Popover. + fx.Injector.InjectKey (Key.S, Direct); + + // The document should now contain "us" at the start. + Assert.StartsWith ("us", editor.Document!.Text); + Assert.Equal (2, editor.CaretOffset); + + // Completion should still be active with filtered results. + Assert.True (editor.IsCompletionActive, "Completion should remain active with 'us' prefix"); + } + + /// + /// Typing a character that eliminates all matches must dismiss the completion popup. + /// + [Fact] + public async Task Typing_NonMatching_Char_Dismisses_Completion () + { + await using AppFixture fx = new (() => new ("using unsafe uint")); + Editor editor = fx.Top.Editor; + editor.SetFocus (); + + editor.CompletionProvider = new TestWordCompletionProvider (); + + // Type "u" to open completion. + fx.Injector.InjectKey (Key.U, Direct); + Assert.True (editor.IsCompletionActive); + + // Type "z" — no words start with "uz", so completion should dismiss. + fx.Injector.InjectKey (Key.Z, Direct); + + Assert.StartsWith ("uz", editor.Document!.Text); + Assert.False (editor.IsCompletionActive, "Completion should dismiss when no items match"); + } + + /// + /// Pressing Enter while completion is active should accept the completion (replace prefix + /// with the selected item's text), not insert a newline. + /// + [Fact] + public async Task Enter_While_Completion_Active_Accepts_Item () + { + await using AppFixture fx = new (() => new ("using unsafe uint")); + Editor editor = fx.Top.Editor; + editor.SetFocus (); + + editor.CompletionProvider = new TestWordCompletionProvider (); + + // Type "us" to open completion with "using" and "unsafe" as matches. + fx.Injector.InjectKey (Key.U, Direct); + fx.Injector.InjectKey (Key.S, Direct); + + Assert.True (editor.IsCompletionActive, "Completion should be active after 'us'"); + + // Press Enter — should accept the first completion item, not insert a newline. + fx.Injector.InjectKey (Key.Enter, Direct); + + // The text should NOT start with "us\n" (newline), it should have the accepted completion. + Assert.DoesNotContain ("\n", editor.Document!.Text.Substring (0, Math.Min (10, editor.Document!.Text.Length))); + Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); + } + + /// + /// Minimal word-completion provider for integration tests. Returns all word tokens + /// from the document that start with the prefix. + /// + private sealed class TestWordCompletionProvider : IEditorCompletionProvider + { + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + if (string.IsNullOrEmpty (prefix)) + { + return []; + } + + var text = document.Text; + HashSet seen = new (StringComparer.OrdinalIgnoreCase); + List results = []; + + var i = 0; + + while (i < text.Length) + { + if (!char.IsLetterOrDigit (text[i]) && text[i] != '_') + { + i++; + + continue; + } + + var start = i; + + while (i < text.Length && (char.IsLetterOrDigit (text[i]) || text[i] == '_')) + { + i++; + } + + var word = text.Substring (start, i - start); + + if (word.Length > prefix.Length + && word.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) + && seen.Add (word)) + { + results.Add (new CompletionItem { Label = word }); + } + } + + return results; + } + + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } + } +} diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index bc38b8a..04b0c0c 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -150,6 +150,96 @@ public void HandleCompletionKey_Returns_False_Without_Provider () Assert.False (editor.HandleCompletionKey (Key.Esc)); } + [Fact] + public void HandleCompletionKey_Returns_False_For_Regular_Chars_While_Active () + { + // When the popup is active, regular character keys must NOT be consumed + // so the Editor can insert the character and re-filter the list. + Editor editor = new () + { + Document = new TextDocument ("us"), + CompletionProvider = new MultiWordCompletionProvider ("using", "unsafe", "uint") + }; + editor.CaretOffset = 2; // after "us" + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // A regular character key should return false — not consumed by the popup. + Assert.False (editor.HandleCompletionKey (new Key ('i'))); + } + + [Fact] + public void Setting_CompletionProvider_To_Null_Dismisses_Active_Session () + { + Editor editor = new () + { + Document = new TextDocument ("hel"), + CompletionProvider = new StubCompletionProvider ("hello") + }; + editor.CaretOffset = 3; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Setting provider to null should dismiss the active session. + editor.CompletionProvider = null; + Assert.False (editor.IsCompletionActive); + } + + [Fact] + public void Typing_Characters_While_Completion_Active_Filters_List () + { + // Simulates: document has "u", popup shows [using, unsafe, uint]. + // User types "s" → document becomes "us", popup re-filters to [using, unsafe]. + Editor editor = new () + { + Document = new TextDocument ("u"), + CompletionProvider = new MultiWordCompletionProvider ("using", "unsafe", "uint") + }; + editor.CaretOffset = 1; + + // Open completion. + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Simulate the Editor inserting "s" (as OnKeyDownNotHandled would). + editor.Document!.Insert (editor.CaretOffset, "s"); + editor.CaretOffset = 2; + + // Re-filter via NotifyCompletionAfterInsert. + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // "us" prefix should match "using" and "unsafe" but not "uint". + // Verify by accepting — the accepted text should be one of the "us" matches. + editor.AcceptCompletion (); + Assert.Contains (editor.Document!.Text, new [] { "using", "unsafe" }); + } + + [Fact] + public void Typing_NonMatching_Char_While_Completion_Active_Dismisses () + { + // Simulates: document has "x", popup shows items for "x". + // User types "z" → document becomes "xz", no matches → popup dismissed. + Editor editor = new () + { + Document = new TextDocument ("us"), + CompletionProvider = new MultiWordCompletionProvider ("using", "unsafe") + }; + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Simulate inserting a character that breaks all matches. + editor.Document!.Insert (editor.CaretOffset, "z"); + editor.CaretOffset = 3; + + editor.NotifyCompletionAfterInsert (); + Assert.False (editor.IsCompletionActive); + } + /// Stub provider that always returns a single hard-coded item. private sealed class StubCompletionProvider : IEditorCompletionProvider { @@ -191,4 +281,33 @@ public bool ShouldTrigger (Key key) return false; } } + + /// Provider that returns all words starting with the given prefix. + private sealed class MultiWordCompletionProvider : IEditorCompletionProvider + { + private readonly string[] _words; + + public MultiWordCompletionProvider (params string[] words) + { + _words = words; + } + + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + if (string.IsNullOrEmpty (prefix)) + { + return []; + } + + return _words + .Where (w => w.StartsWith (prefix, StringComparison.OrdinalIgnoreCase)) + .Select (w => new CompletionItem { Label = w }) + .ToList (); + } + + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } + } } From d12c7134ee41fe148db931ae0ea49a37afeb9a20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 19:41:40 +0000 Subject: [PATCH 12/32] fix: Popover keyboard capture, Enter/Tab routing, and provider-null dismissal - Set Enabled=false on completion Popover so it doesn't capture keyboard input - Move HandleCompletionKey to OnKeyDown (runs before command bindings) so Enter/Tab/arrows are intercepted for completion before newline/tab bindings fire - CompletionProvider setter dismisses active session when set to null Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/0bcde36b-f73e-4ece-8fb4-b38cec93cb0b Co-authored-by: tig <585482+tig@users.noreply.github.com> --- src/Terminal.Gui.Editor/Editor.Completion.cs | 23 +++++++++++++- src/Terminal.Gui.Editor/Editor.Keyboard.cs | 20 +++++++++---- .../EditorCompletionIntegrationTests.cs | 30 ++++++++++++------- .../EditorCompletionTests.cs | 2 +- 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 87a0b99..87a9626 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -20,7 +20,24 @@ public partial class Editor /// Gets or sets the completion provider that supplies suggestions for the in-editor /// autocomplete popup. Set to to disable completion. /// - public IEditorCompletionProvider? CompletionProvider { get; set; } + public IEditorCompletionProvider? CompletionProvider + { + get; + set + { + if (field == value) + { + return; + } + + field = value; + + if (value is null) + { + DismissCompletion (); + } + } + } /// Whether the completion session is currently active (items are available). public bool IsCompletionActive => _completionItems.Count > 0; @@ -285,6 +302,10 @@ private void ShowCompletionPopup () // Position the popup just below the caret. Point caretScreen = GetCaretScreenPosition (); _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); + + // Disable keyboard dispatch so the Popover doesn't capture text input. + // All navigation (Up/Down/Enter/Tab/Esc) is handled by HandleCompletionKey. + _completionPopover.Enabled = false; } private void SelectCompletionItem (int index) diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index dcc82ce..26b3658 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -6,19 +6,29 @@ namespace Terminal.Gui.Editor; public partial class Editor { /// - /// Catches keystrokes that didn't match any registered binding (set up in - /// ) and inserts the typed rune into the document. Skips - /// modified keys and control characters — those are either bound elsewhere or not editor input. + /// Runs before command bindings. When completion is active, intercepts navigation and + /// accept/dismiss keys (Enter, Tab, arrows, Esc) so they don't trigger the normal + /// editor command bindings. Also checks provider-specific trigger keys (e.g. Ctrl+Space). /// /// - protected override bool OnKeyDownNotHandled (Key key) + protected override bool OnKeyDown (Key key) { - // Completion popup gets first priority for navigation / accept / dismiss / trigger keys. if (HandleCompletionKey (key)) { return true; } + return base.OnKeyDown (key); + } + + /// + /// Catches keystrokes that didn't match any registered binding (set up in + /// ) and inserts the typed rune into the document. Skips + /// modified keys and control characters — those are either bound elsewhere or not editor input. + /// + /// + protected override bool OnKeyDownNotHandled (Key key) + { if (key == Key.Esc && HasMultipleCarets) { ClearAdditionalCarets (); diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs index 1ff7df7..1cca348 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs @@ -24,24 +24,25 @@ public class EditorCompletionIntegrationTests [Fact] public async Task Typing_Char_While_Completion_Active_Inserts_Into_Document () { - await using AppFixture fx = new (() => new ("using unsafe uint")); + await using AppFixture fx = new (() => new ("using unsafe uint ")); Editor editor = fx.Top.Editor; editor.SetFocus (); + // Place caret at the end (after trailing space). + editor.CaretOffset = editor.Document!.TextLength; + // Set up a completion provider that matches words from the document. editor.CompletionProvider = new TestWordCompletionProvider (); // Type "u" to open completion. fx.Injector.InjectKey (Key.U, Direct); - Assert.Equal ("u", editor.Document!.GetText (0, 1)); Assert.True (editor.IsCompletionActive, "Completion should be active after typing 'u'"); // Type "s" — this must go to the Editor, not be captured by the Popover. fx.Injector.InjectKey (Key.S, Direct); - // The document should now contain "us" at the start. - Assert.StartsWith ("us", editor.Document!.Text); - Assert.Equal (2, editor.CaretOffset); + // The document should now end with "us". + Assert.EndsWith ("us", editor.Document!.Text); // Completion should still be active with filtered results. Assert.True (editor.IsCompletionActive, "Completion should remain active with 'us' prefix"); @@ -53,20 +54,23 @@ public async Task Typing_Char_While_Completion_Active_Inserts_Into_Document () [Fact] public async Task Typing_NonMatching_Char_Dismisses_Completion () { - await using AppFixture fx = new (() => new ("using unsafe uint")); + await using AppFixture fx = new (() => new ("using unsafe uint ")); Editor editor = fx.Top.Editor; editor.SetFocus (); + // Place caret at the end (after trailing space). + editor.CaretOffset = editor.Document!.TextLength; + editor.CompletionProvider = new TestWordCompletionProvider (); // Type "u" to open completion. fx.Injector.InjectKey (Key.U, Direct); Assert.True (editor.IsCompletionActive); - // Type "z" — no words start with "uz", so completion should dismiss. + // Type "z" — no words in the document start with "uz", so completion should dismiss. fx.Injector.InjectKey (Key.Z, Direct); - Assert.StartsWith ("uz", editor.Document!.Text); + Assert.EndsWith ("uz", editor.Document!.Text); Assert.False (editor.IsCompletionActive, "Completion should dismiss when no items match"); } @@ -77,10 +81,13 @@ public async Task Typing_NonMatching_Char_Dismisses_Completion () [Fact] public async Task Enter_While_Completion_Active_Accepts_Item () { - await using AppFixture fx = new (() => new ("using unsafe uint")); + await using AppFixture fx = new (() => new ("using unsafe uint ")); Editor editor = fx.Top.Editor; editor.SetFocus (); + // Place caret at the end (after trailing space). + editor.CaretOffset = editor.Document!.TextLength; + editor.CompletionProvider = new TestWordCompletionProvider (); // Type "us" to open completion with "using" and "unsafe" as matches. @@ -92,8 +99,9 @@ public async Task Enter_While_Completion_Active_Accepts_Item () // Press Enter — should accept the first completion item, not insert a newline. fx.Injector.InjectKey (Key.Enter, Direct); - // The text should NOT start with "us\n" (newline), it should have the accepted completion. - Assert.DoesNotContain ("\n", editor.Document!.Text.Substring (0, Math.Min (10, editor.Document!.Text.Length))); + // The text should NOT contain a newline near the end; it should have the accepted completion. + var lastChunk = editor.Document!.Text.Substring (editor.Document!.Text.Length - 15); + Assert.DoesNotContain ("\n", lastChunk); Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); } diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 04b0c0c..9bb3cd6 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -214,7 +214,7 @@ public void Typing_Characters_While_Completion_Active_Filters_List () // "us" prefix should match "using" and "unsafe" but not "uint". // Verify by accepting — the accepted text should be one of the "us" matches. editor.AcceptCompletion (); - Assert.Contains (editor.Document!.Text, new [] { "using", "unsafe" }); + Assert.Contains (editor.Document!.Text, new[] { "using", "unsafe" }); } [Fact] From e1e93403925d2f5f99ba337159d8599b81a72e6b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 19:44:11 +0000 Subject: [PATCH 13/32] fix: guard against short text in Enter-accept integration test Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/0bcde36b-f73e-4ece-8fb4-b38cec93cb0b Co-authored-by: tig <585482+tig@users.noreply.github.com> --- .../EditorCompletionIntegrationTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs index 1cca348..9bd564b 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs @@ -100,7 +100,8 @@ public async Task Enter_While_Completion_Active_Accepts_Item () fx.Injector.InjectKey (Key.Enter, Direct); // The text should NOT contain a newline near the end; it should have the accepted completion. - var lastChunk = editor.Document!.Text.Substring (editor.Document!.Text.Length - 15); + var text = editor.Document!.Text; + var lastChunk = text[^Math.Min (15, text.Length)..]; Assert.DoesNotContain ("\n", lastChunk); Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); } From ebdb1049638bedfb3a62b02a75eca32d6e77e299 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 21:12:39 +0000 Subject: [PATCH 14/32] =?UTF-8?q?fix:=20arrow=20key=20navigation=20in=20co?= =?UTF-8?q?mpletion=20popup=20=E2=80=94=20update=20selection=20index=20and?= =?UTF-8?q?=20ListView?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed arrow up/down keys updating the completion selected index correctly - Added UpdateCompletionListSelection helper for visual ListView updates - Added CompletionSelectedIndex internal property for test verification - Fixed unit tests: "unsafe" doesn't start with "us" (starts with "un") - Added ArrowDown_Wraps_To_First_Item test - Added ArrowDown_Then_Enter_Accepts_Second_Item integration test Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/ca58f879-5dc4-433d-8e77-2c1161ff8745 Co-authored-by: tig <585482+tig@users.noreply.github.com> --- src/Terminal.Gui.Editor/Editor.Completion.cs | 43 ++++++------ .../EditorCompletionIntegrationTests.cs | 32 +++++++++ .../EditorCompletionTests.cs | 70 +++++++++++++++++++ 3 files changed, 123 insertions(+), 22 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 87a9626..2fa51df 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -42,6 +42,9 @@ public IEditorCompletionProvider? CompletionProvider /// Whether the completion session is currently active (items are available). public bool IsCompletionActive => _completionItems.Count > 0; + /// Gets the zero-based index of the currently selected completion item. + internal int CompletionSelectedIndex => _completionSelectedIndex; + /// /// Extracts the word-prefix immediately before the caret (letters, digits, underscores) /// for use as the completion filter string. Returns empty when the caret follows @@ -115,14 +118,18 @@ internal bool HandleCompletionKey (Key key) if (key == Key.CursorUp) { - SelectCompletionItem ((_completionSelectedIndex - 1 + _completionItems.Count) % _completionItems.Count); + var newIdx = (_completionSelectedIndex - 1 + _completionItems.Count) % _completionItems.Count; + _completionSelectedIndex = newIdx; + UpdateCompletionListSelection (newIdx); return true; } if (key == Key.CursorDown) { - SelectCompletionItem ((_completionSelectedIndex + 1) % _completionItems.Count); + var newIdx = (_completionSelectedIndex + 1) % _completionItems.Count; + _completionSelectedIndex = newIdx; + UpdateCompletionListSelection (newIdx); return true; } @@ -253,6 +260,18 @@ internal void AcceptCompletion () } } + /// Updates the visible ListView selection when the Popover is showing. + private void UpdateCompletionListSelection (int index) + { + if (_completionListView is null) + { + return; + } + + _completionListView.SelectedItem = index; + _completionListView.SetNeedsDraw (); + } + private void ShowCompletionPopup () { if (_completionItems.Count == 0) @@ -308,26 +327,6 @@ private void ShowCompletionPopup () _completionPopover.Enabled = false; } - private void SelectCompletionItem (int index) - { - if (_completionItems.Count == 0) - { - return; - } - - _completionSelectedIndex = index; - - // If the popover and list are already visible, just update the selection in place. - if (_completionListView is not null && _completionPopover is { Visible: true }) - { - _completionListView.SelectedItem = index; - - return; - } - - ShowCompletionPopup (); - } - /// /// Computes the screen position of the caret (for popup anchoring). /// diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs index 9bd564b..c19fae0 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs @@ -106,6 +106,38 @@ public async Task Enter_While_Completion_Active_Accepts_Item () Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); } + /// + /// Arrow keys while completion is active should navigate the list, not move the caret. + /// Down arrow then Enter should accept the second item. + /// + [Fact] + public async Task ArrowDown_Then_Enter_Accepts_Second_Item () + { + // "hello help world" — typing "he" at the end matches "hello" and "help". + await using AppFixture fx = new (() => new ("hello help world ")); + Editor editor = fx.Top.Editor; + editor.SetFocus (); + + editor.CaretOffset = editor.Document!.TextLength; + editor.CompletionProvider = new TestWordCompletionProvider (); + + // Type "he" to open completion. + fx.Injector.InjectKey (Key.H, Direct); + fx.Injector.InjectKey (Key.E, Direct); + + Assert.True (editor.IsCompletionActive, "Completion should be active after 'he'"); + + // Down arrow to select the second item. + fx.Injector.InjectKey (Key.CursorDown, Direct); + + // Enter to accept. + fx.Injector.InjectKey (Key.Enter, Direct); + + // The accepted text should be the second match (alphabetically: "help" comes after "hello"). + Assert.EndsWith ("help", editor.Document!.Text.TrimEnd ()); + Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); + } + /// /// Minimal word-completion provider for integration tests. Returns all word tokens /// from the document that start with the prefix. diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 9bb3cd6..a80b0de 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -169,6 +169,76 @@ public void HandleCompletionKey_Returns_False_For_Regular_Chars_While_Active () Assert.False (editor.HandleCompletionKey (new Key ('i'))); } + [Fact] + public void ArrowKeys_Navigate_Completion_Selection () + { + // "he" matches "hello" and "help" — two items. + Editor editor = new () + { + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help", "world") + }; + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + Assert.Equal (0, editor.CompletionSelectedIndex); + + // Down arrow should move to index 1. + Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + Assert.Equal (1, editor.CompletionSelectedIndex); + + // Accept the completion — should insert the second item ("help"). + editor.AcceptCompletion (); + Assert.Equal ("help", editor.Document!.Text); + } + + [Fact] + public void ArrowUp_Wraps_To_Last_Item () + { + // "he" matches "hello" and "help" — two items. + Editor editor = new () + { + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help") + }; + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Up from index 0 should wrap to last item (index 1 = "help"). + Assert.True (editor.HandleCompletionKey (Key.CursorUp)); + Assert.Equal (1, editor.CompletionSelectedIndex); + + editor.AcceptCompletion (); + Assert.Equal ("help", editor.Document!.Text); + } + + [Fact] + public void ArrowDown_Wraps_To_First_Item () + { + // "he" matches "hello" and "help" — two items. + Editor editor = new () + { + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help") + }; + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Down to index 1, then down again wraps to 0. + Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + Assert.Equal (1, editor.CompletionSelectedIndex); + Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + Assert.Equal (0, editor.CompletionSelectedIndex); + + editor.AcceptCompletion (); + Assert.Equal ("hello", editor.Document!.Text); + } + [Fact] public void Setting_CompletionProvider_To_Null_Dismisses_Active_Session () { From 845628880c2663a3c7c55d500c3bbf29100f9768 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 21:14:34 +0000 Subject: [PATCH 15/32] docs: fix XML doc and comment per code review feedback Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/ca58f879-5dc4-433d-8e77-2c1161ff8745 Co-authored-by: tig <585482+tig@users.noreply.github.com> --- src/Terminal.Gui.Editor/Editor.Completion.cs | 2 +- .../EditorCompletionIntegrationTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 2fa51df..b1eef28 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -260,7 +260,7 @@ internal void AcceptCompletion () } } - /// Updates the visible ListView selection when the Popover is showing. + /// Updates the visible ListView selection if the list view exists. private void UpdateCompletionListSelection (int index) { if (_completionListView is null) diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs index c19fae0..796652b 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorCompletionIntegrationTests.cs @@ -133,7 +133,7 @@ public async Task ArrowDown_Then_Enter_Accepts_Second_Item () // Enter to accept. fx.Injector.InjectKey (Key.Enter, Direct); - // The accepted text should be the second match (alphabetically: "help" comes after "hello"). + // The accepted text should be the second match as returned by the provider. Assert.EndsWith ("help", editor.Document!.Text.TrimEnd ()); Assert.False (editor.IsCompletionActive, "Completion should be dismissed after accept"); } From 64356e4a7ac9c6151bd7e40f6da58aec8f12aeae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 17 May 2026 22:00:46 +0000 Subject: [PATCH 16/32] =?UTF-8?q?fix:=20rewrite=20completion=20popup=20?= =?UTF-8?q?=E2=80=94=20Enabled=3Dfalse=20for=20keyboard,=20direct=20char?= =?UTF-8?q?=20insert=20in=20HandleCompletionKey,=20mouse=20click=20handlin?= =?UTF-8?q?g=20via=20Editor.OnMouseEvent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Popover intercepts character keys internally (before KeyDown event fires), preventing them from reaching any event handler. Navigation keys (arrows) DO reach the KeyDown event. The solution: 1. Keep Enabled=false on the Popover (visual-only overlay, Editor retains focus) 2. HandleCompletionKey now handles character keys directly (insert + refresh) 3. HandleCompletionMouse detects clicks in the Popover's screen area 4. Arrow keys update ListView selection + SetNeedsDraw on both ListView and Popover 5. Backspace handled in HandleCompletionKey (remove char + refresh) This follows the PopupAutocomplete pattern from TG: the host control owns all input dispatch, the popup just renders. Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/be8ce6c1-d6d7-403d-933a-2e44aabaa22d Co-authored-by: tig <585482+tig@users.noreply.github.com> --- src/Terminal.Gui.Editor/Editor.Completion.cs | 109 +++++++++++++++++- src/Terminal.Gui.Editor/Editor.Mouse.cs | 7 ++ .../EditorCompletionTests.cs | 14 ++- 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index b1eef28..82d5e8e 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -1,5 +1,6 @@ using System.Collections.ObjectModel; using System.Drawing; +using System.Text; using Terminal.Gui.App; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; @@ -90,8 +91,17 @@ internal string GetCompletionPrefix (out int prefixStart) /// /// Called before normal key dispatch. Returns when the completion - /// popup consumed the key (navigation / accept / dismiss / trigger keys). + /// popup consumed the key (navigation / accept / dismiss / character insert). This runs + /// in , which fires before command bindings. /// + /// + /// The Popover is set to Enabled = false after + /// so that it functions as a visual-only overlay — the Editor retains keyboard focus and + /// all keyboard events flow through this method. This mirrors how + /// PopupAutocomplete.ProcessKey in Terminal.Gui works: the host control intercepts + /// keys and routes navigation / accept / dismiss / character insertion to the autocomplete + /// system. Mouse interaction is handled by . + /// internal bool HandleCompletionKey (Key key) { if (CompletionProvider is null) @@ -133,6 +143,48 @@ internal bool HandleCompletionKey (Key key) return true; } + + // Character keys: insert directly into the document while the popup is open, + // then refresh the completion list. We handle this here (rather than letting it + // fall through to OnKeyDownNotHandled) because the Popover is Enabled = false + // and this is the only place that processes keyboard input during a completion + // session. + if (!key.IsCtrl && !key.IsAlt && key.AsRune is { } rune && !Rune.IsControl (rune)) + { + if (_document is not null && !ReadOnly) + { + if (HasSelection) + { + ReplaceSelection (rune.ToString ()); + } + else if (OverwriteMode) + { + OverwriteAtCaret (rune.ToString ()); + } + else + { + _document.Insert (CaretOffset, rune.ToString ()); + } + + // Refresh the completion list with the updated prefix. + NotifyCompletionAfterInsert (); + } + + return true; + } + + // Backspace: delete the character before the caret and refresh. + if (key == Key.Backspace) + { + if (_document is not null && !ReadOnly && CaretOffset > 0) + { + _document.Remove (CaretOffset - 1, 1); + } + + NotifyCompletionAfterInsert (); + + return true; + } } // Check provider-specific triggers (e.g. Ctrl+Space). @@ -146,6 +198,53 @@ internal bool HandleCompletionKey (Key key) return false; } + /// + /// Handles mouse events for the completion popup. When the popup is visible + /// (rendered as a Popover with Enabled = false), mouse clicks + /// in the popup area are detected by the Editor and mapped to completion items. + /// Single-click on an item accepts the completion. + /// + internal bool HandleCompletionMouse (Mouse mouse) + { + if (!IsCompletionActive || _completionPopover is null || _completionListView is null) + { + return false; + } + + if (!mouse.IsSingleClicked) + { + 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; + + if (mouse.ScreenPosition.X < popoverScreenFrame.X + || mouse.ScreenPosition.X >= popoverScreenFrame.Right + || mouse.ScreenPosition.Y < popoverScreenFrame.Y + || mouse.ScreenPosition.Y >= popoverScreenFrame.Bottom) + { + // Click is outside the popup — dismiss. + DismissCompletion (); + + return false; + } + + // Determine which item was clicked by Y offset within the Popover. + var clickedIdx = mouse.ScreenPosition.Y - popoverScreenFrame.Y; + + if (clickedIdx < 0 || clickedIdx >= _completionItems.Count) + { + return false; + } + + _completionSelectedIndex = clickedIdx; + AcceptCompletion (); + + return true; + } + /// /// Called after a character is inserted into the document. Refreshes or opens the /// completion popup if a provider is active. @@ -270,6 +369,7 @@ private void UpdateCompletionListSelection (int index) _completionListView.SelectedItem = index; _completionListView.SetNeedsDraw (); + _completionPopover?.SetNeedsDraw (); } private void ShowCompletionPopup () @@ -322,8 +422,11 @@ private void ShowCompletionPopup () Point caretScreen = GetCaretScreenPosition (); _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); - // Disable keyboard dispatch so the Popover doesn't capture text input. - // All navigation (Up/Down/Enter/Tab/Esc) is handled by HandleCompletionKey. + // Disable the Popover so it acts as a visual-only overlay. All keyboard events + // flow to the Editor via HandleCompletionKey (the Editor retains focus). Mouse + // clicks in the popup area are handled by HandleCompletionMouse called from + // OnMouseEvent. This follows the PopupAutocomplete pattern from Terminal.Gui: + // the host control owns all input dispatch, the popup just renders. _completionPopover.Enabled = false; } diff --git a/src/Terminal.Gui.Editor/Editor.Mouse.cs b/src/Terminal.Gui.Editor/Editor.Mouse.cs index 0e9eeb1..7b49c06 100644 --- a/src/Terminal.Gui.Editor/Editor.Mouse.cs +++ b/src/Terminal.Gui.Editor/Editor.Mouse.cs @@ -16,6 +16,13 @@ public partial class Editor /// protected override bool OnMouseEvent (Mouse mouse) { + // Completion popup click takes priority — when the popup is visible and the + // user clicks in its area, accept the clicked item. + if (HandleCompletionMouse (mouse)) + { + return true; + } + if (_document is null) { return false; diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index a80b0de..e3c6c60 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -151,10 +151,12 @@ public void HandleCompletionKey_Returns_False_Without_Provider () } [Fact] - public void HandleCompletionKey_Returns_False_For_Regular_Chars_While_Active () + public void HandleCompletionKey_Handles_Regular_Chars_While_Active () { - // When the popup is active, regular character keys must NOT be consumed - // so the Editor can insert the character and re-filter the list. + // When the popup is active, regular character keys ARE consumed by + // HandleCompletionKey — the character is inserted directly into the document + // and the completion list is refreshed. This avoids the Popover intercepting + // the key when it is visible. Editor editor = new () { Document = new TextDocument ("us"), @@ -165,8 +167,10 @@ public void HandleCompletionKey_Returns_False_For_Regular_Chars_While_Active () editor.NotifyCompletionAfterInsert (); Assert.True (editor.IsCompletionActive); - // A regular character key should return false — not consumed by the popup. - Assert.False (editor.HandleCompletionKey (new Key ('i'))); + // A regular character key should be consumed and the character inserted. + Assert.True (editor.HandleCompletionKey (new Key ('i'))); + Assert.Equal ("usi", editor.Document!.Text); + Assert.True (editor.IsCompletionActive, "Completion should still be active after 'usi' (matches 'using')"); } [Fact] From bd41bfa471e38c7aba9d391f2fa9965eb381b2a0 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 14:38:31 -0500 Subject: [PATCH 17/32] Refactor completion popup key handling and tests - Update Terminal.Gui to 2.2.0-rc.4. - Refactor Editor completion popup key handling for consistency: - Navigation keys (arrows, Esc) now dismiss popup. - Accept keys (Enter, Tab, Space) accept completion. - Up/down navigation cycles through completion list. - ListView disables internal navigation; Editor handles all input. - Synchronize selection via ValueChanged and Accepted events. - Expand and refactor EditorCompletionTests: - Use Application/Driver to simulate key input. - Add tests for navigation cycling and accept keys. - Use primary constructors for test providers. --- Directory.Build.props | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 88 ++++++-------- src/Terminal.Gui.Editor/Editor.Keyboard.cs | 4 +- .../EditorCompletionTests.cs | 110 +++++++++++++----- 4 files changed, 123 insertions(+), 81 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 5253cb4..83c36a6 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -27,7 +27,7 @@ LICENSE - 2.1.1-develop.98 + 2.2.0-rc.4 diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 82d5e8e..81d6488 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -91,17 +91,9 @@ internal string GetCompletionPrefix (out int prefixStart) /// /// Called before normal key dispatch. Returns when the completion - /// popup consumed the key (navigation / accept / dismiss / character insert). This runs + /// popup consumed the key (navigation / accept / dismiss). This runs /// in , which fires before command bindings. /// - /// - /// The Popover is set to Enabled = false after - /// so that it functions as a visual-only overlay — the Editor retains keyboard focus and - /// all keyboard events flow through this method. This mirrors how - /// PopupAutocomplete.ProcessKey in Terminal.Gui works: the host control intercepts - /// keys and routes navigation / accept / dismiss / character insertion to the autocomplete - /// system. Mouse interaction is handled by . - /// internal bool HandleCompletionKey (Key key) { if (CompletionProvider is null) @@ -112,44 +104,25 @@ internal bool HandleCompletionKey (Key key) // An active popup gets first crack at navigation keys. if (IsCompletionActive) { - if (key == Key.Esc) + // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. + if (key == Key.Esc || key == Key.CursorLeft || key == Key.CursorRight || key == Key.CursorUp || key == Key.CursorDown) { DismissCompletion (); - return true; + return false; } - if (key == Key.Enter || key == Key.Tab) + // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. + if (key == Key.Enter || key == Key.Tab || key == Key.Space) { AcceptCompletion (); return true; } - if (key == Key.CursorUp) - { - var newIdx = (_completionSelectedIndex - 1 + _completionItems.Count) % _completionItems.Count; - _completionSelectedIndex = newIdx; - UpdateCompletionListSelection (newIdx); - - return true; - } - - if (key == Key.CursorDown) - { - var newIdx = (_completionSelectedIndex + 1) % _completionItems.Count; - _completionSelectedIndex = newIdx; - UpdateCompletionListSelection (newIdx); - - return true; - } - // Character keys: insert directly into the document while the popup is open, - // then refresh the completion list. We handle this here (rather than letting it - // fall through to OnKeyDownNotHandled) because the Popover is Enabled = false - // and this is the only place that processes keyboard input during a completion - // session. - if (!key.IsCtrl && !key.IsAlt && key.AsRune is { } rune && !Rune.IsControl (rune)) + // then refresh the completion list. + if (key is { IsCtrl: false, IsAlt: false, AsRune: { } rune } && !Rune.IsControl (rune)) { if (_document is not null && !ReadOnly) { @@ -188,14 +161,15 @@ internal bool HandleCompletionKey (Key key) } // Check provider-specific triggers (e.g. Ctrl+Space). - if (CompletionProvider.ShouldTrigger (key)) + if (!CompletionProvider.ShouldTrigger (key)) { - ShowCompletion (); - - return true; + return false; } - return false; + ShowCompletion (); + + return true; + } /// @@ -368,8 +342,6 @@ private void UpdateCompletionListSelection (int index) } _completionListView.SelectedItem = index; - _completionListView.SetNeedsDraw (); - _completionPopover?.SetNeedsDraw (); } private void ShowCompletionPopup () @@ -398,8 +370,14 @@ private void ShowCompletionPopup () { Source = new ListWrapper (labels), Width = _completionItems.Max (i => i.Label.Length) + 2, - Height = visibleCount + Height = visibleCount, + TabStop = TabBehavior.NoStop }; + // Disable ListView's internal navigation handling since we're managing selection and accept keys at the Editor level. + _completionListView.KeystrokeNavigator = null; + _completionListView.KeyBindings.Add (Key.Tab, Command.Accept); + _completionListView.KeyBindings.Remove (Key.Space); + _completionListView.KeyBindings.Add (Key.Space, Command.Accept); _completionListView.SelectedItem = _completionSelectedIndex; IReadOnlyList capturedItems = _completionItems; @@ -415,19 +393,29 @@ private void ShowCompletionPopup () } return capturedItems[idx]; - } + }, + TabStop = TabBehavior.NoStop }; // Position the popup just below the caret. Point caretScreen = GetCaretScreenPosition (); _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); - // Disable the Popover so it acts as a visual-only overlay. All keyboard events - // flow to the Editor via HandleCompletionKey (the Editor retains focus). Mouse - // clicks in the popup area are handled by HandleCompletionMouse called from - // OnMouseEvent. This follows the PopupAutocomplete pattern from Terminal.Gui: - // the host control owns all input dispatch, the popup just renders. - _completionPopover.Enabled = false; + _completionListView.ValueChanged += (sender, args) => + { + if (args.NewValue is not null) + { + _completionSelectedIndex = args.NewValue.Value; + } + }; + + _completionPopover.Accepted += (sender, args) => + { + if (args.Context?.Value is int value) + { + _completionSelectedIndex = value; + } + }; } /// diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 4b0fe45..926c248 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -9,7 +9,7 @@ public partial class Editor /// Runs before command bindings. When completion is active, intercepts navigation and /// accept/dismiss keys (Enter, Tab, arrows, Esc) so they don't trigger the normal /// editor command bindings. Also checks provider-specific trigger keys (e.g. Ctrl+Space). - /// Additionally tracks the kill-ring consecutive-kill flag: snapshots + /// Additionally, tracks the kill-ring consecutive-kill flag: snapshots /// _lastCommandWasKill into _previousCommandWasKill, then clears /// _lastCommandWasKill. The kill commands re-set it after executing. /// @@ -24,7 +24,7 @@ protected override bool OnKeyDown (Key key) _previousCommandWasKill = _lastCommandWasKill; _lastCommandWasKill = false; - bool result = base.OnKeyDown (key); + var result = base.OnKeyDown (key); // Clear the snapshot so it does not leak into a subsequent InvokeCommand call. // If the dispatched command was a kill, _lastCommandWasKill is already true; diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index e3c6c60..dc63f10 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -1,8 +1,12 @@ // CoPilot - gpt-4.1 +using Terminal.Gui.App; using Terminal.Gui.Document; +using Terminal.Gui.Drivers; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; +using Terminal.Gui.Testing; +using Terminal.Gui.Views; using Xunit; namespace Terminal.Gui.Editor.Tests; @@ -173,15 +177,25 @@ public void HandleCompletionKey_Handles_Regular_Chars_While_Active () Assert.True (editor.IsCompletionActive, "Completion should still be active after 'usi' (matches 'using')"); } + // TODO: All Navigation keys (cursor up/down/pageup/down/etc...) need testing. + // TODO: In VS, these all cycle within the list instead of clamping at the ends. We should test that behavior too. + [Fact] public void ArrowKeys_Navigate_Completion_Selection () { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + // "he" matches "hello" and "help" — two items. Editor editor = new () { Document = new TextDocument ("he"), CompletionProvider = new MultiWordCompletionProvider ("hello", "help", "world") }; + top.Add (editor); + app.Begin (top); + editor.CaretOffset = 2; editor.NotifyCompletionAfterInsert (); @@ -189,7 +203,7 @@ public void ArrowKeys_Navigate_Completion_Selection () Assert.Equal (0, editor.CompletionSelectedIndex); // Down arrow should move to index 1. - Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + app.InjectKey (Key.CursorDown); Assert.Equal (1, editor.CompletionSelectedIndex); // Accept the completion — should insert the second item ("help"). @@ -198,21 +212,28 @@ public void ArrowKeys_Navigate_Completion_Selection () } [Fact] - public void ArrowUp_Wraps_To_Last_Item () + public void ArrowUp_At_First_Item_Wraps_To_Last_Item () { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + // "he" matches "hello" and "help" — two items. Editor editor = new () { Document = new TextDocument ("he"), CompletionProvider = new MultiWordCompletionProvider ("hello", "help") }; + top.Add (editor); + app.Begin (top); + editor.CaretOffset = 2; editor.NotifyCompletionAfterInsert (); Assert.True (editor.IsCompletionActive); // Up from index 0 should wrap to last item (index 1 = "help"). - Assert.True (editor.HandleCompletionKey (Key.CursorUp)); + app.InjectKey (Key.CursorUp); Assert.Equal (1, editor.CompletionSelectedIndex); editor.AcceptCompletion (); @@ -220,27 +241,74 @@ public void ArrowUp_Wraps_To_Last_Item () } [Fact] - public void ArrowDown_Wraps_To_First_Item () + public void ArrowDown_At_Last_Item_Cycles_To_Top () { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + // "he" matches "hello" and "help" — two items. Editor editor = new () { Document = new TextDocument ("he"), CompletionProvider = new MultiWordCompletionProvider ("hello", "help") }; + top.Add (editor); + app.Begin (top); + editor.CaretOffset = 2; editor.NotifyCompletionAfterInsert (); + Assert.NotNull (app.Popovers?.GetActivePopover ()); Assert.True (editor.IsCompletionActive); + Assert.Equal (0, editor.CompletionSelectedIndex); - // Down to index 1, then down again wraps to 0. - Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + // Down to index 1, then down again cycles to 0. + app.InjectKey (Key.CursorDown); Assert.Equal (1, editor.CompletionSelectedIndex); - Assert.True (editor.HandleCompletionKey (Key.CursorDown)); + + app.InjectKey (Key.CursorDown); Assert.Equal (0, editor.CompletionSelectedIndex); - editor.AcceptCompletion (); - Assert.Equal ("hello", editor.Document!.Text); + Assert.True (editor.IsCompletionActive); + } + + // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. + [Theory] + [InlineData (KeyCode.Enter)] + [InlineData (KeyCode.Tab)] + [InlineData (KeyCode.Space)] + public void ValidAcceptKeys_Accept_Completion (KeyCode acceptKey) + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + // "he" matches "hello" and "help" — two items. + Editor editor = new () + { + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + Assert.Equal (0, editor.CompletionSelectedIndex); + + // Down to index 1 + app.InjectKey (Key.CursorDown); + Assert.Equal (1, editor.CompletionSelectedIndex); + + app.InjectKey (acceptKey); + Assert.Equal (1, editor.CompletionSelectedIndex); + + Assert.False (editor.IsCompletionActive); + + Assert.Equal ("help", editor.Document!.Text); } [Fact] @@ -315,15 +383,8 @@ public void Typing_NonMatching_Char_While_Completion_Active_Dismisses () } /// Stub provider that always returns a single hard-coded item. - private sealed class StubCompletionProvider : IEditorCompletionProvider + private sealed class StubCompletionProvider (string word) : IEditorCompletionProvider { - private readonly string _word; - - public StubCompletionProvider (string word) - { - _word = word; - } - public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) { if (string.IsNullOrEmpty (prefix)) @@ -331,8 +392,8 @@ public IReadOnlyList GetCompletions (TextDocument document, int return []; } - return _word.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) - ? [new CompletionItem { Label = _word }] + return word.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) + ? [new CompletionItem { Label = word }] : []; } @@ -357,15 +418,8 @@ public bool ShouldTrigger (Key key) } /// Provider that returns all words starting with the given prefix. - private sealed class MultiWordCompletionProvider : IEditorCompletionProvider + private sealed class MultiWordCompletionProvider (params string[] words) : IEditorCompletionProvider { - private readonly string[] _words; - - public MultiWordCompletionProvider (params string[] words) - { - _words = words; - } - public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) { if (string.IsNullOrEmpty (prefix)) @@ -373,7 +427,7 @@ public IReadOnlyList GetCompletions (TextDocument document, int return []; } - return _words + return words .Where (w => w.StartsWith (prefix, StringComparison.OrdinalIgnoreCase)) .Select (w => new CompletionItem { Label = w }) .ToList (); From 7ad560ef11a28cedf95f8fabbf49157df78a873e Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 16:54:19 -0500 Subject: [PATCH 18/32] Refactor completion key handling to use key bindings Completion popup now respects user key bindings for Quit, NewLine, and InsertTab commands instead of hardcoded keys. Space no longer accepts completion; it inserts a space and dismisses the popup. Tests updated and regression test added for space behavior. Also, OnKeyDownNotHandled now uses the Quit command binding. --- src/Terminal.Gui.Editor/Editor.Completion.cs | 51 +++++++++++-------- src/Terminal.Gui.Editor/Editor.Keyboard.cs | 2 +- .../EditorCompletionTests.cs | 36 ++++++++++++- 3 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 81d6488..6b076a5 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -104,16 +104,19 @@ internal bool HandleCompletionKey (Key key) // An active popup gets first crack at navigation keys. if (IsCompletionActive) { - // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. - if (key == Key.Esc || key == Key.CursorLeft || key == Key.CursorRight || key == Key.CursorUp || key == Key.CursorDown) + // Esc (whatever is bound to Command.Quit) or an arrow key dismisses the popup. + // TODO: query the bindings for the cursor keys too instead of hardcoding them. + if (KeyMatches (Command.Quit) || key == Key.CursorLeft || key == Key.CursorRight || key == Key.CursorUp || key == Key.CursorDown) { DismissCompletion (); return false; } - // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. - if (key == Key.Enter || key == Key.Tab || key == Key.Space) + // Accept on the keys bound to NewLine (Enter) / InsertTab (Tab). SPACE is + // deliberately NOT an accept key: it falls through, inserts a space, and the + // now-empty prefix dismisses the popup (so "this is a test." stays intact). + if (KeyMatches (Command.NewLine) || KeyMatches (Command.InsertTab)) { AcceptCompletion (); @@ -124,25 +127,27 @@ internal bool HandleCompletionKey (Key key) // then refresh the completion list. if (key is { IsCtrl: false, IsAlt: false, AsRune: { } rune } && !Rune.IsControl (rune)) { - if (_document is not null && !ReadOnly) + if (_document is null || ReadOnly) { - if (HasSelection) - { - ReplaceSelection (rune.ToString ()); - } - else if (OverwriteMode) - { - OverwriteAtCaret (rune.ToString ()); - } - else - { - _document.Insert (CaretOffset, rune.ToString ()); - } - - // Refresh the completion list with the updated prefix. - NotifyCompletionAfterInsert (); + return true; } + if (HasSelection) + { + ReplaceSelection (rune.ToString ()); + } + else if (OverwriteMode) + { + OverwriteAtCaret (rune.ToString ()); + } + else + { + _document.Insert (CaretOffset, rune.ToString ()); + } + + // Refresh the completion list with the updated prefix. + NotifyCompletionAfterInsert (); + return true; } @@ -170,6 +175,12 @@ internal bool HandleCompletionKey (Key key) return true; + // Resolve a key against the Editor's own bindings instead of hardcoding literals, + // so completion follows any rebinding of these commands. + bool KeyMatches (Command command) + { + return KeyBindings.GetFirstFromCommands (command) is { } bound && key == bound; + } } /// diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 926c248..931ea52 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -42,7 +42,7 @@ protected override bool OnKeyDown (Key key) /// protected override bool OnKeyDownNotHandled (Key key) { - if (key == Key.Esc && HasMultipleCarets) + if (KeyBindings.GetFirstFromCommands (Command.Quit) is { } boundKey && key == boundKey && HasMultipleCarets) { ClearAdditionalCarets (); diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index dc63f10..16d3a5d 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -273,11 +273,12 @@ public void ArrowDown_At_Last_Item_Cycles_To_Top () Assert.True (editor.IsCompletionActive); } - // TODO: This should be querying the Editor key bindings for the relevant keys instead of hardcoding them here. + // Enter (the key bound to Command.NewLine) and Tab (Command.InsertTab) are the default + // accept keys. SPACE is deliberately NOT one — see + // Space_While_Completion_Active_Inserts_Space_And_Dismisses. [Theory] [InlineData (KeyCode.Enter)] [InlineData (KeyCode.Tab)] - [InlineData (KeyCode.Space)] public void ValidAcceptKeys_Accept_Completion (KeyCode acceptKey) { using IApplication app = Application.Create (); @@ -311,6 +312,37 @@ public void ValidAcceptKeys_Accept_Completion (KeyCode acceptKey) Assert.Equal ("help", editor.Document!.Text); } + // Regression guard for the "this is a test." → "this IsDefaulta test." bug: SPACE must + // not accept the selected item. It is an ordinary printable — it inserts a space, the + // now-empty prefix dismisses the popup, and the typed text is left intact. + [Fact] + public void Space_While_Completion_Active_Inserts_Space_And_Dismisses () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + // "he" matches "hello" and "help" — two items, "hello" preselected. + Editor editor = new () + { + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + app.InjectKey (Key.Space); + + // SPACE inserted literally, popup gone, no completion text applied. + Assert.False (editor.IsCompletionActive); + Assert.Equal ("he ", editor.Document!.Text); + } + [Fact] public void Setting_CompletionProvider_To_Null_Dismisses_Active_Session () { From f77877ed50a56436bf3704174b6e4e6d992653ac Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 17:22:33 -0500 Subject: [PATCH 19/32] Refine completion popup dismissal and selection logic Only Escape and Left/Right arrows now dismiss the completion popup; Up/Down arrows are handled by the ListView for selection. Removed UpdateCompletionListSelection and integrated its logic into the ValueChanged event handler. Simplified popup Accepted event handling. Added missing DragMode.Select case in mouse handling. Introduced tests to verify popup dismissal and caret movement with horizontal arrows. --- src/Terminal.Gui.Editor/Editor.Completion.cs | 32 +++++-------------- src/Terminal.Gui.Editor/Editor.Mouse.cs | 1 + .../EditorCompletionTests.cs | 32 +++++++++++++++++++ 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 6b076a5..f4d85b6 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -104,9 +104,10 @@ internal bool HandleCompletionKey (Key key) // An active popup gets first crack at navigation keys. if (IsCompletionActive) { - // Esc (whatever is bound to Command.Quit) or an arrow key dismisses the popup. - // TODO: query the bindings for the cursor keys too instead of hardcoding them. - if (KeyMatches (Command.Quit) || key == Key.CursorLeft || key == Key.CursorRight || key == Key.CursorUp || key == Key.CursorDown) + // Esc (Command.Quit) or a horizontal caret move (Left/Right) dismisses the popup. + // Up/Down are intentionally absent: the focused popup ListView consumes them to + // move the selection, so they never reach here. + if (KeyMatches (Command.Quit) || KeyMatches (Command.Left) || KeyMatches (Command.Right)) { DismissCompletion (); @@ -344,17 +345,6 @@ internal void AcceptCompletion () } } - /// Updates the visible ListView selection if the list view exists. - private void UpdateCompletionListSelection (int index) - { - if (_completionListView is null) - { - return; - } - - _completionListView.SelectedItem = index; - } - private void ShowCompletionPopup () { if (_completionItems.Count == 0) @@ -412,19 +402,13 @@ private void ShowCompletionPopup () Point caretScreen = GetCaretScreenPosition (); _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); - _completionListView.ValueChanged += (sender, args) => + // While the LV has focus, track selection changes to update the selected index, + // but accept on Enter/Tab/Space (handled in HandleCompletionKey). + _completionListView.ValueChanged += (_, args) => { if (args.NewValue is not null) { - _completionSelectedIndex = args.NewValue.Value; - } - }; - - _completionPopover.Accepted += (sender, args) => - { - if (args.Context?.Value is int value) - { - _completionSelectedIndex = value; + _completionSelectedIndex = args.NewValue.Value; } }; } diff --git a/src/Terminal.Gui.Editor/Editor.Mouse.cs b/src/Terminal.Gui.Editor/Editor.Mouse.cs index 7b49c06..2ac53b1 100644 --- a/src/Terminal.Gui.Editor/Editor.Mouse.cs +++ b/src/Terminal.Gui.Editor/Editor.Mouse.cs @@ -85,6 +85,7 @@ protected override bool OnMouseEvent (Mouse mouse) case DragMode.AddCaret: return true; + case DragMode.Select: default: // Route through the selection helper so SelectionChanged fires only on real changes. ExtendCaretTo (MousePositionToOffset (pos)); diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 16d3a5d..4e39eb9 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -273,6 +273,38 @@ public void ArrowDown_At_Last_Item_Cycles_To_Top () Assert.True (editor.IsCompletionActive); } + // Horizontal caret movement dismisses the popup — unlike Up/Down, which the focused + // popup ListView consumes to move the selection. The key still falls through, so the + // caret moves too (HandleCompletionKey returns false after dismissing). + [Theory] + [InlineData (KeyCode.CursorLeft, 1)] + [InlineData (KeyCode.CursorRight, 3)] + public void Arrow_Left_Or_Right_While_Completion_Active_Dismisses_And_Moves_Caret (KeyCode navKey, int expectedCaret) + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + // Caret after "he"; the trailing 'x' gives CursorRight room to move. + Editor editor = new () + { + Document = new TextDocument ("hex"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + app.InjectKey (navKey); + + Assert.False (editor.IsCompletionActive); + Assert.Equal (expectedCaret, editor.CaretOffset); + } + // Enter (the key bound to Command.NewLine) and Tab (Command.InsertTab) are the default // accept keys. SPACE is deliberately NOT one — see // Space_While_Completion_Active_Inserts_Space_And_Dismisses. From a4bb1a178684c577f16e94c0352024ebffb694f4 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 17:56:11 -0500 Subject: [PATCH 20/32] Refactor completion popup teardown and Esc handling Refactored Editor completion popup teardown with a new DisposeCompletionPopover() for safe, reentrant disposal. Added OnCompletionPopoverVisibleChanged() to sync Editor state with Terminal.Gui's auto-hide, preventing phantom accepts after Esc. Improved ListView setup and XML docs. Fixed multi-caret Esc handling to always clear blocks. Updated tests to use EditorTestHost and added a regression test for Esc/Enter completion behavior. Minor test cleanups included. --- src/Terminal.Gui.Editor/Editor.Completion.cs | 85 +++++++++++++------ src/Terminal.Gui.Editor/Editor.Keyboard.cs | 15 +++- .../EditorTests.cs | 67 ++++++++------- .../EditorCompletionTests.cs | 41 ++++++++- 4 files changed, 144 insertions(+), 64 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index f4d85b6..a59f3d3 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -185,10 +185,10 @@ bool KeyMatches (Command command) } /// - /// Handles mouse events for the completion popup. When the popup is visible - /// (rendered as a Popover with Enabled = false), mouse clicks - /// in the popup area are detected by the Editor and mapped to completion items. - /// Single-click on an item accepts the completion. + /// Handles mouse events for the completion popup: a single click inside the popup + /// area selects and accepts the clicked item; a click outside dismisses the popup. + /// The hit-test maps screen Y to an item index and does not account for a scrolled + /// list (only the first page is addressable). /// internal bool HandleCompletionMouse (Mouse mouse) { @@ -293,18 +293,50 @@ internal void ShowCompletion () ShowCompletionPopup (); } - /// Hides the completion popup if it is visible. + /// Tears down the completion session: disposes the popup and clears the item list. internal void DismissCompletion () { - if (_completionPopover is not null) + DisposeCompletionPopover (); + _completionItems = []; + } + + /// + /// Disposes the popover and its ListView (if any) without clearing + /// — used both to dismiss and to swap in a fresh popup. + /// + /// + /// Null-out-then-dispose order plus unsubscribing first makes this reentrant-safe: + /// disposing the popover flips Visible, but the handler is already detached and + /// the field already , so re-entry is a no-op. + /// + private void DisposeCompletionPopover () + { + Popover? popover = _completionPopover; + _completionPopover = null; + _completionListView = null; + + if (popover is null) { - _completionPopover.Visible = false; - _completionPopover.Dispose (); - _completionPopover = null; - _completionListView = null; + return; } - _completionItems = []; + popover.VisibleChanged -= OnCompletionPopoverVisibleChanged; + popover.Visible = false; + popover.Dispose (); + } + + /// + /// Terminal.Gui auto-hides the popover on Esc, a click outside it, focus change, or + /// another popover opening — it just flips Visible and never tells the Editor. + /// Without this, would stay + /// and a subsequent Enter/click would fire a phantom . + /// + private void OnCompletionPopoverVisibleChanged (object? sender, EventArgs e) + { + if (_completionPopover is { Visible: false }) + { + DismissCompletion (); + } } /// @@ -352,14 +384,9 @@ private void ShowCompletionPopup () return; } - // Dispose previous popover if any — create fresh each time so the list is rebuilt. - if (_completionPopover is not null) - { - _completionPopover.Visible = false; - _completionPopover.Dispose (); - _completionPopover = null; - _completionListView = null; - } + // Drop the previous popover (if any) — a fresh one is built each time so the + // list rebuilds. Reentrant-safe so the VisibleChanged teardown can't NRE here. + DisposeCompletionPopover (); // Build the label list for the ListView. ObservableCollection labels = new (_completionItems.Select (i => i.Label)); @@ -374,11 +401,12 @@ private void ShowCompletionPopup () Height = visibleCount, TabStop = TabBehavior.NoStop }; - // Disable ListView's internal navigation handling since we're managing selection and accept keys at the Editor level. + // The ListView owns no accept/dismiss semantics: HandleCompletionKey resolves + // accept (Enter/Tab) and dismiss (Esc/Left/Right) at the Editor level, while the + // focused ListView's own Up/Down move the selection. Binding Space->Accept here + // would silently re-introduce the "this is a test." accept-on-space bug. Disable + // type-ahead so a stray letter can't hijack the list instead of reaching the editor. _completionListView.KeystrokeNavigator = null; - _completionListView.KeyBindings.Add (Key.Tab, Command.Accept); - _completionListView.KeyBindings.Remove (Key.Space); - _completionListView.KeyBindings.Add (Key.Space, Command.Accept); _completionListView.SelectedItem = _completionSelectedIndex; IReadOnlyList capturedItems = _completionItems; @@ -398,17 +426,22 @@ private void ShowCompletionPopup () TabStop = TabBehavior.NoStop }; + // Tear the session down when TG auto-hides the popover (Esc / click-outside / + // focus change), so IsCompletionActive can't go stale and trigger a phantom accept. + _completionPopover.VisibleChanged += OnCompletionPopoverVisibleChanged; + // Position the popup just below the caret. Point caretScreen = GetCaretScreenPosition (); _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); - // While the LV has focus, track selection changes to update the selected index, - // but accept on Enter/Tab/Space (handled in HandleCompletionKey). + // The focused ListView's Up/Down move its selection; mirror that into + // _completionSelectedIndex so AcceptCompletion inserts the right item. + // Accept/dismiss keys are resolved separately in HandleCompletionKey. _completionListView.ValueChanged += (_, args) => { if (args.NewValue is not null) { - _completionSelectedIndex = args.NewValue.Value; + _completionSelectedIndex = args.NewValue.Value; } }; } diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 931ea52..9a78503 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -1,4 +1,5 @@ using System.Text; +using Terminal.Gui.App; using Terminal.Gui.Input; namespace Terminal.Gui.Editor; @@ -6,9 +7,10 @@ namespace Terminal.Gui.Editor; public partial class Editor { /// - /// Runs before command bindings. When completion is active, intercepts navigation and - /// accept/dismiss keys (Enter, Tab, arrows, Esc) so they don't trigger the normal - /// editor command bindings. Also checks provider-specific trigger keys (e.g. Ctrl+Space). + /// Runs before command bindings. When completion is active, intercepts accept + /// (Enter/Tab) and dismiss (Esc/Left/Right) keys so they don't trigger the normal + /// editor command bindings; Up/Down are left to the focused popup ListView. Also + /// checks provider-specific trigger keys (e.g. Ctrl+Space). /// Additionally, tracks the kill-ring consecutive-kill flag: snapshots /// _lastCommandWasKill into _previousCommandWasKill, then clears /// _lastCommandWasKill. The kill commands re-set it after executing. @@ -42,7 +44,12 @@ protected override bool OnKeyDown (Key key) /// protected override bool OnKeyDownNotHandled (Key key) { - if (KeyBindings.GetFirstFromCommands (Command.Quit) is { } boundKey && key == boundKey && HasMultipleCarets) + // Esc clears a multi-caret block. Resolve the key from the application's + // Command.Quit binding — the Editor itself binds no Quit, so the earlier + // Editor-scoped KeyBindings.GetFirstFromCommands(Command.Quit) lookup resolved + // to null and the clear never ran (regressed multi-caret Esc in 7ad560e). This + // tracks the same key Terminal.Gui uses framework-wide for escape/cancel. + if (key == Application.GetDefaultKey (Command.Quit) && HasMultipleCarets) { ClearAdditionalCarets (); diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorTests.cs index 233a054..095d656 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorTests.cs @@ -1,6 +1,6 @@ // Claude - claude-opus-4-7 -using Terminal.Gui.Drivers; +using System.Drawing; using Terminal.Gui.Editor.IntegrationTests.Testing; using Terminal.Gui.Input; using Terminal.Gui.Testing; @@ -20,7 +20,7 @@ public class EditorTests [Fact] public async Task Renders_InitialDocumentText () { - await using AppFixture fx = new (() => new ("Hello world")); + await using AppFixture fx = new (() => new EditorTestHost ("Hello world")); DriverAssert.ContentsContains (fx.Driver, "Hello world"); } @@ -28,7 +28,7 @@ public async Task Renders_InitialDocumentText () [Fact] public async Task Typing_ASCII_Inserts_Characters () { - await using AppFixture fx = new (() => new ()); + await using AppFixture fx = new (() => new EditorTestHost ()); fx.Top.Editor.SetFocus (); fx.Injector.InjectKey (Key.H, Direct); @@ -42,7 +42,7 @@ public async Task Typing_ASCII_Inserts_Characters () [Fact] public async Task CursorLeft_Right_MovesCaret () { - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 3; @@ -64,7 +64,8 @@ public async Task CursorLeft_Right_MovesCaret () [Fact] public async Task CursorUp_Down_PreservesVirtualColumn_AcrossShortLines () { - await using AppFixture fx = new (() => new ("longer line\nshort\nanother long line")); + await using AppFixture fx = new (() => + new EditorTestHost ("longer line\nshort\nanother long line")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 8; // column 8 of "longer line" @@ -85,7 +86,7 @@ public async Task CursorUp_Down_PreservesVirtualColumn_AcrossShortLines () [Fact] public async Task CursorUp_Down_PreservesVirtualColumn_Across_Tab_Line () { - await using AppFixture fx = new (() => new ("abcde\n\t\nabcde")); + await using AppFixture fx = new (() => new EditorTestHost ("abcde\n\t\nabcde")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 3; @@ -115,7 +116,7 @@ public async Task CursorDown_PreservesVirtualColumn_AcrossThreeIntermediateShort var text = string.Join ("\n", LongTop, Short1, Empty, Short2, LongBottom); - await using AppFixture fx = new (() => new (text)); + await using AppFixture fx = new (() => new EditorTestHost (text)); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 12; // column 12 on the top long line @@ -144,7 +145,7 @@ public async Task Typing_NUL_Does_Not_Insert () // covers it, so the explicit `rune != default` guard in Editor.Keyboard.cs is redundant. // This test locks in the behavior so removing the redundant check can't silently start // inserting NUL into the document. - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 3; @@ -160,7 +161,7 @@ public async Task Typing_NUL_Does_Not_Insert () [Fact] public async Task Home_End_Move_WithinLine () { - await using AppFixture fx = new (() => new ("first\nsecond")); + await using AppFixture fx = new (() => new EditorTestHost ("first\nsecond")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = "first\n".Length + 2; // line 2, col 2 @@ -174,7 +175,7 @@ public async Task Home_End_Move_WithinLine () [Fact] public async Task Backspace_Removes_CharBefore_Caret () { - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 3; @@ -187,7 +188,7 @@ public async Task Backspace_Removes_CharBefore_Caret () [Fact] public async Task Delete_Removes_CharAt_Caret () { - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -200,7 +201,7 @@ public async Task Delete_Removes_CharAt_Caret () [Fact] public async Task Enter_Inserts_Newline () { - await using AppFixture fx = new (() => new ("ab")); + await using AppFixture fx = new (() => new EditorTestHost ("ab")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -214,7 +215,7 @@ public async Task Enter_Inserts_Newline () [Fact] public async Task CtrlZ_Undoes_LastEdit () { - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.Document?.Insert (3, "DEF"); @@ -228,7 +229,7 @@ public async Task CtrlZ_Undoes_LastEdit () [Fact] public async Task CtrlY_Redoes_LastUndo () { - await using AppFixture fx = new (() => new ("abc")); + await using AppFixture fx = new (() => new EditorTestHost ("abc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.Document?.Insert (3, "DEF"); fx.Injector.InjectKey (Key.Z.WithCtrl, Direct); @@ -243,7 +244,7 @@ public async Task CtrlY_Redoes_LastUndo () [Fact] public async Task MultiLine_Document_Renders_AllLines () { - await using AppFixture fx = new (() => new ("alpha\nbeta\ngamma")); + await using AppFixture fx = new (() => new EditorTestHost ("alpha\nbeta\ngamma")); fx.Render (); DriverAssert.ContentsContains (fx.Driver, "alpha"); @@ -261,7 +262,7 @@ public async Task LongDocument_Scrolls_To_Keep_Caret_Visible () lines[i] = $"line-{i:00}"; } - await using AppFixture fx = new (() => new (string.Join ("\n", lines))); + await using AppFixture fx = new (() => new EditorTestHost (string.Join ("\n", lines))); fx.Top.Editor.SetFocus (); // Place caret on line index 40 (0-based). @@ -287,17 +288,19 @@ public async Task MouseWheel_Scrolls_LongDocument () lines[i] = $"line-{i:00}"; } - await using AppFixture fx = new (() => new (string.Join ("\n", lines)), height: 6); + await using AppFixture fx = new (() => new EditorTestHost (string.Join ("\n", lines)), + height: 6); fx.Render (); DriverAssert.ContentsContains (fx.Driver, "line-00"); - fx.Injector.InjectMouse (new () { ScreenPosition = new (1, 1), Flags = MouseFlags.WheeledDown }, Direct); + fx.Injector.InjectMouse (new Mouse { ScreenPosition = new Point (1, 1), Flags = MouseFlags.WheeledDown }, + Direct); fx.Render (); Assert.True (fx.Top.Editor.Viewport.Y > 0); DriverAssert.ContentsDoesNotContain (fx.Driver, "line-00"); - fx.Injector.InjectMouse (new () { ScreenPosition = new (1, 1), Flags = MouseFlags.WheeledUp }, Direct); + fx.Injector.InjectMouse (new Mouse { ScreenPosition = new Point (1, 1), Flags = MouseFlags.WheeledUp }, Direct); fx.Render (); Assert.Equal (0, fx.Top.Editor.Viewport.Y); @@ -313,7 +316,7 @@ public async Task MouseWheel_Scrolls_LongDocument () [Fact] public async Task CtrlAltDown_Adds_Vertically_Aligned_Carets () { - await using AppFixture fx = new (() => new ("longer line\nshrt\nanother line")); + await using AppFixture fx = new (() => new EditorTestHost ("longer line\nshrt\nanother line")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 8; @@ -330,7 +333,7 @@ public async Task CtrlAltDown_Adds_Vertically_Aligned_Carets () [Fact] public async Task CtrlAltUp_Adds_Caret_On_Line_Above () { - await using AppFixture fx = new (() => new ("abcd\nabcd\nabcd")); + await using AppFixture fx = new (() => new EditorTestHost ("abcd\nabcd\nabcd")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 11; @@ -350,7 +353,7 @@ public async Task CtrlAltUp_Adds_Caret_On_Line_Above () [Fact] public async Task CtrlAltDown_Preserves_Exact_Column_On_Next_Long_Line_After_Short_Line () { - await using AppFixture fx = new (() => new ("abcde\nx\nabcde")); + await using AppFixture fx = new (() => new EditorTestHost ("abcde\nx\nabcde")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 4; @@ -363,7 +366,7 @@ public async Task CtrlAltDown_Preserves_Exact_Column_On_Next_Long_Line_After_Sho [Fact] public async Task CtrlAltDown_Preserves_Column_With_Tabs () { - await using AppFixture fx = new (() => new ("a\tbcde\na\tbcde\na\tbcde")); + await using AppFixture fx = new (() => new EditorTestHost ("a\tbcde\na\tbcde\na\tbcde")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 3; @@ -377,7 +380,7 @@ public async Task CtrlAltDown_Preserves_Column_With_Tabs () [Fact] public async Task Esc_Dismisses_MultiCaret_And_Down_Can_Move_Past_Previous_Block () { - await using AppFixture fx = new (() => new ("abcd\nabcd\nabcd\nabcd")); + await using AppFixture fx = new (() => new EditorTestHost ("abcd\nabcd\nabcd\nabcd")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -398,7 +401,7 @@ public async Task Esc_Dismisses_MultiCaret_And_Down_Can_Move_Past_Previous_Block [Fact] public async Task Esc_After_Moving_Within_MultiCaret_Allows_Moving_Below_Last_Former_Multi () { - await using AppFixture fx = new (() => new ("abcd\nabcd\nabcd\nabcd")); + await using AppFixture fx = new (() => new EditorTestHost ("abcd\nabcd\nabcd\nabcd")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -415,7 +418,7 @@ public async Task Esc_After_Moving_Within_MultiCaret_Allows_Moving_Below_Last_Fo [Fact] public async Task Vertical_MultiCaret_Does_Not_Duplicate_When_Primary_Moves_Onto_Additional () { - await using AppFixture fx = new (() => new ("aa\naa\naa")); + await using AppFixture fx = new (() => new EditorTestHost ("aa\naa\naa")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -430,7 +433,7 @@ public async Task Vertical_MultiCaret_Does_Not_Duplicate_When_Primary_Moves_Onto [Fact] public async Task Tab_Inserts_At_All_Carets () { - await using AppFixture fx = new (() => new ("ab\nab\nab")); + await using AppFixture fx = new (() => new EditorTestHost ("ab\nab\nab")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -445,7 +448,7 @@ public async Task Tab_Inserts_At_All_Carets () public async Task Tab_Twice_Inserts_Consistently_At_All_Vertical_Carets_With_Spaces () { await using AppFixture fx = - new (() => new ("using Ted;\nusing Terminal.Gui.App;\nusing Terminal.Gui.Configuration;")); + new (() => new EditorTestHost ("using Ted;\nusing Terminal.Gui.App;\nusing Terminal.Gui.Configuration;")); fx.Top.Editor.SetFocus (); fx.Top.Editor.ConvertTabsToSpaces = true; fx.Top.Editor.CaretOffset = "using".Length; @@ -468,7 +471,7 @@ public async Task Tab_Twice_Inserts_Consistently_At_All_Vertical_Carets_With_Spa [Fact] public async Task ShiftTab_Unindents_At_All_Carets_In_One_Undo_Step () { - await using AppFixture fx = new (() => new ("\tab\n\tab\n\tab")); + await using AppFixture fx = new (() => new EditorTestHost ("\tab\n\tab\n\tab")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; @@ -487,7 +490,7 @@ public async Task ShiftTab_Unindents_At_All_Carets_In_One_Undo_Step () [Fact] public async Task CtrlAltDown_Then_CtrlAltUp_Collapses_Last_Down_Selection () { - await using AppFixture fx = new (() => new ("a\nb\nc")); + await using AppFixture fx = new (() => new EditorTestHost ("a\nb\nc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 0; @@ -502,7 +505,7 @@ public async Task CtrlAltDown_Then_CtrlAltUp_Collapses_Last_Down_Selection () [Fact] public async Task CtrlAltUp_Then_CtrlAltDown_Collapses_Last_Up_Selection () { - await using AppFixture fx = new (() => new ("a\nb\nc")); + await using AppFixture fx = new (() => new EditorTestHost ("a\nb\nc")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 2; @@ -517,7 +520,7 @@ public async Task CtrlAltUp_Then_CtrlAltDown_Collapses_Last_Up_Selection () [Fact] public async Task Primary_Caret_Is_Visible_After_Exiting_MultiCaret () { - await using AppFixture fx = new (() => new ("abcd\nabcd\nabcd\nabcd")); + await using AppFixture fx = new (() => new EditorTestHost ("abcd\nabcd\nabcd\nabcd")); fx.Top.Editor.SetFocus (); fx.Top.Editor.CaretOffset = 1; diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 4e39eb9..1409563 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -177,8 +177,8 @@ public void HandleCompletionKey_Handles_Regular_Chars_While_Active () Assert.True (editor.IsCompletionActive, "Completion should still be active after 'usi' (matches 'using')"); } - // TODO: All Navigation keys (cursor up/down/pageup/down/etc...) need testing. - // TODO: In VS, these all cycle within the list instead of clamping at the ends. We should test that behavior too. + // Up/Down navigation and end-of-list cycling are covered below. Still untested: + // PageUp/PageDown/Home/End within the list, and mouse selection. [Fact] public void ArrowKeys_Navigate_Completion_Selection () @@ -305,6 +305,43 @@ public void Arrow_Left_Or_Right_While_Completion_Active_Dismisses_And_Moves_Care Assert.Equal (expectedCaret, editor.CaretOffset); } + // Regression (user-reported): type "te", Esc, Enter produced "Ted" instead of "te\n". + // Terminal.Gui auto-hides the popover on Esc but never tells the Editor, so + // IsCompletionActive stayed true and the next Enter accepted. The popover's + // VisibleChanged handler must tear the session down on auto-hide. + [Fact] + public void Esc_AutoHides_Popover_So_Following_Enter_Newlines_Not_Accepts () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + Editor editor = new () + { + Document = new TextDocument ("te"), + CompletionProvider = new MultiWordCompletionProvider ("Ted", "TedApp") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Esc: TG auto-hides the popover → VisibleChanged → session torn down. + app.InjectKey (Key.Esc); + Assert.False (editor.IsCompletionActive); + + // Enter must newline, not resurrect the dead completion. + app.InjectKey (Key.Enter); + + Assert.StartsWith ("te", editor.Document!.Text); + Assert.Contains ("\n", editor.Document!.Text); + Assert.DoesNotContain ("Ted", editor.Document!.Text); + Assert.False (editor.IsCompletionActive); + } + // Enter (the key bound to Command.NewLine) and Tab (Command.InsertTab) are the default // accept keys. SPACE is deliberately NOT one — see // Space_While_Completion_Active_Inserts_Space_And_Dismisses. From ba56970fa20a174b56ca505392be9c974f10afdc Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:05:57 -0500 Subject: [PATCH 21/32] Update completion popup dismissal to use app Quit key Completion popup is now dismissed only when the application's default Quit key is pressed, instead of any Editor-scoped Quit binding. Left/Right arrow key behavior is unchanged. Comments were clarified. --- src/Terminal.Gui.Editor/Editor.Completion.cs | 30 +++++++++++--------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index a59f3d3..8c13bc4 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -15,7 +15,6 @@ public partial class Editor private ListView? _completionListView; private Popover? _completionPopover; private int _completionPrefixStart; - private int _completionSelectedIndex; /// /// Gets or sets the completion provider that supplies suggestions for the in-editor @@ -44,7 +43,7 @@ public IEditorCompletionProvider? CompletionProvider public bool IsCompletionActive => _completionItems.Count > 0; /// Gets the zero-based index of the currently selected completion item. - internal int CompletionSelectedIndex => _completionSelectedIndex; + internal int CompletionSelectedIndex { get; private set; } /// /// Extracts the word-prefix immediately before the caret (letters, digits, underscores) @@ -104,10 +103,12 @@ internal bool HandleCompletionKey (Key key) // An active popup gets first crack at navigation keys. if (IsCompletionActive) { - // Esc (Command.Quit) or a horizontal caret move (Left/Right) dismisses the popup. - // Up/Down are intentionally absent: the focused popup ListView consumes them to - // move the selection, so they never reach here. - if (KeyMatches (Command.Quit) || KeyMatches (Command.Left) || KeyMatches (Command.Right)) + // Esc (the application's Command.Quit key — the Editor binds no Quit, so this + // must come from the app, not the Editor-scoped KeyMatches) or a horizontal + // caret move (Left/Right) dismisses the popup. Up/Down are intentionally absent: + // the focused popup ListView consumes them to move the selection. + if (key == Application.GetDefaultKey (Command.Quit) || KeyMatches (Command.Left) || + KeyMatches (Command.Right)) { DismissCompletion (); @@ -225,7 +226,7 @@ internal bool HandleCompletionMouse (Mouse mouse) return false; } - _completionSelectedIndex = clickedIdx; + CompletionSelectedIndex = clickedIdx; AcceptCompletion (); return true; @@ -263,7 +264,7 @@ internal void NotifyCompletionAfterInsert () _completionPrefixStart = prefixStart; _completionItems = items; - _completionSelectedIndex = 0; + CompletionSelectedIndex = 0; ShowCompletionPopup (); } @@ -289,7 +290,7 @@ internal void ShowCompletion () _completionPrefixStart = prefixStart; _completionItems = items; - _completionSelectedIndex = 0; + CompletionSelectedIndex = 0; ShowCompletionPopup (); } @@ -350,14 +351,14 @@ internal void AcceptCompletion () return; } - if (_completionSelectedIndex < 0 || _completionSelectedIndex >= _completionItems.Count) + if (CompletionSelectedIndex < 0 || CompletionSelectedIndex >= _completionItems.Count) { DismissCompletion (); return; } - CompletionItem selected = _completionItems[_completionSelectedIndex]; + CompletionItem selected = _completionItems[CompletionSelectedIndex]; var insertText = selected.TextToInsert; var replaceLength = CaretOffset - _completionPrefixStart; @@ -407,7 +408,8 @@ private void ShowCompletionPopup () // would silently re-introduce the "this is a test." accept-on-space bug. Disable // type-ahead so a stray letter can't hijack the list instead of reaching the editor. _completionListView.KeystrokeNavigator = null; - _completionListView.SelectedItem = _completionSelectedIndex; + _completionListView.MouseBindings.ReplaceCommands (MouseFlags.LeftButtonClicked, Command.Accept); + _completionListView.SelectedItem = CompletionSelectedIndex; IReadOnlyList capturedItems = _completionItems; @@ -441,9 +443,11 @@ private void ShowCompletionPopup () { if (args.NewValue is not null) { - _completionSelectedIndex = args.NewValue.Value; + CompletionSelectedIndex = args.NewValue.Value; } }; + + _completionListView.Accepted += (_, _) => AcceptCompletion (); } /// From 8f07a316fcecd1dacabaa209219473a66ed2389f Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:30:11 -0500 Subject: [PATCH 22/32] Fix Enter double-accept; add real InjectMouse completion tests Accepted fires on both Enter and mouse-click; the handler now only syncs the selected index instead of calling AcceptCompletion(), which double-handled Enter and leaked a trailing newline. Acceptance is driven explicitly by HandleCompletionKey (Enter/Tab) and HandleCompletionMouse (click). Adds two regression tests via real app.InjectMouse against the live popover: single-click on an item accepts it; a press outside the popover dismisses it and inserts nothing (Bug-2 guard). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Terminal.Gui.Editor/Editor.Completion.cs | 12 ++- .../EditorCompletionTests.cs | 90 +++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 8c13bc4..ad21f9a 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -447,7 +447,17 @@ private void ShowCompletionPopup () } }; - _completionListView.Accepted += (_, _) => AcceptCompletion (); + // 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) => + { + if (args.Context?.Value is int idx) + { + CompletionSelectedIndex = idx; + } + }; } /// diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 1409563..f5a3903 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -1,11 +1,13 @@ // CoPilot - gpt-4.1 +using System.Drawing; using Terminal.Gui.App; using Terminal.Gui.Document; using Terminal.Gui.Drivers; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; using Terminal.Gui.Testing; +using Terminal.Gui.ViewBase; using Terminal.Gui.Views; using Xunit; @@ -483,6 +485,94 @@ public void Typing_NonMatching_Char_While_Completion_Active_Dismisses () Assert.False (editor.IsCompletionActive); } + // Missing-coverage regression: clicking an item in the popover must select+accept that + // item. Nothing exercised HandleCompletionMouse before, so its hit-test could (and did) + // break silently. + [Fact] + public void SingleClick_On_Popover_Item_Accepts_That_Item () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + // "he" → [hello, help, helm]. We click index 1 ("help"). + Editor editor = new () + { + Width = Dim.Fill (), + Height = Dim.Fill (), + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help", "helm") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + // Lay the popover out so its screen Frame is valid before we hit-test against it. + app.LayoutAndDraw (true); + var popover = (View)app.Popovers!.GetActivePopover ()!; + Rectangle frame = popover.Frame; + + // HandleCompletionMouse maps clickedIdx = ScreenPosition.Y - Frame.Y, so Frame.Y + 1 + // is the second item. + app.InjectMouse ( + new Mouse + { + ScreenPosition = new Point (frame.X, frame.Y + 1), + Flags = MouseFlags.LeftButtonClicked, + Timestamp = new DateTime (2025, 1, 1, 12, 0, 0) + }); + + Assert.False (editor.IsCompletionActive); + Assert.Equal ("help", editor.Document!.Text); + } + + // Bug-2 regression (user-reported: clicking elsewhere inserted "TedApp" at the click). + // Terminal.Gui hides an active popover on a mouse PRESS outside it (the press → + // release → click cycle), so this injects a real LeftButtonPressed clear of the + // popover frame and asserts the popup is gone with nothing accepted/inserted. + [Fact] + public void Click_Outside_Popover_Dismisses_And_Inserts_Nothing () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + Editor editor = new () + { + Width = Dim.Fill (), + Height = Dim.Fill (), + Document = new TextDocument ("he"), + CompletionProvider = new MultiWordCompletionProvider ("hello", "help", "helm") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 2; + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + app.LayoutAndDraw (true); + var popover = (View)app.Popovers!.GetActivePopover ()!; + Rectangle frame = popover.Frame; + + var before = editor.Document!.Text; + + // Press well clear of the popover (column 0 is left of it; a few rows below). + app.InjectMouse ( + new Mouse + { + ScreenPosition = new Point (0, frame.Bottom + 3), + Flags = MouseFlags.LeftButtonPressed, + Timestamp = new DateTime (2025, 1, 1, 12, 0, 0) + }); + + Assert.False (editor.IsCompletionActive); + Assert.Equal (before, editor.Document!.Text); + } + /// Stub provider that always returns a single hard-coded item. private sealed class StubCompletionProvider (string word) : IEditorCompletionProvider { From d9f9fb921eb5d8fae63f8b4e207063f24275d667 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:43:50 -0500 Subject: [PATCH 23/32] Route completion popup edits through canonical insert/delete helpers The completion popup's key handler re-implemented character insertion and Backspace, and did so incorrectly: it skipped the multi-caret branch (typed at a single caret only) and the Backspace path ignored selection and multi-caret. Extract InsertTypedText and DeleteCharLeftAndRefresh as the single canonical typed-edit + completion-refresh paths. OnKeyDownNotHandled, Command.DeleteCharLeft, and HandleCompletionKey now all route through them, so behavior cannot drift. Backspace in the popup is resolved via KeyMatches(Command.DeleteCharLeft) instead of a hardcoded Key.Backspace literal (the binding exists, unlike the earlier Command.Quit case). Adds regression tests proving a typed char and Backspace while the popup is open are applied at every caret. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Terminal.Gui.Editor/Editor.Commands.cs | 8 +--- src/Terminal.Gui.Editor/Editor.Completion.cs | 39 +++------------- src/Terminal.Gui.Editor/Editor.Keyboard.cs | 36 +++++++++++++-- .../EditorCompletionTests.cs | 46 +++++++++++++++++++ 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Commands.cs b/src/Terminal.Gui.Editor/Editor.Commands.cs index 33faac4..c720fd7 100644 --- a/src/Terminal.Gui.Editor/Editor.Commands.cs +++ b/src/Terminal.Gui.Editor/Editor.Commands.cs @@ -183,13 +183,7 @@ private void CreateCommandsAndBindings () return MultiCaretNewLine (); }); - AddCommand (Command.DeleteCharLeft, () => - { - var result = MultiCaretDeleteLeft (); - NotifyCompletionAfterInsert (); - - return result; - }); + AddCommand (Command.DeleteCharLeft, DeleteCharLeftAndRefresh); AddCommand (Command.DeleteCharRight, () => { DismissCompletion (); diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index ad21f9a..f13ea9f 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -125,43 +125,18 @@ internal bool HandleCompletionKey (Key key) return true; } - // Character keys: insert directly into the document while the popup is open, - // then refresh the completion list. + // Printable keys and Backspace edit through the same canonical path the + // editor uses without the popup open (multi-caret / selection / overwrite + // aware), then re-filter. The popup is focused, so these never reach + // OnKeyDownNotHandled on their own — they must be handled here. if (key is { IsCtrl: false, IsAlt: false, AsRune: { } rune } && !Rune.IsControl (rune)) { - if (_document is null || ReadOnly) - { - return true; - } - - if (HasSelection) - { - ReplaceSelection (rune.ToString ()); - } - else if (OverwriteMode) - { - OverwriteAtCaret (rune.ToString ()); - } - else - { - _document.Insert (CaretOffset, rune.ToString ()); - } - - // Refresh the completion list with the updated prefix. - NotifyCompletionAfterInsert (); - - return true; + return InsertTypedText (rune.ToString ()); } - // Backspace: delete the character before the caret and refresh. - if (key == Key.Backspace) + if (KeyMatches (Command.DeleteCharLeft)) { - if (_document is not null && !ReadOnly && CaretOffset > 0) - { - _document.Remove (CaretOffset - 1, 1); - } - - NotifyCompletionAfterInsert (); + DeleteCharLeftAndRefresh (); return true; } diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 9a78503..6145f11 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -67,6 +67,18 @@ protected override bool OnKeyDownNotHandled (Key key) return false; } + return InsertTypedText (rune.ToString ()); + } + + /// + /// The single canonical "type text at the caret" path: honors read-only, + /// multi-caret, selection, and overwrite mode, then refreshes the completion + /// popup. Shared by and the completion popup + /// key handler so the two never drift — the completion path previously skipped + /// the multi-caret branch and inserted at a single caret only. + /// + private bool InsertTypedText (string text) + { if (ReadOnly) { return true; @@ -74,27 +86,41 @@ protected override bool OnKeyDownNotHandled (Key key) if (HasMultipleCarets) { - MultiCaretInsert (rune.ToString ()); + MultiCaretInsert (text); return true; } if (HasSelection) { - ReplaceSelection (rune.ToString ()); + ReplaceSelection (text); } else if (OverwriteMode && _document is not null) { - OverwriteAtCaret (rune.ToString ()); + OverwriteAtCaret (text); } else { - _document!.Insert (CaretOffset, rune.ToString ()); + _document!.Insert (CaretOffset, text); } - // After inserting a character, notify the completion system so it can open / filter. + // After inserting, notify the completion system so it can open / filter. NotifyCompletionAfterInsert (); return true; } + + /// + /// Canonical delete-left: deletes the selection or the grapheme before the + /// caret(s) (multi-caret aware) and refreshes completion. Shared by the + /// binding and the completion popup key + /// handler so Backspace behaves identically with or without the popup open. + /// + private bool? DeleteCharLeftAndRefresh () + { + var result = MultiCaretDeleteLeft (); + NotifyCompletionAfterInsert (); + + return result; + } } diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index f5a3903..33b6bab 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -179,6 +179,52 @@ public void HandleCompletionKey_Handles_Regular_Chars_While_Active () Assert.True (editor.IsCompletionActive, "Completion should still be active after 'usi' (matches 'using')"); } + // Regression for #4: the popup key handler used to insert at a single caret only, + // bypassing multi-caret. It now routes through the canonical InsertTypedText, so a + // typed char while the popup is open is applied at every caret. + [Fact] + public void Typing_While_Completion_Active_And_MultiCaret_Inserts_At_All_Carets () + { + Editor editor = new () + { + Document = new TextDocument ("us\nus"), + CompletionProvider = new MultiWordCompletionProvider ("using", "unsafe") + }; + editor.CaretOffset = 2; // end of first "us" + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + editor.ToggleCaretAt (5); // additional caret at end of second "us" + Assert.True (editor.HasMultipleCarets); + + Assert.True (editor.HandleCompletionKey (new Key ('i'))); + Assert.Equal ("usi\nusi", editor.Document!.Text); + } + + // Regression for #4: the popup key handler used to delete a single char at the + // primary caret only. Backspace now routes through the canonical DeleteCharLeft, + // so it deletes before every caret. + [Fact] + public void Backspace_While_Completion_Active_And_MultiCaret_Deletes_At_All_Carets () + { + Editor editor = new () + { + Document = new TextDocument ("ab\nab"), + CompletionProvider = new StubCompletionProvider ("abc") + }; + editor.CaretOffset = 2; // end of first "ab" + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + editor.ToggleCaretAt (5); // additional caret at end of second "ab" + Assert.True (editor.HasMultipleCarets); + + Assert.True (editor.HandleCompletionKey (Key.Backspace)); + Assert.Equal ("a\na", editor.Document!.Text); + } + // Up/Down navigation and end-of-list cycling are covered below. Still untested: // PageUp/PageDown/Home/End within the list, and mouse selection. From dd6553ce7c42cc5530ab27fd487308876cd67c92 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:47:15 -0500 Subject: [PATCH 24/32] Measure completion popup width in display columns, not char count ShowCompletionPopup sized the ListView by Label.Length, so wide/CJK graphemes (2 cells each) produced a too-narrow popup that truncated labels. Use string.GetColumns() (the codebase's cell-measurement idiom), guarded with Math.Max(0, ...) like the rest of the rendering code. Adds a regression test asserting a 4-char / 8-column CJK label widens the popup to >= 8 (char-count math yielded ~6). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Terminal.Gui.Editor/Editor.Completion.cs | 4 ++- .../EditorCompletionTests.cs | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index f13ea9f..5c90c25 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -4,6 +4,7 @@ using Terminal.Gui.App; using Terminal.Gui.Editor.Completion; using Terminal.Gui.Input; +using Terminal.Gui.Text; using Terminal.Gui.ViewBase; using Terminal.Gui.Views; @@ -373,7 +374,8 @@ private void ShowCompletionPopup () _completionListView = new ListView { Source = new ListWrapper (labels), - Width = _completionItems.Max (i => i.Label.Length) + 2, + // Width in display columns, not char count — wide/CJK graphemes are 2 cells. + Width = _completionItems.Max (i => Math.Max (0, i.Label.GetColumns ())) + 2, Height = visibleCount, TabStop = TabBehavior.NoStop }; diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 33b6bab..6dcccfb 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -619,6 +619,39 @@ public void Click_Outside_Popover_Dismisses_And_Inserts_Nothing () Assert.Equal (before, editor.Document!.Text); } + // Regression for #6: popup width is measured in display columns, not char count. + // "你好世界" is 4 chars but 8 cells (East-Asian-wide). The buggy Label.Length math + // sized the popup to ~6; column-aware sizing must be >= 8. + [Fact] + public void Popup_Width_Accounts_For_Wide_Characters () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + Editor editor = new () + { + Width = Dim.Fill (), + Height = Dim.Fill (), + Document = new TextDocument ("你"), + CompletionProvider = new StubCompletionProvider ("你好世界") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 1; // after "你" — prefix is "你" + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + app.LayoutAndDraw (true); + var popover = (View)app.Popovers!.GetActivePopover ()!; + + // 4 wide chars = 8 display columns. Char-count math would yield ~6 (< 8). + Assert.True ( + popover.Frame.Width >= 8, + $"Popup width {popover.Frame.Width} should be >= the 8 display columns of \"你好世界\""); + } + /// Stub provider that always returns a single hard-coded item. private sealed class StubCompletionProvider (string word) : IEditorCompletionProvider { From 600fe0cd04bedbed7d5fa23823a666067c8d31d9 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:50:16 -0500 Subject: [PATCH 25/32] Dedup ShowCompletion / NotifyCompletionAfterInsert The two methods shared a ~12-line body (query provider, dismiss-if-empty, capture state, show popup). Extract QueryAndShowCompletion; each caller keeps only its guard and its one intentional difference: filter-as-you-type (NotifyCompletionAfterInsert) closes on an empty prefix without querying, while explicit ShowCompletion queries even on an empty prefix so a provider can offer a full list. Adds two tests pinning that asymmetry so a future merge can't silently collapse it. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Terminal.Gui.Editor/Editor.Completion.cs | 33 ++++++------ .../EditorCompletionTests.cs | 51 +++++++++++++++++++ 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 5c90c25..6af363a 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -221,6 +221,9 @@ internal void NotifyCompletionAfterInsert () var prefix = GetCompletionPrefix (out var prefixStart); + // Filter-as-you-type: an empty prefix means the caret moved off the word + // (e.g. Backspace past it), so close instead of re-querying for everything. + // This is the one intentional difference from ShowCompletion. if (prefix.Length == 0) { DismissCompletion (); @@ -228,20 +231,7 @@ internal void NotifyCompletionAfterInsert () return; } - IReadOnlyList items = - CompletionProvider.GetCompletions (_document!, CaretOffset, prefix); - - if (items.Count == 0) - { - DismissCompletion (); - - return; - } - - _completionPrefixStart = prefixStart; - _completionItems = items; - CompletionSelectedIndex = 0; - ShowCompletionPopup (); + QueryAndShowCompletion (prefix, prefixStart); } /// Opens the completion popup, querying the provider for items. @@ -252,10 +242,23 @@ internal void ShowCompletion () return; } + // Explicit trigger (e.g. Ctrl+Space): query even on an empty prefix so a + // provider can offer a full list — unlike the filter-as-you-type path. var prefix = GetCompletionPrefix (out var prefixStart); + QueryAndShowCompletion (prefix, prefixStart); + } + + /// + /// Shared body of and + /// : query the provider, dismiss if it returns + /// nothing, otherwise capture state and (re)show the popup. Callers guard + /// / first. + /// + private void QueryAndShowCompletion (string prefix, int prefixStart) + { IReadOnlyList items = - CompletionProvider.GetCompletions (_document, CaretOffset, prefix); + CompletionProvider!.GetCompletions (_document!, CaretOffset, prefix); if (items.Count == 0) { diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 6dcccfb..91f1a37 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -652,6 +652,43 @@ public void Popup_Width_Accounts_For_Wide_Characters () $"Popup width {popover.Frame.Width} should be >= the 8 display columns of \"你好世界\""); } + // #10: ShowCompletion and NotifyCompletionAfterInsert share a body but must keep one + // intentional asymmetry — explicit ShowCompletion queries the provider even on an + // empty prefix (provider may offer a full list); filter-as-you-type + // NotifyCompletionAfterInsert closes on empty prefix without querying. These two + // tests prevent a future "simplification" from collapsing that difference. + [Fact] + public void ShowCompletion_With_Empty_Prefix_Still_Queries_Provider () + { + Editor editor = new () + { + Document = new TextDocument ("x "), + CompletionProvider = new AlwaysCompletionProvider () + }; + editor.CaretOffset = 2; // after the space → empty prefix + + Assert.Equal (string.Empty, editor.GetCompletionPrefix ()); + + editor.ShowCompletion (); + + Assert.True (editor.IsCompletionActive); + } + + [Fact] + public void NotifyCompletionAfterInsert_With_Empty_Prefix_Dismisses () + { + Editor editor = new () + { + Document = new TextDocument ("x "), + CompletionProvider = new AlwaysCompletionProvider () + }; + editor.CaretOffset = 2; // after the space → empty prefix + + editor.NotifyCompletionAfterInsert (); + + Assert.False (editor.IsCompletionActive); + } + /// Stub provider that always returns a single hard-coded item. private sealed class StubCompletionProvider (string word) : IEditorCompletionProvider { @@ -687,6 +724,20 @@ public bool ShouldTrigger (Key key) } } + /// Provider that returns one item regardless of prefix (including empty). + private sealed class AlwaysCompletionProvider : IEditorCompletionProvider + { + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + return [new CompletionItem { Label = "always" }]; + } + + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } + } + /// Provider that returns all words starting with the given prefix. private sealed class MultiWordCompletionProvider (params string[] words) : IEditorCompletionProvider { From b93edaf14474e8d2b6f2a4de26c9863f50f2adb0 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:57:36 -0500 Subject: [PATCH 26/32] Remove unwired CompletionItem.Detail; test the real InsertText path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detail was declared and documented as "shown alongside the label" but is read nowhere — the popup renders Label only. Speculative/unused public API is removed rather than shelved-and-documented. Dropped from CompletionItem and the API specs (completion spec, public-api, DEC-009 rationale). InsertText is genuinely used (AcceptCompletion -> TextToInsert => InsertText ?? Label) so it stays; added a regression test for the InsertText-differs-from- Label path, which nothing exercised. Co-Authored-By: Claude Opus 4.7 (1M context) --- specs/completion/spec.md | 1 - specs/decisions.md | 2 +- specs/public-api.md | 1 - .../Completion/CompletionItem.cs | 10 +---- .../EditorCompletionTests.cs | 40 +++++++++++++++++++ 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/specs/completion/spec.md b/specs/completion/spec.md index 955631b..ff4f7f8 100644 --- a/specs/completion/spec.md +++ b/specs/completion/spec.md @@ -22,7 +22,6 @@ public sealed class CompletionItem { public required string Label { get; init; } public string? InsertText { get; init; } // defaults to Label - public string? Detail { get; init; } } public interface IEditorCompletionProvider diff --git a/specs/decisions.md b/specs/decisions.md index 4999b66..23c56a8 100644 --- a/specs/decisions.md +++ b/specs/decisions.md @@ -74,7 +74,7 @@ need to know the document construction details. **Decision**: Use a fresh LSP-flavored `IEditorCompletionProvider` interface and `CompletionItem` type (`Terminal.Gui.Editor.Completion` namespace), **not** Terminal.Gui's existing `IAutocomplete` / `PopupAutocomplete`. -**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows the LSP shape (Label, InsertText, Detail) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration. The popup uses a `Popover` positioned at the caret. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. +**Rationale**: TG's `IAutocomplete` is tightly coupled to `TextView` (it assumes its own `PopupAutocomplete` rendering, owns selection state, and embeds key-handling that conflicts with `Editor`'s command architecture). A clean provider interface — `GetCompletions(document, caretOffset, prefix)` + `ShouldTrigger(key)` — keeps the completion *data* separate from the *UI*. `CompletionItem` follows a minimal LSP-flavored shape (Label, InsertText) rather than reusing `IAutocomplete`'s string list, which simplifies future LSP integration; richer fields (e.g. detail/documentation) are added when the popup actually renders them, not before. The popup uses a `Popover` positioned at the caret. Accept applies inside a single `RunUpdate` scope so the entire replacement is one undo step. **Date**: 2026-05-17 diff --git a/specs/public-api.md b/specs/public-api.md index d536883..286d4ac 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -127,7 +127,6 @@ public sealed class CompletionItem { public required string Label { get; init; } public string? InsertText { get; init; } - public string? Detail { get; init; } } public interface IEditorCompletionProvider diff --git a/src/Terminal.Gui.Editor/Completion/CompletionItem.cs b/src/Terminal.Gui.Editor/Completion/CompletionItem.cs index d97c01c..a0c586c 100644 --- a/src/Terminal.Gui.Editor/Completion/CompletionItem.cs +++ b/src/Terminal.Gui.Editor/Completion/CompletionItem.cs @@ -2,8 +2,8 @@ namespace Terminal.Gui.Editor.Completion; /// /// A single completion suggestion returned by an . -/// Modelled after the LSP CompletionItem shape — label + insertText + optional detail — -/// but kept minimal for the terminal context. +/// Modelled after the LSP CompletionItem shape — label + insertText — but kept +/// minimal for the terminal context. /// public sealed class CompletionItem { @@ -19,12 +19,6 @@ public sealed class CompletionItem /// public string? InsertText { get; init; } - /// - /// An optional secondary description shown alongside the label (e.g. a type signature or - /// source module). - /// - public string? Detail { get; init; } - /// The text that will actually be inserted: ?? . internal string TextToInsert => InsertText ?? Label; } diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 91f1a37..a73e278 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -689,6 +689,46 @@ public void NotifyCompletionAfterInsert_With_Empty_Prefix_Dismisses () Assert.False (editor.IsCompletionActive); } + // #13a: InsertText is real (AcceptCompletion inserts TextToInsert => InsertText ?? + // Label) but no test exercised the InsertText-differs-from-Label path. A descriptive + // label must not be what lands in the document. + [Fact] + public void AcceptCompletion_Inserts_InsertText_Not_Label_When_They_Differ () + { + Editor editor = new () + { + Document = new TextDocument ("Wr"), + CompletionProvider = new DistinctInsertTextProvider () + }; + editor.CaretOffset = 2; // after "Wr" + + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + + editor.AcceptCompletion (); + + Assert.Equal ("WriteLine", editor.Document!.Text); + } + + /// Provider whose single item has a descriptive Label distinct from InsertText. + private sealed class DistinctInsertTextProvider : IEditorCompletionProvider + { + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + if (string.IsNullOrEmpty (prefix)) + { + return []; + } + + return [new CompletionItem { Label = "WriteLine — write a line", InsertText = "WriteLine" }]; + } + + public bool ShouldTrigger (Key key) + { + return key == Key.Space.WithCtrl; + } + } + /// Stub provider that always returns a single hard-coded item. private sealed class StubCompletionProvider (string word) : IEditorCompletionProvider { From 77fbae14e16c156c9893dc5bcc59311d8e6ba2cd Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 18:59:53 -0500 Subject: [PATCH 27/32] Codify the no-speculative-API rule in CLAUDE.md Non-goals Reviewing #13 surfaced that 'don't ship unwired public API; delete it rather than shelve+document' was applied but not written down. Add it to Non-goals so it's enforced by the reviewer, not folklore. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 041e452..2792933 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -234,6 +234,7 @@ Don't accidentally do these — they were considered and rejected: - RTL bidi or rich text shaping beyond grapheme width. - Pixel/proportional font fidelity. - Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `Popover` for completion). +- Speculative / unwired public API "for the future" (LSP-shape parity, forward-compat, etc.). Add a member when the code that consumes it lands, not before. Unwired public surface is **deleted**, not shelved-and-documented — hypothetical APIs lie to consumers and accrete compat cost for value that may never arrive. (Distinguish from genuinely-used-but-untested API: that stays and gets a test.) ## Open decisions From 9277acaf6fe7c258718be31a635840a220b20c75 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 19:03:24 -0500 Subject: [PATCH 28/32] Revert CLAUDE.md no-speculative-API bullet; constitution R9 already covers it specs/constitution.md R9 (No unused public/internal APIs in src/) is the authoritative, reviewer-citable rule and is stronger ("tests don't count as a consumer"). Duplicating it in CLAUDE.md Non-goals invites drift; the constitution is the highest-authority doc. Reverts 77fbae1. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2792933..041e452 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -234,7 +234,6 @@ Don't accidentally do these — they were considered and rejected: - RTL bidi or rich text shaping beyond grapheme width. - Pixel/proportional font fidelity. - Porting AvaloniaEdit's `Editing/`, `Rendering/`, or `CodeCompletion/` namespaces — those are Avalonia-UI-specific and replaced by TG-native equivalents (`Editor` partials, cell-grid `Rendering/`, `Popover` for completion). -- Speculative / unwired public API "for the future" (LSP-shape parity, forward-compat, etc.). Add a member when the code that consumes it lands, not before. Unwired public surface is **deleted**, not shelved-and-documented — hypothetical APIs lie to consumers and accrete compat cost for value that may never arrive. (Distinguish from genuinely-used-but-untested API: that stays and gets a test.) ## Open decisions From 19980c7f9c6aeaba6e7c69a409f7fe2fbbdce205 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 19:05:06 -0500 Subject: [PATCH 29/32] Refresh completion popup in place instead of rebuilding every keystroke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ShowCompletionPopup disposed the Popover + ListView, recreated both, and re-subscribed three event handlers on every filter keystroke — visible flicker and needless allocation/churn. When a popover is already live, swap the ListView Source/dims/selection and reposition instead; only the first show builds and wires events. ResultExtractor now references the live _completionItems (not a captured snapshot) so in-place refresh stays correct. Adds a test asserting the active popover is the same instance across a re-filter and that the list actually narrows. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Terminal.Gui.Editor/Editor.Completion.cs | 39 ++++++++++++------ .../EditorCompletionTests.cs | 40 +++++++++++++++++++ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index 6af363a..ea83e1f 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -364,21 +364,35 @@ private void ShowCompletionPopup () return; } - // Drop the previous popover (if any) — a fresh one is built each time so the - // list rebuilds. Reentrant-safe so the VisibleChanged teardown can't NRE here. - DisposeCompletionPopover (); - - // Build the label list for the ListView. ObservableCollection labels = new (_completionItems.Select (i => i.Label)); // Cap visible height at 10 items to avoid oversized popups. var visibleCount = Math.Min (_completionItems.Count, 10); + // Width in display columns, not char count — wide/CJK graphemes are 2 cells. + var width = _completionItems.Max (i => Math.Max (0, i.Label.GetColumns ())) + 2; + + // Filter-as-you-type refresh: reuse the live popover/ListView rather than + // disposing and rebuilding (Popover + ListView + 3 event subscriptions) on + // every keystroke — that flickered and churned allocations. + if (_completionListView is not null && _completionPopover is not null) + { + _completionListView.Source = new ListWrapper (labels); + _completionListView.Width = width; + _completionListView.Height = visibleCount; + _completionListView.SelectedItem = CompletionSelectedIndex; + + Point caret = GetCaretScreenPosition (); + _completionPopover.MakeVisible (new Point (caret.X, caret.Y + 1)); + + return; + } + + // First show — build the ListView + Popover and wire events once. _completionListView = new ListView { Source = new ListWrapper (labels), - // Width in display columns, not char count — wide/CJK graphemes are 2 cells. - Width = _completionItems.Max (i => Math.Max (0, i.Label.GetColumns ())) + 2, + Width = width, Height = visibleCount, TabStop = TabBehavior.NoStop }; @@ -391,19 +405,20 @@ private void ShowCompletionPopup () _completionListView.MouseBindings.ReplaceCommands (MouseFlags.LeftButtonClicked, Command.Accept); _completionListView.SelectedItem = CompletionSelectedIndex; - IReadOnlyList capturedItems = _completionItems; - _completionPopover = new Popover (_completionListView) { Target = new WeakReference (this), + + // Reference the live _completionItems (not a captured snapshot) so the + // in-place refresh above stays consistent. ResultExtractor = lv => { - if (lv.SelectedItem is not { } idx || idx < 0 || idx >= capturedItems.Count) + if (lv.SelectedItem is not { } idx || idx < 0 || idx >= _completionItems.Count) { return null; } - return capturedItems[idx]; + return _completionItems[idx]; }, TabStop = TabBehavior.NoStop }; @@ -417,7 +432,7 @@ private void ShowCompletionPopup () _completionPopover.MakeVisible (new Point (caretScreen.X, caretScreen.Y + 1)); // The focused ListView's Up/Down move its selection; mirror that into - // _completionSelectedIndex so AcceptCompletion inserts the right item. + // CompletionSelectedIndex so AcceptCompletion inserts the right item. // Accept/dismiss keys are resolved separately in HandleCompletionKey. _completionListView.ValueChanged += (_, args) => { diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index a73e278..1ff5302 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -710,6 +710,46 @@ public void AcceptCompletion_Inserts_InsertText_Not_Label_When_They_Differ () Assert.Equal ("WriteLine", editor.Document!.Text); } + // #9b: filter-as-you-type must reuse the live popover, not dispose+rebuild it on + // every keystroke (flicker/churn). The active popover must be the SAME instance + // after a re-filter, and the list must actually narrow. + [Fact] + public void Filter_Refresh_Reuses_Same_Popover_Instance () + { + using IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + Runnable top = new (); + + Editor editor = new () + { + Width = Dim.Fill (), + Height = Dim.Fill (), + Document = new TextDocument ("u"), + CompletionProvider = new MultiWordCompletionProvider ("using", "unsafe", "uint") + }; + top.Add (editor); + app.Begin (top); + + editor.CaretOffset = 1; + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + app.LayoutAndDraw (true); + View popover1 = (View)app.Popovers!.GetActivePopover ()!; + + // Type 's' → re-filter "u" → "us". Must reuse the popover, not rebuild it. + editor.Document!.Insert (editor.CaretOffset, "s"); + editor.CaretOffset = 2; + editor.NotifyCompletionAfterInsert (); + Assert.True (editor.IsCompletionActive); + View popover2 = (View)app.Popovers!.GetActivePopover ()!; + + Assert.Same (popover1, popover2); + + // The list actually re-filtered: "us" matches using/unsafe, not uint. + editor.AcceptCompletion (); + Assert.Contains (editor.Document!.Text, new[] { "using", "unsafe" }); + } + /// Provider whose single item has a descriptive Label distinct from InsertText. private sealed class DistinctInsertTextProvider : IEditorCompletionProvider { From 6f7922c297c9bc921f04cc71b16204efe3820555 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 19:07:23 -0500 Subject: [PATCH 30/32] Strengthen filter/dismiss tests to drive the real key path Typing_Characters_While_Completion_Active_Filters_List and Typing_NonMatching_Char_While_Completion_Active_Dismisses simulated typing via a hand-rolled Document.Insert + NotifyCompletionAfterInsert, so they tested the helper, not the HandleCompletionKey -> InsertTypedText wiring they claim to cover (false confidence). Drive them through HandleCompletionKey instead, and assert the resulting document text. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../EditorCompletionTests.cs | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs index 1ff5302..91411be 100644 --- a/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs +++ b/tests/Terminal.Gui.Editor.Tests/EditorCompletionTests.cs @@ -481,8 +481,9 @@ public void Setting_CompletionProvider_To_Null_Dismisses_Active_Session () [Fact] public void Typing_Characters_While_Completion_Active_Filters_List () { - // Simulates: document has "u", popup shows [using, unsafe, uint]. - // User types "s" → document becomes "us", popup re-filters to [using, unsafe]. + // Popup is open on prefix "u"; the user types "s". Driven through the real key + // path (HandleCompletionKey -> InsertTypedText -> insert + re-filter) rather + // than a hand-rolled Document.Insert, so it catches a break in that wiring. Editor editor = new () { Document = new TextDocument ("u"), @@ -490,20 +491,14 @@ public void Typing_Characters_While_Completion_Active_Filters_List () }; editor.CaretOffset = 1; - // Open completion. editor.NotifyCompletionAfterInsert (); Assert.True (editor.IsCompletionActive); - // Simulate the Editor inserting "s" (as OnKeyDownNotHandled would). - editor.Document!.Insert (editor.CaretOffset, "s"); - editor.CaretOffset = 2; - - // Re-filter via NotifyCompletionAfterInsert. - editor.NotifyCompletionAfterInsert (); + Assert.True (editor.HandleCompletionKey (new Key ('s'))); + Assert.Equal ("us", editor.Document!.Text); Assert.True (editor.IsCompletionActive); - // "us" prefix should match "using" and "unsafe" but not "uint". - // Verify by accepting — the accepted text should be one of the "us" matches. + // "us" matches "using" / "unsafe" but not "uint" — confirm via accept. editor.AcceptCompletion (); Assert.Contains (editor.Document!.Text, new[] { "using", "unsafe" }); } @@ -511,8 +506,8 @@ public void Typing_Characters_While_Completion_Active_Filters_List () [Fact] public void Typing_NonMatching_Char_While_Completion_Active_Dismisses () { - // Simulates: document has "x", popup shows items for "x". - // User types "z" → document becomes "xz", no matches → popup dismissed. + // Popup open on "us"; typing "z" makes "usz", which matches nothing → dismiss. + // Driven through the real key path, not a hand-rolled Document.Insert. Editor editor = new () { Document = new TextDocument ("us"), @@ -523,11 +518,8 @@ public void Typing_NonMatching_Char_While_Completion_Active_Dismisses () editor.NotifyCompletionAfterInsert (); Assert.True (editor.IsCompletionActive); - // Simulate inserting a character that breaks all matches. - editor.Document!.Insert (editor.CaretOffset, "z"); - editor.CaretOffset = 3; - - editor.NotifyCompletionAfterInsert (); + Assert.True (editor.HandleCompletionKey (new Key ('z'))); + Assert.Equal ("usz", editor.Document!.Text); Assert.False (editor.IsCompletionActive); } From affe0fafb76a38f67c72e9266d0729da1b7b3fc3 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 18 May 2026 19:34:25 -0500 Subject: [PATCH 31/32] Fix flaky OpenFileAsync background-thread test (intrinsically racy thread-id assert) OpenFileAsync_Loads_Stream_On_Background_Thread asserted Assert.NotEqual(callerThreadId, readThreadId) via Environment.CurrentManagedThreadId. Managed thread IDs are recycled and continuations can inline, so under a loaded parallel CI runner the read thread occasionally reused the finished caller's id -> false failure (seen on macos-latest CI, same SHA passed on the PR-event run; also flaked repeatedly in local parallel runs this session). Not a regression from this PR. The async/non-blocking guarantee is already proven deterministically by the existing gate assertions (ReadStarted fired, openTask not completed while gated, spinner visible). Remove the redundant racy assertion plus the now-dead testThreadId local and GatedReadStream.ReadThreadId plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs index b9d2747..1071c76 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/TedAppTests.cs @@ -126,7 +126,6 @@ public async Task OpenFileAsync_Updates_LoadStatusShortcut () public async Task OpenFileAsync_Loads_Stream_On_Background_Thread () { TedApp app = new (configPath: TedTestConfig.NewPath ()); - var testThreadId = Environment.CurrentManagedThreadId; GatedReadStream stream = new (Encoding.UTF8.GetBytes (new string ('x', 100_000))); app.ShowOpenDialog = () => "/tmp/ted-progress.txt"; app.OpenRead = _ => stream; @@ -144,7 +143,6 @@ public async Task OpenFileAsync_Loads_Stream_On_Background_Thread () stream.AllowRead.SetResult (); Assert.True (await openTask); - Assert.NotEqual (testThreadId, stream.ReadThreadId); Assert.Equal ("Loaded 97.7 KiB", app.LoadSpinnerShortcut.Title); Assert.Equal ("Loaded 97.7 KiB", app.LoadSpinnerShortcut.HelpText); } @@ -865,7 +863,7 @@ public override ValueTask WriteAsync (ReadOnlyMemory buffer, CancellationT } } - /// Gates async reads and captures the reading thread ID for background-load tests. + /// Gates async reads so background-load tests can observe the in-flight state. private sealed class GatedReadStream : MemoryStream { public GatedReadStream (byte[] buffer) @@ -877,11 +875,8 @@ public GatedReadStream (byte[] buffer) public TaskCompletionSource ReadStarted { get; } = new (TaskCreationOptions.RunContinuationsAsynchronously); - public int ReadThreadId { get; private set; } - public override ValueTask ReadAsync (Memory buffer, CancellationToken cancellationToken = default) { - ReadThreadId = Environment.CurrentManagedThreadId; ReadStarted.TrySetResult (); return new ValueTask (ReadAfterGateAsync (buffer, cancellationToken)); From 3de0c57ad4dd1040bf9889b406f19046d0665582 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 19 May 2026 00:58:54 +0000 Subject: [PATCH 32/32] fix: completion-consumed keys break kill-ring consecutive run; dismiss on provider swap; fix spec/changelog - Added failing test CompletionConsumedKey_BreaksConsecutiveKillRun - Fixed: HandleCompletionKey returning true now clears _lastCommandWasKill - Fixed: CompletionProvider setter dismisses stale session on provider swap (not just null) - Fixed: spec.md incorrectly said Enter dismisses (it accepts) - Fixed: public-api.md changelog said "record" but CompletionItem is a sealed class Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/813ac8d3-83ad-45c9-b1f8-f0efa109ae9e Co-authored-by: tig <585482+tig@users.noreply.github.com> --- specs/completion/spec.md | 3 +- specs/public-api.md | 2 +- src/Terminal.Gui.Editor/Editor.Completion.cs | 7 +++ src/Terminal.Gui.Editor/Editor.Keyboard.cs | 3 ++ .../EditorKillRingTests.cs | 54 +++++++++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-) diff --git a/specs/completion/spec.md b/specs/completion/spec.md index ff4f7f8..6060517 100644 --- a/specs/completion/spec.md +++ b/specs/completion/spec.md @@ -63,8 +63,9 @@ inside a single `Document.RunUpdate()` scope, so one `Ctrl+Z` undoes the entire ### Dismissing -Pressing `Esc`, pressing `Enter` on NewLine command, typing a non-word character that empties the prefix, +Pressing `Esc`, typing a non-word character that empties the prefix, or the provider returning zero items — all dismiss the popup. +Pressing `Enter`/`Tab` accepts the selected item (see "Accepting" above), not dismiss. ## Positioning diff --git a/specs/public-api.md b/specs/public-api.md index 286d4ac..91a1581 100644 --- a/specs/public-api.md +++ b/specs/public-api.md @@ -176,6 +176,6 @@ public readonly record struct TextDocumentProgress ( | 2026-05-11 | ReadOnly property landed on Editor | read-only | | 2026-05-12 | `ISearchStrategy?` `SearchStrategy { get; set; }` landed on Editor; string-based FindNext/FindPrevious/ReplaceNext/ReplaceAll overloads retained as convenience wrappers | find-and-replace | | 2026-05-16 | Vertical multi-caret keybindings (`Ctrl+Alt+CursorUp/Down`, `Alt+Drag`) added via `Editor.DefaultKeyBindings`; no new public Editor API (R8) | vertical-multi-caret | -| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` record; `Popover`-based popup; DEC-009 resolves OPEN-002 | completion | +| 2026-05-17 | `IEditorCompletionProvider?` `CompletionProvider` + `bool IsCompletionActive` landed; `CompletionItem` sealed class; `Popover`-based popup; DEC-009 resolves OPEN-002 | completion | | 2026-05-17 | Streaming `TextDocument.LoadAsync` / `TextDocument.SaveAsync`, `TextDocumentProgress`, `TextDocument.Encoding`, and delegating `Editor.LoadAsync` / `Editor.SaveAsync` landed | file-io | | 2026-05-17 | `Editor` implements `IDesignable`; `EnableForDesign()` seeds C# sample code with syntax highlighting and line numbers | design-time | diff --git a/src/Terminal.Gui.Editor/Editor.Completion.cs b/src/Terminal.Gui.Editor/Editor.Completion.cs index ea83e1f..86ba126 100644 --- a/src/Terminal.Gui.Editor/Editor.Completion.cs +++ b/src/Terminal.Gui.Editor/Editor.Completion.cs @@ -31,6 +31,13 @@ public IEditorCompletionProvider? CompletionProvider return; } + // Dismiss stale completion when the provider changes — prevents accepting + // suggestions from the previous provider after a swap. + if (IsCompletionActive) + { + DismissCompletion (); + } + field = value; if (value is null) diff --git a/src/Terminal.Gui.Editor/Editor.Keyboard.cs b/src/Terminal.Gui.Editor/Editor.Keyboard.cs index 6145f11..5071ce2 100644 --- a/src/Terminal.Gui.Editor/Editor.Keyboard.cs +++ b/src/Terminal.Gui.Editor/Editor.Keyboard.cs @@ -20,6 +20,9 @@ protected override bool OnKeyDown (Key key) { if (HandleCompletionKey (key)) { + // A completion-consumed key is not a kill command — break any consecutive-kill run. + _lastCommandWasKill = false; + return true; } diff --git a/tests/Terminal.Gui.Editor.IntegrationTests/EditorKillRingTests.cs b/tests/Terminal.Gui.Editor.IntegrationTests/EditorKillRingTests.cs index 15da153..8b7bb03 100644 --- a/tests/Terminal.Gui.Editor.IntegrationTests/EditorKillRingTests.cs +++ b/tests/Terminal.Gui.Editor.IntegrationTests/EditorKillRingTests.cs @@ -1,6 +1,8 @@ // Copilot - gpt-4.1 +using Terminal.Gui.Document; using Terminal.Gui.Drivers; +using Terminal.Gui.Editor.Completion; using Terminal.Gui.Editor.IntegrationTests.Testing; using Terminal.Gui.Input; using Terminal.Gui.Testing; @@ -301,4 +303,56 @@ public async Task CutToStartOfLine_PreservesText_When_Clipboard_Unavailable () Assert.Equal ("hello world", fx.Top.Editor.Document?.Text); } + + // ───────────────────── Completion breaks kill run ───────────────────── + + [Fact] + public async Task CompletionConsumedKey_BreaksConsecutiveKillRun () + { + // When HandleCompletionKey consumes a key (returns true), it must clear _lastCommandWasKill + // so the next kill command (via InvokeCommand) does NOT append to clipboard. + await using AppFixture fx = new (() => new ("abc\ndef")); + EnsureFakeClipboard (fx); + Editor editor = fx.Top.Editor; + editor.SetFocus (); + editor.CompletionProvider = new KillRingTestCompletionProvider (); + editor.CaretOffset = 0; + + // First CutToEndOfLine via InvokeCommand: "abc" cut, clipboard = "abc" + editor.InvokeCommand (Command.CutToEndOfLine); + Assert.Equal ("\ndef", editor.Document?.Text); + + string? data = null; + Assert.True (fx.App.Clipboard?.TryGetClipboardData (out data)); + Assert.Equal ("abc", data); + + // Ctrl+Space goes through OnKeyDown → HandleCompletionKey → returns true (ShouldTrigger). + // This should break the consecutive-kill run. + fx.Injector.InjectKey (Key.Space.WithCtrl, new InputInjectionOptions { Mode = InputInjectionMode.Direct }); + + // Dismiss completion programmatically (not via keyboard). + editor.CompletionProvider = null; + editor.CompletionProvider = new KillRingTestCompletionProvider (); + + // Second CutToEndOfLine — caret at 0, text = "\ndef", cuts "\n". + editor.InvokeCommand (Command.CutToEndOfLine); + + Assert.True (fx.App.Clipboard?.TryGetClipboardData (out data)); + + // Bug: clipboard = "abc\n" (appended because _lastCommandWasKill was never cleared) + // Fix: clipboard = "\n" (replaced because Ctrl+Space broke the run) + Assert.Equal ("\n", data); + } + + /// Simple completion provider for kill-ring interaction tests. + private sealed class KillRingTestCompletionProvider : IEditorCompletionProvider + { + public IReadOnlyList GetCompletions (TextDocument document, int caretOffset, string prefix) + { + // Always return items so the popup stays open regardless of prefix. + return [new CompletionItem { Label = "ghi" }, new CompletionItem { Label = "ghijk" }]; + } + + public bool ShouldTrigger (Key key) => key == Key.Space.WithCtrl; + } }