From c316b31a44b2f8b7df879dd238aec316ff6b8457 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 22 May 2026 10:34:23 -0600 Subject: [PATCH 01/13] BREAKING CHANGE: Fixes #5366. Make View.Text notifications CWP-compliant Add signal-only CWP-compliant text-change workflow to View.Text: - Add protected virtual bool OnTextChanging() pre-change hook - Add public event EventHandler? TextChanging - Refactor OnTextChanged() from public void to protected virtual void (raises TextChanged event; subclasses can override) - Text setter now follows CWP flow: early-return on same value, OnTextChanging, TextChanging event, mutate, OnTextChanged - Canceling TextChanging prevents text mutation and suppresses TextChanged - Add 'new' keyword to TextField.TextChanging (intentional hiding with richer ResultEventArgs semantics) - Add comprehensive unit tests for CWP behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Text.cs | 89 +++++++++-- .../TextInput/TextField/TextField.Text.cs | 2 +- .../ViewBase/TextCwpTests.cs | 151 ++++++++++++++++++ 3 files changed, 230 insertions(+), 12 deletions(-) create mode 100644 Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index 5625c77ecd..ade1e0c3ed 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -1,14 +1,11 @@ +using System.ComponentModel; + namespace Terminal.Gui.ViewBase; public partial class View // Text Property APIs { private string _text = string.Empty; - /// - /// Called when the has changed. Fires the event. - /// - public void OnTextChanged () => TextChanged?.Invoke (this, EventArgs.Empty); - /// /// Gets or sets whether trailing spaces at the end of word-wrapped lines are preserved /// or not when is enabled. @@ -50,7 +47,17 @@ public bool PreserveTrailingSpaces /// If or are using , /// the will be adjusted to fit the text. /// - /// When the text changes, the is fired. + /// + /// Setting to the same value as the current value is a no-op; neither + /// nor will be raised. + /// + /// + /// Before the text is changed, the CWP hook is invoked. If cancelled, + /// the text remains unchanged and is not raised. + /// + /// + /// After the text is changed, the event is raised. + /// /// public virtual string Text { @@ -62,14 +69,79 @@ public virtual string Text return; } + if (OnTextChanging ()) + { + return; + } + + CancelEventArgs args = new (); + TextChanging?.Invoke (this, args); + + if (args.Cancel) + { + return; + } + _text = value; UpdateTextFormatterText (); SetNeedsLayout (); + OnTextChanged (); } } + /// + /// Called before the changes. Invokes the event, which can + /// be cancelled. + /// + /// + /// + /// This is a signal-only notification. It does not carry old or new text values because + /// semantics vary across derived views. + /// + /// + /// if the text change should be cancelled; otherwise . + protected virtual bool OnTextChanging () => false; + + /// + /// Raised when the is about to change. Set to + /// to prevent the change. + /// + /// + /// + /// This is a signal-only notification at the level. It does not carry old or new + /// text values. Derived controls that need richer text-edit semantics may expose their own specific events. + /// + /// + public event EventHandler? TextChanging; + + /// + /// Called after the has been changed. Raises the event. + /// + /// + /// + /// This is a signal-only notification. It does not carry old or new text values because + /// semantics vary across derived views. + /// + /// + /// Derived views that override and do not call base.Text should call + /// this method after mutating text to participate in the CWP workflow. + /// + /// + protected virtual void OnTextChanged () => TextChanged?.Invoke (this, EventArgs.Empty); + + /// + /// Raised after the has been changed. + /// + /// + /// + /// This is a signal-only notification at the level. It does not carry old or new + /// text values. Derived controls that need richer text-edit semantics may expose their own specific events. + /// + /// + public event EventHandler? TextChanged; + /// /// Gets or sets how the View's is aligned horizontally when drawn. Changing this property will /// redisplay the . @@ -92,11 +164,6 @@ public Alignment TextAlignment } } - /// - /// Text changed event, raised when the text has changed. - /// - public event EventHandler? TextChanged; - /// /// Gets or sets the direction of the View's . Changing this property will redisplay the /// . diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index b6fce8850d..619eaf3dfc 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -9,7 +9,7 @@ public partial class TextField private string? _lastPastedText; /// Raised before changes. The change can be canceled the text adjusted. - public event EventHandler>? TextChanging; + public new event EventHandler>? TextChanging; /// /// Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so diff --git a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs new file mode 100644 index 0000000000..e05a85bd2f --- /dev/null +++ b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs @@ -0,0 +1,151 @@ +// Copilot + +using System.ComponentModel; + +namespace ViewBaseTests; + +/// +/// Tests for the CWP-compliant notifications: +/// and . +/// +public class TextCwpTests +{ + [Fact] + public void Text_SetSameValue_NoEventsRaised () + { + View view = new () { Text = "hello" }; + bool changingRaised = false; + bool changedRaised = false; + view.TextChanging += (_, _) => changingRaised = true; + view.TextChanged += (_, _) => changedRaised = true; + + view.Text = "hello"; + + Assert.False (changingRaised); + Assert.False (changedRaised); + } + + [Fact] + public void Text_SetDifferentValue_BothEventsRaised () + { + View view = new () { Text = "old" }; + bool changingRaised = false; + bool changedRaised = false; + view.TextChanging += (_, _) => changingRaised = true; + view.TextChanged += (_, _) => changedRaised = true; + + view.Text = "new"; + + Assert.True (changingRaised); + Assert.True (changedRaised); + } + + [Fact] + public void TextChanging_Cancel_PreventsTextChange () + { + View view = new () { Text = "original" }; + view.TextChanging += (_, e) => e.Cancel = true; + + view.Text = "modified"; + + Assert.Equal ("original", view.Text); + } + + [Fact] + public void TextChanging_Cancel_SuppressesTextChanged () + { + View view = new () { Text = "original" }; + bool changedRaised = false; + view.TextChanging += (_, e) => e.Cancel = true; + view.TextChanged += (_, _) => changedRaised = true; + + view.Text = "modified"; + + Assert.False (changedRaised); + } + + [Fact] + public void TextChanging_RaisedBeforeMutation () + { + View view = new () { Text = "before" }; + string? textDuringChanging = null; + view.TextChanging += (sender, _) => textDuringChanging = ((View)sender!).Text; + + view.Text = "after"; + + Assert.Equal ("before", textDuringChanging); + } + + [Fact] + public void TextChanged_RaisedAfterMutation () + { + View view = new () { Text = "before" }; + string? textDuringChanged = null; + view.TextChanged += (sender, _) => textDuringChanged = ((View)sender!).Text; + + view.Text = "after"; + + Assert.Equal ("after", textDuringChanged); + } + + [Fact] + public void OnTextChanging_Override_CanCancel () + { + CancellingView view = new (); + // Set initial text before enabling cancellation + view.AllowChange = true; + view.Text = "initial"; + view.AllowChange = false; + + view.Text = "blocked"; + + Assert.Equal ("initial", view.Text); + } + + [Fact] + public void OnTextChanged_Override_CalledAfterChange () + { + TrackingView view = new () { Text = "start" }; + + view.Text = "end"; + + Assert.True (view.OnTextChangedCalled); + Assert.Equal ("end", view.TextAtOnTextChanged); + } + + [Fact] + public void TextChanging_EventOrder_ChangingBeforeChanged () + { + View view = new () { Text = "a" }; + List order = []; + view.TextChanging += (_, _) => order.Add ("changing"); + view.TextChanged += (_, _) => order.Add ("changed"); + + view.Text = "b"; + + Assert.Equal (["changing", "changed"], order); + } + + /// A test subclass that cancels text changes via . + private class CancellingView : View + { + public bool AllowChange { get; set; } + + protected override bool OnTextChanging () => !AllowChange; + } + + /// A test subclass that tracks calls to . + private class TrackingView : View + { + public bool OnTextChangedCalled { get; private set; } + public string? TextAtOnTextChanged { get; private set; } + + protected override void OnTextChanged () + { + OnTextChangedCalled = true; + TextAtOnTextChanged = Text; + + base.OnTextChanged (); + } + } +} From 9ad0d8a86908c2efe74369aa128853acd410cc0a Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 22 May 2026 11:06:34 -0600 Subject: [PATCH 02/13] BREAKING CHANGE: Make View.Text non-virtual; migrate all overrides to CWP events Remove irtual from View.Text property and add SetTextDirect() for derived views to update base text without re-entering CWP flow. Migration patterns: - Simple views (Label, Button, CheckBox, TitleView, Code, Shortcut): Override OnTextChanged() to sync with internal models. - Domain adapters (DatePicker, ColorPicker, ProgressBar, ArrangerButton, LinearRangeViewBase): Override OnTextChanged() for parsing + SetTextDirect() for reverse sync. - Markdown views: Override OnTextChanged() + UpdateTextFormatterText() to suppress raw text rendering. - TextField/TextView: Use 'new' keyword (independent storage) + SetTextDirect() to keep base in sync. - TextValidateField: Override OnTextChanged() + SetTextDirect() after provider transforms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ViewBase/Adornment/ArrangerButton.cs | 23 +++-- Terminal.Gui/ViewBase/Adornment/TitleView.cs | 7 +- Terminal.Gui/ViewBase/View.Text.cs | 22 ++++- Terminal.Gui/Views/Button.cs | 9 +- Terminal.Gui/Views/CheckBox.cs | 9 +- Terminal.Gui/Views/Code.cs | 14 +-- Terminal.Gui/Views/Color/ColorPicker.cs | 15 ++-- Terminal.Gui/Views/DatePicker.cs | 15 ++-- Terminal.Gui/Views/Label.cs | 9 +- .../Views/LinearRange/LinearRangeViewBase.cs | 29 +++--- Terminal.Gui/Views/Markdown/Markdown.cs | 18 +++- .../Views/Markdown/MarkdownCodeBlock.cs | 16 ++-- Terminal.Gui/Views/Markdown/MarkdownTable.cs | 83 +++++++++-------- Terminal.Gui/Views/ProgressBar.cs | 14 +-- Terminal.Gui/Views/Shortcut.cs | 14 +-- .../TextInput/TextField/TextField.Text.cs | 5 +- .../Views/TextInput/TextValidateField.cs | 90 ++++++++++++------- .../Views/TextInput/TextView/TextView.Text.cs | 5 +- .../Views/TextInput/TextView/TextView.cs | 4 + .../ViewBase/Keyboard/KeyboardEventTests.cs | 2 +- 20 files changed, 249 insertions(+), 154 deletions(-) diff --git a/Terminal.Gui/ViewBase/Adornment/ArrangerButton.cs b/Terminal.Gui/ViewBase/Adornment/ArrangerButton.cs index 58c5b108ab..0da318c9be 100644 --- a/Terminal.Gui/ViewBase/Adornment/ArrangerButton.cs +++ b/Terminal.Gui/ViewBase/Adornment/ArrangerButton.cs @@ -108,6 +108,7 @@ public ArrangeButtons ButtonType } _buttonType = value; + SetTextDirect (GetButtonGlyph ()); ApplyOrientationAndDirection (); SetupKeyBindings (); } @@ -123,19 +124,15 @@ public ArrangeButtons ButtonType public NavigationDirection Direction { get; set; } /// - public override string Text - { - get => - ButtonType switch - { - ArrangeButtons.Move => $"{Glyphs.Move}", - ArrangeButtons.AllSize => $"{Glyphs.SizeBottomRight}", - ArrangeButtons.LeftSize or ArrangeButtons.RightSize => $"{Glyphs.SizeHorizontal}", - ArrangeButtons.TopSize or ArrangeButtons.BottomSize => $"{Glyphs.SizeVertical}", - _ => base.Text - }; - set => base.Text = value; - } + private string GetButtonGlyph () => + ButtonType switch + { + ArrangeButtons.Move => $"{Glyphs.Move}", + ArrangeButtons.AllSize => $"{Glyphs.SizeBottomRight}", + ArrangeButtons.LeftSize or ArrangeButtons.RightSize => $"{Glyphs.SizeHorizontal}", + ArrangeButtons.TopSize or ArrangeButtons.BottomSize => $"{Glyphs.SizeVertical}", + _ => Text + }; /// /// Sets and based on . diff --git a/Terminal.Gui/ViewBase/Adornment/TitleView.cs b/Terminal.Gui/ViewBase/Adornment/TitleView.cs index bb3a78fa23..9bff8ad1a0 100644 --- a/Terminal.Gui/ViewBase/Adornment/TitleView.cs +++ b/Terminal.Gui/ViewBase/Adornment/TitleView.cs @@ -112,7 +112,12 @@ protected override bool OnGettingAttributeForRole (in VisualRole role, ref Attri public NavigationDirection Direction { get; set; } /// - public override string Text { get => base.Text; set => base.Text = Title = value; } + protected override void OnTextChanged () + { + Title = Text; + + base.OnTextChanged (); + } /// /// Binds the appropriate arrow keys to their directional based on diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index ade1e0c3ed..026c5c564f 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -59,7 +59,7 @@ public bool PreserveTrailingSpaces /// After the text is changed, the event is raised. /// /// - public virtual string Text + public string Text { get => _text; set @@ -91,6 +91,26 @@ public virtual string Text } } + /// + /// Sets the backing field directly without raising + /// or events and without invoking the + /// or virtual methods. + /// + /// + /// + /// Use this method in derived views that maintain an internal text model (e.g., an editor + /// buffer) and need to keep in sync after internal edits without + /// re-entering the CWP flow. + /// + /// + /// The new text value to store. + protected void SetTextDirect (string value) + { + _text = value; + UpdateTextFormatterText (); + SetNeedsLayout (); + } + /// /// Called before the changes. Invokes the event, which can /// be cancelled. diff --git a/Terminal.Gui/Views/Button.cs b/Terminal.Gui/Views/Button.cs index 76cddab16a..6b71208b8f 100644 --- a/Terminal.Gui/Views/Button.cs +++ b/Terminal.Gui/Views/Button.cs @@ -170,13 +170,18 @@ private void SetMouseBindings (MouseFlags? mouseHoldRepeat) private void Button_TitleChanged (object? sender, EventArgs e) { - base.Text = e.Value; + SetTextDirect (e.Value); TextFormatter.HotKeySpecifier = HotKeySpecifier; _interiorTextFormatter.HotKeySpecifier = HotKeySpecifier; } /// - public override string Text { get => Title; set => base.Text = Title = value; } + protected override void OnTextChanged () + { + Title = Text; + + base.OnTextChanged (); + } /// public override Rune HotKeySpecifier diff --git a/Terminal.Gui/Views/CheckBox.cs b/Terminal.Gui/Views/CheckBox.cs index a277078007..ee8c3521c5 100644 --- a/Terminal.Gui/Views/CheckBox.cs +++ b/Terminal.Gui/Views/CheckBox.cs @@ -63,12 +63,17 @@ protected override void OnActivated (ICommandContext? commandContext) private void Checkbox_TitleChanged (object? sender, EventArgs e) { - base.Text = e.Value; + SetTextDirect (e.Value); TextFormatter.HotKeySpecifier = HotKeySpecifier; } /// - public override string Text { get => Title; set => base.Text = Title = value; } + protected override void OnTextChanged () + { + Title = Text; + + base.OnTextChanged (); + } /// public override Rune HotKeySpecifier { get => base.HotKeySpecifier; set => TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value; } diff --git a/Terminal.Gui/Views/Code.cs b/Terminal.Gui/Views/Code.cs index e40e6e9cc3..cbdc6c8e1a 100644 --- a/Terminal.Gui/Views/Code.cs +++ b/Terminal.Gui/Views/Code.cs @@ -13,19 +13,11 @@ public Code () } /// Gets or sets the source text to render. - public override string Text + protected override void OnTextChanged () { - get => base.Text; - set - { - if (base.Text == value) - { - return; - } + UpdateStyledLines (); - base.Text = value; - UpdateStyledLines (); - } + base.OnTextChanged (); } /// Gets or sets the language hint used for syntax highlighting. diff --git a/Terminal.Gui/Views/Color/ColorPicker.cs b/Terminal.Gui/Views/Color/ColorPicker.cs index 9e97e6dd57..8365b823b3 100644 --- a/Terminal.Gui/Views/Color/ColorPicker.cs +++ b/Terminal.Gui/Views/Color/ColorPicker.cs @@ -153,16 +153,14 @@ protected override bool OnDrawingContent (DrawContext? context) protected virtual void OnValueChanged (ValueChangedEventArgs args) { } /// - public override string Text + protected override void OnTextChanged () { - get => SelectedColor.ToString (); - set + if (_colorNameResolver.TryParseColor (Text, out Color newColor)) { - if (_colorNameResolver.TryParseColor (value, out Color newColor)) - { - SelectedColor = newColor; - } + SelectedColor = newColor; } + + base.OnTextChanged (); } /// @@ -288,6 +286,9 @@ private void SetSelectedColor (Color value, bool syncBars) // Do the work _selectedColor = value; + // Keep Text in sync with the new color + SetTextDirect (_selectedColor.ToString ()); + // CWP: Fire ValueChanged ValueChangedEventArgs changedArgs = new (oldValue, value); OnValueChanged (changedArgs); diff --git a/Terminal.Gui/Views/DatePicker.cs b/Terminal.Gui/Views/DatePicker.cs index ec9740848b..84840b07d3 100644 --- a/Terminal.Gui/Views/DatePicker.cs +++ b/Terminal.Gui/Views/DatePicker.cs @@ -47,16 +47,14 @@ public CultureInfo? Culture } /// - public override string Text + protected override void OnTextChanged () { - get => Value.ToString (Format); - set + if (DateTime.TryParse (Text, out DateTime result)) { - if (DateTime.TryParse (value, out DateTime result)) - { - Value = result; - } + Value = result; } + + base.OnTextChanged (); } #region IValue Implementation @@ -90,6 +88,9 @@ public DateTime Value _date = value; + // Keep Text in sync with the new date value + SetTextDirect (_date.ToString (Format)); + // Propagate value to embedded editor _dateEditor?.Value = value; diff --git a/Terminal.Gui/Views/Label.cs b/Terminal.Gui/Views/Label.cs index 94073acf28..a553a59d1c 100644 --- a/Terminal.Gui/Views/Label.cs +++ b/Terminal.Gui/Views/Label.cs @@ -28,12 +28,17 @@ public Label () private void Label_TitleChanged (object? sender, EventArgs e) { - base.Text = e.Value; + SetTextDirect (e.Value); TextFormatter.HotKeySpecifier = HotKeySpecifier; } /// - public override string Text { get => Title; set => base.Text = Title = value; } + protected override void OnTextChanged () + { + Title = Text; + + base.OnTextChanged (); + } /// public override Rune HotKeySpecifier { get => base.HotKeySpecifier; set => TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value; } diff --git a/Terminal.Gui/Views/LinearRange/LinearRangeViewBase.cs b/Terminal.Gui/Views/LinearRange/LinearRangeViewBase.cs index 522b8eac67..44dd2c8bec 100644 --- a/Terminal.Gui/Views/LinearRange/LinearRangeViewBase.cs +++ b/Terminal.Gui/Views/LinearRange/LinearRangeViewBase.cs @@ -189,22 +189,21 @@ protected LinearRangeViewBase (List? options, Orientation orientation, /// /// Setting the Text of a linear range is a shortcut to setting options. The text is a CSV string of the options. /// - public override string Text + protected override void OnTextChanged () { - // Return labels as a CSV string - get => _options is null or { Count: 0 } ? string.Empty : string.Join (",", _options); - set + string value = Text; + + if (string.IsNullOrEmpty (value)) { - if (string.IsNullOrEmpty (value)) - { - Options = []; - } - else - { - IEnumerable list = value.Split (',').Select (x => x.Trim ()); - Options = list.Select (x => new LinearRangeOption { Legend = x }).ToList (); - } + Options = []; + } + else + { + IEnumerable list = value.Split (',').Select (x => x.Trim ()); + Options = list.Select (x => new LinearRangeOption { Legend = x }).ToList (); } + + base.OnTextChanged (); } /// Allow no selection. @@ -370,6 +369,10 @@ public List> Options // Drop any selected indices that are no longer valid _setOptions.RemoveAll (i => i < 0 || i >= _options.Count); + // Keep Text in sync with Options + string csv = _options.Count == 0 ? string.Empty : string.Join (",", _options); + SetTextDirect (csv); + if (_options.Count == 0) { return; diff --git a/Terminal.Gui/Views/Markdown/Markdown.cs b/Terminal.Gui/Views/Markdown/Markdown.cs index a0cac24c2e..fca9e6110d 100644 --- a/Terminal.Gui/Views/Markdown/Markdown.cs +++ b/Terminal.Gui/Views/Markdown/Markdown.cs @@ -123,7 +123,23 @@ protected override bool OnActivating (CommandEventArgs args) /// Gets or sets the Markdown-formatted text displayed by this view. /// The raw Markdown string. Setting this property triggers reparsing, re-layout, and a redraw. - public override string Text { get => _markdown; set => SetMarkdown (value); } + protected override void OnTextChanged () + { + SetMarkdown (Text); + + base.OnTextChanged (); + } + + /// + /// + /// does not use for rendering. + /// It uses its own styled-line pipeline. Clearing the formatter text prevents the base + /// from drawing raw markdown as plain text. + /// + protected override void UpdateTextFormatterText () + { + TextFormatter.Text = string.Empty; + } /// /// diff --git a/Terminal.Gui/Views/Markdown/MarkdownCodeBlock.cs b/Terminal.Gui/Views/Markdown/MarkdownCodeBlock.cs index 1ee078382f..a1b046dfc3 100644 --- a/Terminal.Gui/Views/Markdown/MarkdownCodeBlock.cs +++ b/Terminal.Gui/Views/Markdown/MarkdownCodeBlock.cs @@ -43,15 +43,17 @@ public MarkdownCodeBlock () /// (```lang\ncode\n```) and extracts the language automatically. Plain text /// (without fences) is also accepted and treated as language-less code. /// - public override string Text + protected override void OnTextChanged () { - get - { - string body = string.Join ("\n", CodeLines); + ParseFencedText (Text); - return !string.IsNullOrEmpty (Language) ? $"```{Language}\n{body}\n```" : body; - } - set => ParseFencedText (value); + base.OnTextChanged (); + } + + /// + protected override void UpdateTextFormatterText () + { + TextFormatter.Text = string.Empty; } /// diff --git a/Terminal.Gui/Views/Markdown/MarkdownTable.cs b/Terminal.Gui/Views/Markdown/MarkdownTable.cs index a503326905..b0083e3a65 100644 --- a/Terminal.Gui/Views/Markdown/MarkdownTable.cs +++ b/Terminal.Gui/Views/Markdown/MarkdownTable.cs @@ -86,55 +86,61 @@ public MarkdownTable () /// The setter parses the text via and updates . /// Invalid or empty text clears the table. /// - public override string Text + protected override void OnTextChanged () { - get + string value = Text; + + if (string.IsNullOrWhiteSpace (value)) { - if (_data.ColumnCount == 0) - { - return string.Empty; - } + TableData = _emptyData; + } + else + { + string [] lines = value.Split ('\n', StringSplitOptions.RemoveEmptyEntries); + TableData? parsed = TableData.TryParse (lines); + TableData = parsed ?? _emptyData; + } - // Reconstruct pipe-delimited table text - List lines = [$"| {string.Join (" | ", _data.Headers)} |"]; + base.OnTextChanged (); + } - // Separator row - var seps = new string [_data.ColumnCount]; + /// + protected override void UpdateTextFormatterText () + { + TextFormatter.Text = string.Empty; + } - for (var i = 0; i < _data.ColumnCount; i++) - { - seps [i] = _data.ColumnAlignments [i] switch - { - Alignment.Center => ":---:", - Alignment.End => "---:", - _ => "---" - }; - } + private string BuildTableText () + { + if (_data.ColumnCount == 0) + { + return string.Empty; + } - lines.Add ($"| {string.Join (" | ", seps)} |"); + // Reconstruct pipe-delimited table text + List lines = [$"| {string.Join (" | ", _data.Headers)} |"]; - foreach (string [] row in _data.Rows) - { - lines.Add ($"| {string.Join (" | ", row)} |"); - } + // Separator row + string [] seps = new string [_data.ColumnCount]; - return string.Join ("\n", lines); - } - set + for (var i = 0; i < _data.ColumnCount; i++) { - // Guard: View base constructor calls Text setter before MarkdownTable() initializes fields. - - if (string.IsNullOrWhiteSpace (value)) - { - TableData = _emptyData; + seps [i] = _data.ColumnAlignments [i] switch + { + Alignment.Center => ":---:", + Alignment.End => "---:", + _ => "---" + }; + } - return; - } + lines.Add ($"| {string.Join (" | ", seps)} |"); - string [] lines = value.Split ('\n', StringSplitOptions.RemoveEmptyEntries); - TableData? parsed = TableData.TryParse (lines); - TableData = parsed ?? _emptyData; + foreach (string [] row in _data.Rows) + { + lines.Add ($"| {string.Join (" | ", row)} |"); } + + return string.Join ("\n", lines); } /// @@ -155,6 +161,9 @@ public TableData TableData _rowSegments [r] = ParseCellSegments (value.Rows [r], MarkdownStyleRole.Normal); } + // Keep Text in sync with TableData + SetTextDirect (BuildTableText ()); + // Compute initial layout using current Frame width (or a default for standalone use) int initialWidth = Frame.Width > 0 ? Frame.Width : 80; _lastComputedWidth = -1; diff --git a/Terminal.Gui/Views/ProgressBar.cs b/Terminal.Gui/Views/ProgressBar.cs index cab3b90455..51449c845f 100644 --- a/Terminal.Gui/Views/ProgressBar.cs +++ b/Terminal.Gui/Views/ProgressBar.cs @@ -72,6 +72,7 @@ public float Fraction { _fraction = Math.Min (value, 1); _isActivity = false; + SetTextDirect ($"{_fraction * 100:F0}%"); UpdateTerminalProgress (); SetNeedsDraw (); } @@ -154,16 +155,15 @@ public ProgressBarStyle ProgressBarStyle /// is the percentage will be /// displayed. If is a marquee style, the text will be displayed. /// - public override string Text + protected override bool OnTextChanging () { - get => string.IsNullOrEmpty (base.Text) ? $"{_fraction * 100:F0}%" : base.Text; - set + // Only allow text to be set externally in marquee mode + if (ProgressBarStyle is not (ProgressBarStyle.MarqueeBlocks or ProgressBarStyle.MarqueeContinuous)) { - if (ProgressBarStyle is ProgressBarStyle.MarqueeBlocks or ProgressBarStyle.MarqueeContinuous) - { - base.Text = value; - } + return true; } + + return false; } /// diff --git a/Terminal.Gui/Views/Shortcut.cs b/Terminal.Gui/Views/Shortcut.cs index 7f29fd8660..a08b0eb621 100644 --- a/Terminal.Gui/Views/Shortcut.cs +++ b/Terminal.Gui/Views/Shortcut.cs @@ -139,6 +139,7 @@ public Shortcut (Key key, string? commandText, Action? action, string? helpText HelpView.Id = "_helpView"; #endif HelpView.Text = helpText ?? string.Empty; + SetTextDirect (helpText ?? string.Empty); HelpView.GettingAttributeForRole += SubViewOnGettingAttributeForRole; #if DEBUG @@ -679,14 +680,12 @@ private void SetHelpViewDefaultLayout () /// Gets or sets the help text displayed in the middle of the Shortcut. Identical in function to /// . /// - public override string Text + protected override void OnTextChanged () { - get => HelpView.Text; - set - { - HelpView.Text = value; - ShowHide (); - } + HelpView.Text = Text; + ShowHide (); + + base.OnTextChanged (); } /// @@ -698,6 +697,7 @@ public string HelpText set { HelpView.Text = value; + SetTextDirect (value); ShowHide (); } } diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 619eaf3dfc..6675750fe9 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -394,7 +394,7 @@ private void InsertText (Key a, bool usePreTextChangedCursorPos) private void SetText (IEnumerable newText) => SetText (newText.ToList ()); /// Sets or gets the text held by the view. - public override string Text + public new string Text { get => StringExtensions.ToString (_text); set @@ -439,6 +439,9 @@ public override string Text // Note we use NewValue here; TextChanging subscribers may have changed it _text = args.Result!.ToStringList (); + // Keep base View._text in sync + SetTextDirect (StringExtensions.ToString (_text)); + if (!Secret && !_historyText.IsFromHistory) { _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); diff --git a/Terminal.Gui/Views/TextInput/TextValidateField.cs b/Terminal.Gui/Views/TextInput/TextValidateField.cs index e32ed82e4d..7a0318126c 100644 --- a/Terminal.Gui/Views/TextInput/TextValidateField.cs +++ b/Terminal.Gui/Views/TextInput/TextValidateField.cs @@ -152,6 +152,7 @@ public ITextValidateProvider? Provider { _provider.TextChanged += ProviderOnTextChanged; _lastKnownText = _provider.Text; + SetTextDirect (_provider.Text); } if (_provider!.Fixed) @@ -203,56 +204,76 @@ protected virtual void OnValueChanged (ValueChangedEventArgs args) { } #endregion /// Text - public override string Text + protected override void OnTextChanged () { - get => _provider is null ? string.Empty : _provider.Text; - set + string value = Text; + + if (_provider is null) { - if (_provider is null) - { - return; - } + base.OnTextChanged (); + + return; + } - string oldValue = _provider.Text; + string oldValue = _provider.Text; - if (oldValue == value) - { - return; - } + if (oldValue == value) + { + base.OnTextChanged (); - if (!SuppressValueEvents) + return; + } + + if (!SuppressValueEvents) + { + ValueChangingEventArgs args = new (oldValue, value); + + if (OnValueChanging (args) || args.Handled) { - ValueChangingEventArgs args = new (oldValue, value); + // Revert Text to the provider's current value + SetTextDirect (oldValue); - if (OnValueChanging (args) || args.Handled) - { - return; - } + return; + } - ValueChanging?.Invoke (this, args); + ValueChanging?.Invoke (this, args); - if (args.Handled) - { - return; - } + if (args.Handled) + { + // Revert Text to the provider's current value + SetTextDirect (oldValue); - // Allow subscribers to modify the new value - value = args.NewValue ?? string.Empty; + return; } - _lastKnownText = value; - _provider.Text = value; + // Allow subscribers to modify the new value + value = args.NewValue ?? string.Empty; - if (!SuppressValueEvents) + if (value != Text) { - ValueChangedEventArgs changedArgs = new (oldValue, value); - OnValueChanged (changedArgs); - ValueChanged?.Invoke (this, changedArgs); - ValueChangedUntyped?.Invoke (this, new ValueChangedEventArgs (oldValue, value)); + SetTextDirect (value); } + } - SetNeedsDraw (); + _lastKnownText = value; + _provider.Text = value; + + // The provider may transform the text (e.g., mask formatting). + // Keep base _text in sync with the provider's actual text. + SetTextDirect (_provider.Text); + + if (!SuppressValueEvents) + { + string oldVal = oldValue; + ValueChangedEventArgs changedArgs = new (oldVal, _provider.Text); + OnValueChanged (changedArgs); + ValueChanged?.Invoke (this, changedArgs); + ValueChangedUntyped?.Invoke (this, new ValueChangedEventArgs (oldVal, _provider.Text)); } + + SetNeedsDraw (); + + base.OnTextChanged (); } private int _insertionPoint; @@ -324,6 +345,9 @@ private void ProviderOnTextChanged (object? sender, EventArgs e) // Sync _lastKnownText with actual provider state (may have been reverted by handler) _lastKnownText = _provider.Text; + + // Keep base Text in sync with provider + SetTextDirect (_provider.Text); } private void RevertProviderText (string oldText) diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index 48b2da8839..f849675299 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -165,7 +165,7 @@ public int TabWidth /// The event is fired whenever this property is set. Note, however, that Text is not /// set by as the user types. /// - public override string Text + public new string Text { get { @@ -188,6 +188,9 @@ public override string Text _lastWrapWidth = Viewport.Width; } + // Keep base View._text in sync + SetTextDirect (value); + OnTextChanged (); SetNeedsDraw (); diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.cs index 0bc370c5b4..5464e1806f 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.cs @@ -127,6 +127,10 @@ public TextView () _model.LinesLoaded += Model_LinesLoaded!; _historyText.ChangeText += HistoryText_ChangeText; + // Initialize the model with at least one empty line + _model.LoadString (string.Empty); + _historyText.Clear (_model.GetAllLines ()); + CreateCommandsAndBindings (); _currentCulture = Thread.CurrentThread.CurrentUICulture; diff --git a/Tests/UnitTestsParallelizable/ViewBase/Keyboard/KeyboardEventTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Keyboard/KeyboardEventTests.cs index cbefc74404..ffd16b24bc 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Keyboard/KeyboardEventTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Keyboard/KeyboardEventTests.cs @@ -249,7 +249,7 @@ public class OnNewKeyTestView : View public bool CancelVirtualMethods { set; private get; } public bool OnKeyDownCalled { get; set; } public bool OnProcessKeyDownCalled { get; set; } - public override string Text { get; set; } + public new string Text { get; set; } protected override bool OnKeyDown (Key keyEvent) { From 3173757591fca987145296fb471efd73615357f5 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 22 May 2026 11:40:31 -0600 Subject: [PATCH 03/13] Fix CI: ReactiveExample source generator conflict and XML cref warnings - ReactiveExample: Replace .Events().TextChanged on TextField with Observable.FromEventPattern to avoid ReactiveMarbles source generator producing conflicting handler code for the new/base TextChanging event pair (CS0123) - DateEditor/TimeEditor: Fix CS1574 by changing cref from TextValidateField.Text to View.Text Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Examples/ReactiveExample/LoginView.cs | 12 ++++++------ Terminal.Gui/Views/TextInput/DateEditor.cs | 2 +- Terminal.Gui/Views/TextInput/TimeEditor.cs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Examples/ReactiveExample/LoginView.cs b/Examples/ReactiveExample/LoginView.cs index 9cceecde66..f5fff0413b 100644 --- a/Examples/ReactiveExample/LoginView.cs +++ b/Examples/ReactiveExample/LoginView.cs @@ -48,9 +48,9 @@ public LoginView (LoginViewModel viewModel) .BindTo (unInput, x => x.Text) .DisposeWith (_disposable); - unInput - .Events () - .TextChanged + Observable.FromEventPattern ( + h => unInput.TextChanged += h, + h => unInput.TextChanged -= h) .Select (_ => unInput.Text) .DistinctUntilChanged () .BindTo (ViewModel, x => x.Username) @@ -79,9 +79,9 @@ public LoginView (LoginViewModel viewModel) .BindTo (pwInput, x => x.Text) .DisposeWith (_disposable); - pwInput - .Events () - .TextChanged + Observable.FromEventPattern ( + h => pwInput.TextChanged += h, + h => pwInput.TextChanged -= h) .Select (_ => pwInput.Text) .DistinctUntilChanged () .BindTo (ViewModel, x => x.Password) diff --git a/Terminal.Gui/Views/TextInput/DateEditor.cs b/Terminal.Gui/Views/TextInput/DateEditor.cs index 7cac78f36e..193509ddeb 100644 --- a/Terminal.Gui/Views/TextInput/DateEditor.cs +++ b/Terminal.Gui/Views/TextInput/DateEditor.cs @@ -160,7 +160,7 @@ bool IValue.TrySetValueFromString (string input) /// /// Synchronizes the backing field when the base class - /// property changes programmatically. + /// property changes programmatically. /// protected override void OnValueChanged (ValueChangedEventArgs args) => _value = DateProvider.DateValue; diff --git a/Terminal.Gui/Views/TextInput/TimeEditor.cs b/Terminal.Gui/Views/TextInput/TimeEditor.cs index 4f995754df..f545e8c0a2 100644 --- a/Terminal.Gui/Views/TextInput/TimeEditor.cs +++ b/Terminal.Gui/Views/TextInput/TimeEditor.cs @@ -176,7 +176,7 @@ bool IValue.TrySetValueFromString (string input) /// /// Synchronizes the backing field when the base class - /// property changes programmatically. + /// property changes programmatically. /// protected override void OnValueChanged (ValueChangedEventArgs args) => _value = TimeProvider.TimeValue; From fdd4ea2a5272bfb62ac7e9fa62aad9cd49217a21 Mon Sep 17 00:00:00 2001 From: Tig Date: Fri, 22 May 2026 11:52:29 -0600 Subject: [PATCH 04/13] Address CR feedback: CWP compliance, polymorphic sync, ProgressBar fallback 1. OnTextChanging() now raises TextChanging event (CWP: virtual raises event) 2. TextField: override OnTextChanged() to sync internal _text from base.Text when set polymorphically via a View reference 3. TextView: override OnTextChanged() to sync internal TextModel from base.Text when set polymorphically via a View reference 4. ProgressBar: initialize Text to '0%' in constructor so SimplePlusPercentage renders on first draw Added 4 regression tests (verified failing before fixes, passing after). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Text.cs | 16 ++-- Terminal.Gui/Views/ProgressBar.cs | 1 + .../TextInput/TextField/TextField.Text.cs | 26 +++++- .../Views/TextInput/TextView/TextView.Text.cs | 31 ++++++++ .../ViewBase/TextCwpTests.cs | 79 +++++++++++++++++++ 5 files changed, 142 insertions(+), 11 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index 026c5c564f..8b30b191fb 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -74,14 +74,6 @@ public string Text return; } - CancelEventArgs args = new (); - TextChanging?.Invoke (this, args); - - if (args.Cancel) - { - return; - } - _text = value; UpdateTextFormatterText (); @@ -122,7 +114,13 @@ protected void SetTextDirect (string value) /// /// /// if the text change should be cancelled; otherwise . - protected virtual bool OnTextChanging () => false; + protected virtual bool OnTextChanging () + { + CancelEventArgs args = new (); + TextChanging?.Invoke (this, args); + + return args.Cancel; + } /// /// Raised when the is about to change. Set to diff --git a/Terminal.Gui/Views/ProgressBar.cs b/Terminal.Gui/Views/ProgressBar.cs index 51449c845f..f350e0ee1b 100644 --- a/Terminal.Gui/Views/ProgressBar.cs +++ b/Terminal.Gui/Views/ProgressBar.cs @@ -55,6 +55,7 @@ public ProgressBar () Height = Dim.Auto (DimAutoStyle.Content, 1); CanFocus = false; _fraction = 0; + SetTextDirect ("0%"); } /// diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 6675750fe9..bbc7ca021e 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -466,8 +466,30 @@ private void InsertText (Key a, bool usePreTextChangedCursorPos) } } - /// - /// Returns if the current cursor position is at the end of the . This + /// + /// + /// Syncs the internal text buffer when is set through a polymorphic + /// () reference, ensuring the TextField's editing model stays consistent. + /// + protected override void OnTextChanged () + { + string baseText = base.Text; + + // Only sync when the internal model diverges (polymorphic setter case). + // When TextField's own `new Text` setter runs, it already updates _text + // and calls SetTextDirect, so base.Text matches _text — this is a no-op. + if (StringExtensions.ToString (_text) != baseText) + { + _text = baseText.ToStringList (); + + if (InsertionPoint > _text.Count) + { + InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); + } + } + + base.OnTextChanged (); + } /// includes when it is empty. /// /// diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index f849675299..74e6c83fc1 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -198,6 +198,37 @@ public int TabWidth } } + /// + /// + /// Syncs the internal when is set through a + /// polymorphic () reference, ensuring the TextView's editing model stays consistent. + /// + protected override void OnTextChanged () + { + string baseText = base.Text; + + // Only sync when the internal model diverges (polymorphic setter case). + // When TextView's own `new Text` setter runs, it already calls LoadString + // and SetTextDirect, so base.Text matches _model — this is a no-op. + if (_model.ToString () != baseText) + { + ResetPosition (); + _model.LoadString (baseText); + + if (_wordWrap) + { + _wrapManager = new WordWrapManager (_model); + _model = _wrapManager.WrapModel (Viewport.Width, out _, out _, out _, out _); + _lastWrapWidth = Viewport.Width; + } + + _historyText.Clear (_model.GetAllLines ()); + SetNeedsDraw (); + } + + base.OnTextChanged (); + } + /// /// Tracks whether the text view should be considered "used", that is, that the user has moved in the entry, so /// new input should be appended at the cursor position, rather than clearing the entry diff --git a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs index e05a85bd2f..76d8641a72 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs @@ -126,6 +126,69 @@ public void TextChanging_EventOrder_ChangingBeforeChanged () Assert.Equal (["changing", "changed"], order); } + // --- CR feedback tests: these must fail before fixes, pass after --- + + /// + /// CR Issue 1: should raise + /// (CWP pattern: virtual method raises the event). A subclass that calls base.OnTextChanging() + /// should get the event's cancellation result. + /// + [Fact] + public void OnTextChanging_BaseRaisesEvent_SubclassGetsCancel () // Copilot + { + DelegatingView view = new (); + view.TextChanging += (_, e) => e.Cancel = true; + + view.Text = "hello"; + + // base.OnTextChanging() should have raised TextChanging and returned true (cancelled) + Assert.True (view.BaseReturnedCancel); + Assert.Equal (string.Empty, view.Text); + } + + /// + /// CR Issue 2: Setting polymorphically on a + /// should sync the TextField's internal model so that textField.Text returns the new value. + /// + [Fact] + public void TextField_PolymorphicSet_SyncsInternalModel () // Copilot + { + TextField tf = new (); + View v = tf; + + v.Text = "hello"; + + Assert.Equal ("hello", tf.Text); + } + + /// + /// CR Issue 3: Setting polymorphically on a + /// should sync the TextView's internal model so that textView.Text returns the new value. + /// + [Fact] + public void TextView_PolymorphicSet_SyncsInternalModel () // Copilot + { + TextView tv = new (); + View v = tv; + + v.Text = "hello"; + + Assert.Equal ("hello", tv.Text); + } + + /// + /// CR Issue 4: A newly constructed should have + /// set to "0%" so that renders the percentage + /// on first draw. + /// + [Fact] + public void ProgressBar_Constructor_TextShowsZeroPercent () // Copilot + { + ProgressBar pb = new (); + + Assert.Equal ("0%", pb.Text); + } + /// A test subclass that cancels text changes via . private class CancellingView : View { @@ -148,4 +211,20 @@ protected override void OnTextChanged () base.OnTextChanged (); } } + + /// + /// A test subclass that delegates to base.OnTextChanging() and records + /// whether the base returned a cancel signal. + /// + private class DelegatingView : View + { + public bool BaseReturnedCancel { get; private set; } + + protected override bool OnTextChanging () + { + BaseReturnedCancel = base.OnTextChanging (); + + return BaseReturnedCancel; + } + } } From 5246356361765978ee2de10679126287df6584d8 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 12:08:51 -0600 Subject: [PATCH 05/13] Fix TextField and TextView to raise View.TextChanging in their setters TextField's ew Text setter now calls ase.OnTextChanging() after its own validation passes (ValueChanging, TextField.TextChanging), so subscribers holding a View reference see the pre-change notification and can cancel. TextView's ew Text setter now calls OnTextChanging() at the start, so View.TextChanging is raised before the model is modified. This ensures both controls participate in the base-level CWP flow when their Text property is set directly (not just polymorphically). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs | 6 ++++++ Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index bbc7ca021e..658808e6d5 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -434,6 +434,12 @@ private void InsertText (Key a, bool usePreTextChangedCursorPos) return; } + // Raise View.TextChanging so subscribers holding a View reference can cancel. + if (base.OnTextChanging ()) + { + return; + } + ClearAllSelection (); // Note we use NewValue here; TextChanging subscribers may have changed it diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index 74e6c83fc1..1532f3ac02 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -178,6 +178,12 @@ public int TabWidth } set { + // Raise View.TextChanging so subscribers holding a View reference can cancel. + if (OnTextChanging ()) + { + return; + } + ResetPosition (); _model.LoadString (value); From 20affe7be53a8bdc5cdb11a578227fd95a1f157e Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 15:26:39 -0600 Subject: [PATCH 06/13] Remove TextField's 'new Text' - use CWP overrides instead TextField now uses View.Text directly instead of hiding it with 'new'. This fixes the polymorphism issue: setting Text through a View reference now correctly goes through TextField's validation and model sync. Changes: - View.OnTextChanging now accepts 'string proposedValue' so overrides can validate the incoming text - TextField overrides OnTextChanging to sanitize (strip tabs/newlines), fire IValue.ValueChanging, and fire TextField.TextChanging - TextField overrides OnTextChanged to sync the internal grapheme-list model, track history, fire ValueChanged, and adjust cursor - ProgressBar.OnTextChanging updated for new signature - TextView.OnTextChanging call updated for new signature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Text.cs | 9 +- Terminal.Gui/Views/ProgressBar.cs | 4 +- .../TextInput/TextField/TextField.Text.cs | 156 +++++++++--------- .../Views/TextInput/TextView/TextView.Text.cs | 2 +- .../ViewBase/TextCwpTests.cs | 6 +- 5 files changed, 91 insertions(+), 86 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index 8b30b191fb..8c2669b688 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -69,7 +69,7 @@ public string Text return; } - if (OnTextChanging ()) + if (OnTextChanging (value)) { return; } @@ -109,12 +109,13 @@ protected void SetTextDirect (string value) /// /// /// - /// This is a signal-only notification. It does not carry old or new text values because - /// semantics vary across derived views. + /// The base implementation raises the event. Override in derived views + /// to perform validation or fire control-specific pre-change events. /// /// + /// The proposed new text value. /// if the text change should be cancelled; otherwise . - protected virtual bool OnTextChanging () + protected virtual bool OnTextChanging (string newText) { CancelEventArgs args = new (); TextChanging?.Invoke (this, args); diff --git a/Terminal.Gui/Views/ProgressBar.cs b/Terminal.Gui/Views/ProgressBar.cs index f350e0ee1b..ab0af77d41 100644 --- a/Terminal.Gui/Views/ProgressBar.cs +++ b/Terminal.Gui/Views/ProgressBar.cs @@ -156,7 +156,7 @@ public ProgressBarStyle ProgressBarStyle /// is the percentage will be /// displayed. If is a marquee style, the text will be displayed. /// - protected override bool OnTextChanging () + protected override bool OnTextChanging (string newText) { // Only allow text to be set externally in marquee mode if (ProgressBarStyle is not (ProgressBarStyle.MarqueeBlocks or ProgressBarStyle.MarqueeContinuous)) @@ -164,7 +164,7 @@ protected override bool OnTextChanging () return true; } - return false; + return base.OnTextChanging (newText); } /// diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 658808e6d5..a7db51e766 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -390,112 +390,116 @@ private void InsertText (Key a, bool usePreTextChangedCursorPos) /// private List _text; + /// + /// Stashes the final text value determined during (after sanitization + /// and possible subscriber modification) for use in . + /// + private string? _pendingText; + private void SetText (List newText) => Text = StringExtensions.ToString (newText); private void SetText (IEnumerable newText) => SetText (newText.ToList ()); - /// Sets or gets the text held by the view. - public new string Text + /// + /// + /// + /// TextField overrides this to sanitize text (strip tabs and newlines), fire + /// , and raise the TextField-specific + /// event (which allows subscribers to modify the text). + /// + /// + protected override bool OnTextChanging (string newText) { - get => StringExtensions.ToString (_text); - set + // Guard against base constructor calling before _text is initialized + // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract + if (_text is null) { - // Guard against base constructor calling before _text is initialized - // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract - if (_text is null) - { - return; - } - - var oldText = StringExtensions.ToString (_text); - - if (oldText == value) - { - return; - } - - string newText = value.Replace ("\t", "").Split ("\n") [0]; - - // Raise IValue.ValueChanging - if (RaiseValueChanging (oldText, newText)) - { - return; - } - - ResultEventArgs args = new (newText); - RaiseTextChanging (args); - - if (args.Handled) - { - if (InsertionPoint > _text.Count) - { - InsertionPoint = _text.Count; - } - - return; - } - - // Raise View.TextChanging so subscribers holding a View reference can cancel. - if (base.OnTextChanging ()) - { - return; - } - - ClearAllSelection (); - - // Note we use NewValue here; TextChanging subscribers may have changed it - _text = args.Result!.ToStringList (); - - // Keep base View._text in sync - SetTextDirect (StringExtensions.ToString (_text)); + return true; + } - if (!Secret && !_historyText.IsFromHistory) - { - _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); + // Sanitize: single-line, no tabs + string sanitized = newText.Replace ("\t", "").Split ("\n") [0]; - _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); - } + string oldText = StringExtensions.ToString (_text); - OnTextChanged (); + // After sanitization, check if there is an effective change + if (oldText == sanitized) + { + return true; + } - // Raise IValue.ValueChanged - RaiseValueChanged (oldText, StringExtensions.ToString (_text)); + // Raise IValue.ValueChanging + if (RaiseValueChanging (oldText, sanitized)) + { + return true; + } - ProcessAutocomplete (); + ResultEventArgs args = new (sanitized); + RaiseTextChanging (args); + if (args.Handled) + { if (InsertionPoint > _text.Count) { - InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); + InsertionPoint = _text.Count; } - Adjust (); - SetNeedsDraw (); + return true; } + + // Stash the (possibly subscriber-modified) result for OnTextChanged + _pendingText = args.Result; + + return base.OnTextChanging (newText); } /// /// - /// Syncs the internal text buffer when is set through a polymorphic - /// () reference, ensuring the TextField's editing model stays consistent. + /// + /// Syncs the internal grapheme-list editing model from the base value, + /// applying any sanitization or subscriber modifications stashed during . + /// /// protected override void OnTextChanged () { - string baseText = base.Text; + // Use stashed text from OnTextChanging if available; otherwise sanitize base.Text + string finalText = _pendingText ?? base.Text.Replace ("\t", "").Split ("\n") [0]; + _pendingText = null; - // Only sync when the internal model diverges (polymorphic setter case). - // When TextField's own `new Text` setter runs, it already updates _text - // and calls SetTextDirect, so base.Text matches _text — this is a no-op. - if (StringExtensions.ToString (_text) != baseText) + string oldText = StringExtensions.ToString (_text); + + ClearAllSelection (); + _text = finalText.ToStringList (); + + // Ensure base View._text holds the sanitized/modified value + if (base.Text != finalText) { - _text = baseText.ToStringList (); + SetTextDirect (finalText); + } - if (InsertionPoint > _text.Count) - { - InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); - } + if (!Secret && !_historyText.IsFromHistory) + { + _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); + + _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); } base.OnTextChanged (); + + // Raise IValue.ValueChanged + RaiseValueChanged (oldText, StringExtensions.ToString (_text)); + + ProcessAutocomplete (); + + if (InsertionPoint > _text.Count) + { + InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); + } + + Adjust (); + SetNeedsDraw (); } + + /// Returns if the current cursor position is at the end of the , /// includes when it is empty. /// /// diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index 1532f3ac02..95550cedb2 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -179,7 +179,7 @@ public int TabWidth set { // Raise View.TextChanging so subscribers holding a View reference can cancel. - if (OnTextChanging ()) + if (OnTextChanging (value)) { return; } diff --git a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs index 76d8641a72..9de7131bb3 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs @@ -194,7 +194,7 @@ private class CancellingView : View { public bool AllowChange { get; set; } - protected override bool OnTextChanging () => !AllowChange; + protected override bool OnTextChanging (string newText) => !AllowChange; } /// A test subclass that tracks calls to . @@ -220,9 +220,9 @@ private class DelegatingView : View { public bool BaseReturnedCancel { get; private set; } - protected override bool OnTextChanging () + protected override bool OnTextChanging (string newText) { - BaseReturnedCancel = base.OnTextChanging (); + BaseReturnedCancel = base.OnTextChanging (newText); return BaseReturnedCancel; } From f2c8fbd7f4b2eee735c7045ec87c6ee24d198b27 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 15:36:19 -0600 Subject: [PATCH 07/13] code cleanup --- .../TextInput/TextField/TextField.Mouse.cs | 2 +- .../TextInput/TextField/TextField.Text.cs | 887 +++++++++--------- 2 files changed, 449 insertions(+), 440 deletions(-) diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs index 1de72c882c..bfeff263f6 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs @@ -65,7 +65,7 @@ private int SetInsertionPointFromMouse (Mouse mouse) } else { - int cols = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset + pX, out int clipOffset, out int newInsertionIndex); + TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset + pX, out int _, out int newInsertionIndex); InsertionPoint = newInsertionIndex; } diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index a7db51e766..04c56571ee 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -1,5 +1,4 @@ using System.Globalization; -using Terminal.Gui.Drawing; namespace Terminal.Gui.Views; @@ -8,21 +7,100 @@ public partial class TextField private CultureInfo _currentCulture; private string? _lastPastedText; - /// Raised before changes. The change can be canceled the text adjusted. - public new event EventHandler>? TextChanging; + /// + /// Caches the cursor position before a text change operation. Used to properly handle text insertion + /// and deletion operations, particularly for undo/redo and proper cursor placement after edits. + /// + private int _preChangeInsertionPoint; /// - /// Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so - /// new input should be appended at the cursor position, rather than clearing the entry + /// The text content stored as a list of strings, where each string represents a single text element + /// (grapheme cluster). This allows proper handling of Unicode combining characters and emoji. /// - public bool Used { get; set; } + private List _text; - private TextModel GetModel () + /// + /// Stashes the final text value determined during (after sanitization + /// and possible subscriber modification) for use in . + /// + private string? _pendingText; + + private int _insertionPoint; + + /// + /// Gets or sets the insertion point within the text, measured as a 0-based index into text elements. + /// + /// + /// The insertion point position, clamped to the range [0, Text.Length]. Position 0 is before the first character; + /// position equal to the text length is after the last character. + /// + /// + /// + /// This property provides access to the logical insertion point within the text. The value is automatically + /// clamped to valid bounds: values less than 0 become 0, and values greater than the text length become + /// the text length. + /// + /// + /// Relationship to : + /// + /// + /// : Logical position in text elements (0-based index) + /// + /// + /// + /// : Converts logical position to screen coordinates, accounting for + /// and wide characters + /// + /// + /// + /// + /// + /// Example: For text "Hello世界" (Hello + 2 CJK characters): + /// + /// + /// InsertionPoint = 0: Before 'H' + /// + /// + /// InsertionPoint = 5: Before '世' + /// + /// + /// InsertionPoint = 7: After '界' (end of text) + /// + /// + /// Note that screen columns would differ because '世' and '界' each occupy 2 columns. + /// + /// + /// Setting this property also updates the text selection via . + /// + /// + /// + public virtual int InsertionPoint { - TextModel model = new (); - model.LoadString (Text); + get => _insertionPoint; + set + { + int previousInsertionPoint = _insertionPoint; - return model; + if (value < 0) + { + _insertionPoint = 0; + } + else if (value > _text.Count) + { + _insertionPoint = _text.Count; + } + else + { + _insertionPoint = value; + } + + PrepareSelection (_selectionAnchor, _insertionPoint - _selectionAnchor); + + if (_insertionPoint != previousInsertionPoint) + { + UpdateCursor (); + } + } } /// @@ -40,29 +118,72 @@ public void InsertText (string toAdd, bool useOldCursorPos = true) } } - /// + /// Raises the event, enabling canceling the change or adjusting the text. + /// The event arguments. + /// if the event was cancelled or the text was adjusted by the event. + public bool RaiseTextChanging (ResultEventArgs args) + { + // TODO: CWP: Add an OnTextChanging protected virtual method that can be overridden to handle text changing events. + + TextChanging?.Invoke (this, args); + + return args.Handled; + } + + /// + /// Gets the horizontal scroll offset, representing the index of the first visible text element. + /// + /// + /// A 0-based index into the text elements indicating which element appears at the left edge of the viewport. + /// /// - /// TextField is single-line — keep only the first line of the paste and drop C0/C1 control - /// characters, including tab. This matches what can actually accept and - /// keeps aligned with the text that will be inserted. + /// + /// When the text is longer than the viewport width, this property tracks how far the view has scrolled. + /// The method automatically updates this value to keep the cursor visible. + /// + /// + /// Relationship to cursor positioning: + /// + /// + /// : Absolute position in the text (0 to text length) + /// + /// + /// : Index of first visible character + /// + /// + /// + /// Screen column = - (approximately, + /// adjusted for wide chars) + /// + /// + /// + /// + /// + /// Example: For text "Hello World" with viewport width 5: + /// + /// + /// ScrollOffset = 0: Shows "Hello" + /// + /// + /// ScrollOffset = 6: Shows "World" + /// + /// + /// /// - protected override string OnSanitizingPaste (string raw) - { - int newline = raw.IndexOfAny (['\r', '\n']); - string firstLine = newline >= 0 ? raw [..newline] : raw; + /// + public int ScrollOffset { get; private set; } - StringBuilder sb = new (firstLine.Length); + /// Raised before changes. The change can be canceled the text adjusted. + public new event EventHandler>? TextChanging; - foreach (char c in firstLine) - { - if ((c >= 0x20 && c < 0x7F) || c >= 0xA0) - { - sb.Append (c); - } - } + /// + /// Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so + /// new input should be appended at the cursor position, rather than clearing the entry + /// + public bool Used { get; set; } - return sb.ToString (); - } + /// + protected override string? GetPastedEventText (string text) => _lastPastedText; /// /// @@ -82,11 +203,13 @@ protected override bool OnPaste (string text) int selStart = _selectionStart == -1 ? InsertionPoint : _selectionStart; int selectedLength = SelectedLength; List oldText = [.. _text]; - string oldTextString = StringExtensions.ToString (oldText); + var oldTextString = StringExtensions.ToString (oldText); List pastedText = text.ToStringList (); - List proposedText = [.. oldText.GetRange (0, selStart), - .. pastedText, - .. oldText.GetRange (selStart + selectedLength, oldText.Count - (selStart + selectedLength))]; + + List proposedText = + [ + .. oldText.GetRange (0, selStart), .. pastedText, .. oldText.GetRange (selStart + selectedLength, oldText.Count - (selStart + selectedLength)) + ]; Text = StringExtensions.ToString (proposedText); bool proposedEqualsFinal = AreEqual (proposedText, _text); @@ -118,287 +241,76 @@ protected override bool OnPaste (string text) } /// - protected override bool ShouldRaisePastedEvent (string text) => !ReadOnly && !string.IsNullOrEmpty (_lastPastedText); - - /// - protected override string? GetPastedEventText (string text) => _lastPastedText; - - private static void GetActualPastedText (string text, List proposedText, int pasteStart, int pastedLength, List finalText, out string actualPastedText, out int insertionPoint) + /// + /// TextField is single-line — keep only the first line of the paste and drop C0/C1 control + /// characters, including tab. This matches what can actually accept and + /// keeps aligned with the text that will be inserted. + /// + protected override string OnSanitizingPaste (string raw) { - if (AreEqual (proposedText, finalText)) - { - actualPastedText = text; - insertionPoint = Math.Min (pasteStart + pastedLength, finalText.Count); + int newline = raw.IndexOfAny (['\r', '\n']); + string firstLine = newline >= 0 ? raw [..newline] : raw; - return; - } + StringBuilder sb = new (firstLine.Length); - if (proposedText.Count == finalText.Count) + foreach (char c in firstLine) { - int actualPastedLength = Math.Min (pastedLength, finalText.Count - pasteStart); - actualPastedText = actualPastedLength > 0 - ? StringExtensions.ToString (finalText.GetRange (pasteStart, actualPastedLength)) - : string.Empty; - insertionPoint = Math.Min (pasteStart + actualPastedLength, finalText.Count); - - return; + if ((c >= 0x20 && c < 0x7F) || c >= 0xA0) + { + sb.Append (c); + } } - actualPastedText = GetActualPastedTextFromEditOperations (proposedText, pasteStart, pastedLength, finalText, out insertionPoint); + return sb.ToString (); } - private static string GetActualPastedTextFromEditOperations (List proposedText, int pasteStart, int pastedLength, List finalText, out int insertionPoint) + /// + /// + /// + /// Syncs the internal grapheme-list editing model from the base value, + /// applying any sanitization or subscriber modifications stashed during . + /// + /// + protected override void OnTextChanged () { - List operations = BuildPasteEditOperations (proposedText, finalText); - int pasteEnd = pasteStart + pastedLength; - int proposedIndex = 0; - int finalIndex = 0; - int pastedStart = -1; - int pastedEnd = -1; - int boundaryAfterPaste = -1; - - foreach (PasteEditOperation operation in operations) - { - if (ConsumesFinal (operation) - && IsWithinPasteRange (operation, proposedIndex, pasteStart, pasteEnd)) - { - if (pastedStart == -1) - { - pastedStart = finalIndex; - } - - pastedEnd = finalIndex + 1; - } - - if (ConsumesProposed (operation)) - { - proposedIndex++; + // Use stashed text from OnTextChanging if available; otherwise sanitize base.Text + string finalText = _pendingText ?? Text.Replace ("\t", "").Split ("\n") [0]; + _pendingText = null; - if (proposedIndex == pasteEnd) - { - boundaryAfterPaste = finalIndex + (ConsumesFinal (operation) ? 1 : 0); - } - } - if (ConsumesFinal (operation)) - { - finalIndex++; - } - } + var oldText = StringExtensions.ToString (_text); - insertionPoint = boundaryAfterPaste == -1 ? finalIndex : boundaryAfterPaste; + ClearAllSelection (); + _text = finalText.ToStringList (); - if (pastedStart == -1 || pastedEnd == -1 || pastedEnd <= pastedStart) + // Ensure base View._text holds the sanitized/modified value + if (Text != finalText) { - return string.Empty; + SetTextDirect (finalText); } - return StringExtensions.ToString (finalText.GetRange (pastedStart, pastedEnd - pastedStart)); - } - - private static List BuildPasteEditOperations (List proposedText, List finalText) - { - int [,] costs = new int [proposedText.Count + 1, finalText.Count + 1]; - - for (int i = 0; i <= proposedText.Count; i++) - { - costs [i, 0] = i; - } - - for (int j = 0; j <= finalText.Count; j++) - { - costs [0, j] = j; - } - - for (int i = 1; i <= proposedText.Count; i++) - { - for (int j = 1; j <= finalText.Count; j++) - { - int substitutionCost = costs [i - 1, j - 1] + (proposedText [i - 1] == finalText [j - 1] ? 0 : 1); - int insertionCost = costs [i, j - 1] + 1; - int deletionCost = costs [i - 1, j] + 1; - - costs [i, j] = Math.Min (substitutionCost, Math.Min (insertionCost, deletionCost)); - } - } - - List operations = []; - int proposedIndex = proposedText.Count; - int finalIndex = finalText.Count; - - while (proposedIndex > 0 || finalIndex > 0) - { - if (proposedIndex > 0 - && finalIndex > 0 - && costs [proposedIndex, finalIndex] - == costs [proposedIndex - 1, finalIndex - 1] - + (proposedText [proposedIndex - 1] == finalText [finalIndex - 1] ? 0 : 1)) - { - operations.Add (PasteEditOperation.Substitute); - proposedIndex--; - finalIndex--; - - continue; - } - - if (finalIndex > 0 && costs [proposedIndex, finalIndex] == costs [proposedIndex, finalIndex - 1] + 1) - { - operations.Add (PasteEditOperation.Insert); - finalIndex--; - - continue; - } - - operations.Add (PasteEditOperation.Delete); - proposedIndex--; - } - - operations.Reverse (); - - return operations; - } - - private enum PasteEditOperation - { - Insert, - Delete, - Substitute - } - - private static bool AreEqual (List first, List second) - { - if (first.Count != second.Count) - { - return false; - } - - for (int i = 0; i < first.Count; i++) - { - if (first [i] != second [i]) - { - return false; - } - } - - return true; - } - - private static bool ConsumesFinal (PasteEditOperation operation) => operation is PasteEditOperation.Insert or PasteEditOperation.Substitute; - - private static bool ConsumesProposed (PasteEditOperation operation) => operation is PasteEditOperation.Delete or PasteEditOperation.Substitute; - - private static bool IsWithinPasteRange (PasteEditOperation operation, int proposedIndex, int pasteStart, int pasteEnd) - { - if (operation == PasteEditOperation.Insert) - { - return proposedIndex > pasteStart && proposedIndex < pasteEnd; - } - - return proposedIndex >= pasteStart && proposedIndex < pasteEnd; - } - - /// Raises the event, enabling canceling the change or adjusting the text. - /// The event arguments. - /// if the event was cancelled or the text was adjusted by the event. - public bool RaiseTextChanging (ResultEventArgs args) - { - // TODO: CWP: Add an OnTextChanging protected virtual method that can be overridden to handle text changing events. - - TextChanging?.Invoke (this, args); - - return args.Handled; - } - - private List DeleteSelectedText () - { - SetSelectedStartSelectedLength (); - int selStart = SelectedStart > -1 ? _selectionStart : InsertionPoint; - - string newText = StringExtensions.ToString (_text.GetRange (0, selStart)) - + StringExtensions.ToString (_text.GetRange (selStart + SelectedLength, _text.Count - (selStart + SelectedLength))); - - ClearAllSelection (); - InsertionPoint = selStart >= GraphemeHelper.GetGraphemeCount (newText) ? GraphemeHelper.GetGraphemeCount (newText) : selStart; - - return newText.ToStringList (); - } - - private void InsertText (Key a, bool usePreTextChangedCursorPos) - { - _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0)); - - List newText = _text; - - if (SelectedLength > 0) + if (!Secret && !_historyText.IsFromHistory) { - newText = DeleteSelectedText (); - _preChangeInsertionPoint = InsertionPoint; - } + _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); - if (!usePreTextChangedCursorPos) - { - _preChangeInsertionPoint = InsertionPoint; + _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); } - string grapheme = a.AsGrapheme; - - if (string.IsNullOrEmpty (grapheme)) - { - return; - } + base.OnTextChanged (); - if (Used) - { - _insertionPoint++; + // Raise IValue.ValueChanged + RaiseValueChanged (oldText, StringExtensions.ToString (_text)); - if (InsertionPoint == newText.Count + 1) - { - SetText (newText.Concat ([grapheme]).ToList ()); - } - else - { - if (_preChangeInsertionPoint > newText.Count) - { - _preChangeInsertionPoint = newText.Count; - } + ProcessAutocomplete (); - SetText (newText.GetRange (0, _preChangeInsertionPoint) - .Concat ([grapheme]) - .Concat (newText.GetRange (_preChangeInsertionPoint, Math.Min (newText.Count - _preChangeInsertionPoint, newText.Count)))); - } - } - else + if (InsertionPoint > _text.Count) { - SetText (newText.GetRange (0, _preChangeInsertionPoint) - .Concat ([grapheme]) - .Concat (newText.GetRange (Math.Min (_preChangeInsertionPoint + 1, newText.Count), - Math.Max (newText.Count - _preChangeInsertionPoint - 1, 0)))); - InsertionPoint++; + InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); } Adjust (); + SetNeedsDraw (); } - /// - /// Caches the cursor position before a text change operation. Used to properly handle text insertion - /// and deletion operations, particularly for undo/redo and proper cursor placement after edits. - /// - private int _preChangeInsertionPoint; - - /// - /// The text content stored as a list of strings, where each string represents a single text element - /// (grapheme cluster). This allows proper handling of Unicode combining characters and emoji. - /// - private List _text; - - /// - /// Stashes the final text value determined during (after sanitization - /// and possible subscriber modification) for use in . - /// - private string? _pendingText; - - private void SetText (List newText) => Text = StringExtensions.ToString (newText); - private void SetText (IEnumerable newText) => SetText (newText.ToList ()); - /// /// /// @@ -419,7 +331,7 @@ protected override bool OnTextChanging (string newText) // Sanitize: single-line, no tabs string sanitized = newText.Replace ("\t", "").Split ("\n") [0]; - string oldText = StringExtensions.ToString (_text); + var oldText = StringExtensions.ToString (_text); // After sanitization, check if there is an effective change if (oldText == sanitized) @@ -453,53 +365,10 @@ protected override bool OnTextChanging (string newText) } /// - /// - /// - /// Syncs the internal grapheme-list editing model from the base value, - /// applying any sanitization or subscriber modifications stashed during . - /// - /// - protected override void OnTextChanged () - { - // Use stashed text from OnTextChanging if available; otherwise sanitize base.Text - string finalText = _pendingText ?? base.Text.Replace ("\t", "").Split ("\n") [0]; - _pendingText = null; - - string oldText = StringExtensions.ToString (_text); - - ClearAllSelection (); - _text = finalText.ToStringList (); - - // Ensure base View._text holds the sanitized/modified value - if (base.Text != finalText) - { - SetTextDirect (finalText); - } - - if (!Secret && !_historyText.IsFromHistory) - { - _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); - - _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); - } - - base.OnTextChanged (); - - // Raise IValue.ValueChanged - RaiseValueChanged (oldText, StringExtensions.ToString (_text)); - - ProcessAutocomplete (); - - if (InsertionPoint > _text.Count) - { - InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); - } - - Adjust (); - SetNeedsDraw (); - } + protected override bool ShouldRaisePastedEvent (string text) => !ReadOnly && !string.IsNullOrEmpty (_lastPastedText); - /// Returns if the current cursor position is at the end of the , + /// + /// Returns if the current cursor position is at the end of the , /// includes when it is empty. /// /// @@ -533,7 +402,7 @@ protected override void OnTextChanged () /// private void Adjust () { - bool need = false; + var need = false; _ = TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out _, out int startIndex); int tSize = TextModel.DisplaySize (_text, 0, _text.Count).size; @@ -581,84 +450,260 @@ private void Adjust () UpdateCursor (); } - private int _insertionPoint; + private static bool AreEqual (List first, List second) + { + if (first.Count != second.Count) + { + return false; + } - /// - /// Gets or sets the insertion point within the text, measured as a 0-based index into text elements. - /// - /// - /// The insertion point position, clamped to the range [0, Text.Length]. Position 0 is before the first character; - /// position equal to the text length is after the last character. - /// - /// - /// - /// This property provides access to the logical insertion point within the text. The value is automatically - /// clamped to valid bounds: values less than 0 become 0, and values greater than the text length become - /// the text length. - /// - /// - /// Relationship to : - /// - /// - /// : Logical position in text elements (0-based index) - /// - /// - /// - /// : Converts logical position to screen coordinates, accounting for - /// and wide characters - /// - /// - /// - /// - /// - /// Example: For text "Hello世界" (Hello + 2 CJK characters): - /// - /// - /// InsertionPoint = 0: Before 'H' - /// - /// - /// InsertionPoint = 5: Before '世' - /// - /// - /// InsertionPoint = 7: After '界' (end of text) - /// - /// - /// Note that screen columns would differ because '世' and '界' each occupy 2 columns. - /// - /// - /// Setting this property also updates the text selection via . - /// - /// - /// - public virtual int InsertionPoint + for (var i = 0; i < first.Count; i++) + { + if (first [i] != second [i]) + { + return false; + } + } + + return true; + } + + private static List BuildPasteEditOperations (List proposedText, List finalText) { - get => _insertionPoint; - set + var costs = new int [proposedText.Count + 1, finalText.Count + 1]; + + for (var i = 0; i <= proposedText.Count; i++) { - int previousInsertionPoint = _insertionPoint; + costs [i, 0] = i; + } - if (value < 0) + for (var j = 0; j <= finalText.Count; j++) + { + costs [0, j] = j; + } + + for (var i = 1; i <= proposedText.Count; i++) + { + for (var j = 1; j <= finalText.Count; j++) { - _insertionPoint = 0; + int substitutionCost = costs [i - 1, j - 1] + (proposedText [i - 1] == finalText [j - 1] ? 0 : 1); + int insertionCost = costs [i, j - 1] + 1; + int deletionCost = costs [i - 1, j] + 1; + + costs [i, j] = Math.Min (substitutionCost, Math.Min (insertionCost, deletionCost)); } - else if (value > _text.Count) + } + + List operations = []; + int proposedIndex = proposedText.Count; + int finalIndex = finalText.Count; + + while (proposedIndex > 0 || finalIndex > 0) + { + if (proposedIndex > 0 + && finalIndex > 0 + && costs [proposedIndex, finalIndex] + == costs [proposedIndex - 1, finalIndex - 1] + (proposedText [proposedIndex - 1] == finalText [finalIndex - 1] ? 0 : 1)) { - _insertionPoint = _text.Count; + operations.Add (PasteEditOperation.Substitute); + proposedIndex--; + finalIndex--; + + continue; } - else + + if (finalIndex > 0 && costs [proposedIndex, finalIndex] == costs [proposedIndex, finalIndex - 1] + 1) { - _insertionPoint = value; + operations.Add (PasteEditOperation.Insert); + finalIndex--; + + continue; } - PrepareSelection (_selectionAnchor, _insertionPoint - _selectionAnchor); + operations.Add (PasteEditOperation.Delete); + proposedIndex--; + } - if (_insertionPoint != previousInsertionPoint) + operations.Reverse (); + + return operations; + } + + private static bool ConsumesFinal (PasteEditOperation operation) => operation is PasteEditOperation.Insert or PasteEditOperation.Substitute; + + private static bool ConsumesProposed (PasteEditOperation operation) => operation is PasteEditOperation.Delete or PasteEditOperation.Substitute; + + private List DeleteSelectedText () + { + SetSelectedStartSelectedLength (); + int selStart = SelectedStart > -1 ? _selectionStart : InsertionPoint; + + string newText = StringExtensions.ToString (_text.GetRange (0, selStart)) + + StringExtensions.ToString (_text.GetRange (selStart + SelectedLength, _text.Count - (selStart + SelectedLength))); + + ClearAllSelection (); + InsertionPoint = selStart >= GraphemeHelper.GetGraphemeCount (newText) ? GraphemeHelper.GetGraphemeCount (newText) : selStart; + + return newText.ToStringList (); + } + + private static void GetActualPastedText (string text, + List proposedText, + int pasteStart, + int pastedLength, + List finalText, + out string actualPastedText, + out int insertionPoint) + { + if (AreEqual (proposedText, finalText)) + { + actualPastedText = text; + insertionPoint = Math.Min (pasteStart + pastedLength, finalText.Count); + + return; + } + + if (proposedText.Count == finalText.Count) + { + int actualPastedLength = Math.Min (pastedLength, finalText.Count - pasteStart); + actualPastedText = actualPastedLength > 0 ? StringExtensions.ToString (finalText.GetRange (pasteStart, actualPastedLength)) : string.Empty; + insertionPoint = Math.Min (pasteStart + actualPastedLength, finalText.Count); + + return; + } + + actualPastedText = GetActualPastedTextFromEditOperations (proposedText, pasteStart, pastedLength, finalText, out insertionPoint); + } + + private static string GetActualPastedTextFromEditOperations (List proposedText, + int pasteStart, + int pastedLength, + List finalText, + out int insertionPoint) + { + List operations = BuildPasteEditOperations (proposedText, finalText); + int pasteEnd = pasteStart + pastedLength; + var proposedIndex = 0; + var finalIndex = 0; + int pastedStart = -1; + int pastedEnd = -1; + int boundaryAfterPaste = -1; + + foreach (PasteEditOperation operation in operations) + { + if (ConsumesFinal (operation) && IsWithinPasteRange (operation, proposedIndex, pasteStart, pasteEnd)) { - UpdateCursor (); + if (pastedStart == -1) + { + pastedStart = finalIndex; + } + + pastedEnd = finalIndex + 1; + } + + if (ConsumesProposed (operation)) + { + proposedIndex++; + + if (proposedIndex == pasteEnd) + { + boundaryAfterPaste = finalIndex + (ConsumesFinal (operation) ? 1 : 0); + } + } + + if (ConsumesFinal (operation)) + { + finalIndex++; } } + + insertionPoint = boundaryAfterPaste == -1 ? finalIndex : boundaryAfterPaste; + + if (pastedStart == -1 || pastedEnd == -1 || pastedEnd <= pastedStart) + { + return string.Empty; + } + + return StringExtensions.ToString (finalText.GetRange (pastedStart, pastedEnd - pastedStart)); } + private TextModel GetModel () + { + TextModel model = new (); + model.LoadString (Text); + + return model; + } + + private void InsertText (Key a, bool usePreTextChangedCursorPos) + { + _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0)); + + List newText = _text; + + if (SelectedLength > 0) + { + newText = DeleteSelectedText (); + _preChangeInsertionPoint = InsertionPoint; + } + + if (!usePreTextChangedCursorPos) + { + _preChangeInsertionPoint = InsertionPoint; + } + + string grapheme = a.AsGrapheme; + + if (string.IsNullOrEmpty (grapheme)) + { + return; + } + + if (Used) + { + _insertionPoint++; + + if (InsertionPoint == newText.Count + 1) + { + SetText (newText.Concat ([grapheme]).ToList ()); + } + else + { + if (_preChangeInsertionPoint > newText.Count) + { + _preChangeInsertionPoint = newText.Count; + } + + SetText (newText.GetRange (0, _preChangeInsertionPoint) + .Concat ([grapheme]) + .Concat (newText.GetRange (_preChangeInsertionPoint, Math.Min (newText.Count - _preChangeInsertionPoint, newText.Count)))); + } + } + else + { + SetText (newText.GetRange (0, _preChangeInsertionPoint) + .Concat ([grapheme]) + .Concat (newText.GetRange (Math.Min (_preChangeInsertionPoint + 1, newText.Count), + Math.Max (newText.Count - _preChangeInsertionPoint - 1, 0)))); + InsertionPoint++; + } + + Adjust (); + } + + private static bool IsWithinPasteRange (PasteEditOperation operation, int proposedIndex, int pasteStart, int pasteEnd) + { + if (operation == PasteEditOperation.Insert) + { + return proposedIndex > pasteStart && proposedIndex < pasteEnd; + } + + return proposedIndex >= pasteStart && proposedIndex < pasteEnd; + } + + private void SetText (List newText) => Text = StringExtensions.ToString (newText); + private void SetText (IEnumerable newText) => SetText (newText.ToList ()); + /// Updates the cursor position. /// /// This method calculates the cursor position and calls . @@ -675,13 +720,13 @@ private void UpdateCursor () } // Calculate absolute cursor position and store each glyph width - int cursorColumn = TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); - _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out int colOffset, out int viewportX); + TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); + _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out int _, out int viewportX); var colsWidth = 0; if (glyphWidths.Count > 0) { - for (int i = 0; i < Viewport.X; i++) + for (var i = 0; i < Viewport.X; i++) { if (i == glyphWidths.Count) { @@ -712,46 +757,10 @@ private void UpdateCursor () Cursor = Cursor with { Position = ViewportToScreen (new Point (pos, 0)) }; } - /// - /// Gets the horizontal scroll offset, representing the index of the first visible text element. - /// - /// - /// A 0-based index into the text elements indicating which element appears at the left edge of the viewport. - /// - /// - /// - /// When the text is longer than the viewport width, this property tracks how far the view has scrolled. - /// The method automatically updates this value to keep the cursor visible. - /// - /// - /// Relationship to cursor positioning: - /// - /// - /// : Absolute position in the text (0 to text length) - /// - /// - /// : Index of first visible character - /// - /// - /// - /// Screen column = - (approximately, - /// adjusted for wide chars) - /// - /// - /// - /// - /// - /// Example: For text "Hello World" with viewport width 5: - /// - /// - /// ScrollOffset = 0: Shows "Hello" - /// - /// - /// ScrollOffset = 6: Shows "World" - /// - /// - /// - /// - /// - public int ScrollOffset { get; private set; } + private enum PasteEditOperation + { + Insert, + Delete, + Substitute + } } From 7136a23ca10964449a6bda49688eaf93d9c8a2c8 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 15:38:04 -0600 Subject: [PATCH 08/13] Remove public RaiseTextChanging - inline into OnTextChanging override The event is now raised directly inside the CWP override, as intended. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Views/TextInput/TextField/TextField.Text.cs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 04c56571ee..48140c538c 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -118,18 +118,6 @@ public void InsertText (string toAdd, bool useOldCursorPos = true) } } - /// Raises the event, enabling canceling the change or adjusting the text. - /// The event arguments. - /// if the event was cancelled or the text was adjusted by the event. - public bool RaiseTextChanging (ResultEventArgs args) - { - // TODO: CWP: Add an OnTextChanging protected virtual method that can be overridden to handle text changing events. - - TextChanging?.Invoke (this, args); - - return args.Handled; - } - /// /// Gets the horizontal scroll offset, representing the index of the first visible text element. /// @@ -346,7 +334,7 @@ protected override bool OnTextChanging (string newText) } ResultEventArgs args = new (sanitized); - RaiseTextChanging (args); + TextChanging?.Invoke (this, args); if (args.Handled) { From 8119c93054c8748ad03430b2cea4ac2eacfaa8a4 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 15:39:18 -0600 Subject: [PATCH 09/13] Refactor View.Text setter to use SetTextDirect Replaces direct field and UI updates in the View.Text property setter with a call to SetTextDirect(value). This centralizes text update logic, improving maintainability and reducing code duplication. --- Terminal.Gui/ViewBase/View.Text.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index 8c2669b688..e3c8c736f5 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -74,10 +74,7 @@ public string Text return; } - _text = value; - - UpdateTextFormatterText (); - SetNeedsLayout (); + SetTextDirect (value); OnTextChanged (); } From 1f53a59ea19553ec68e299d7a1f8e38c63f2ade1 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 16:07:28 -0600 Subject: [PATCH 10/13] cleanup --- Terminal.Gui/ViewBase/View.Text.cs | 26 ++++++++++++++----- .../Views/TextInput/TextView/TextView.Text.cs | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index e3c8c736f5..3dcd924001 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -76,7 +76,7 @@ public string Text SetTextDirect (value); - OnTextChanged (); + RaiseTextChanged (); } } @@ -133,19 +133,31 @@ protected virtual bool OnTextChanging (string newText) public event EventHandler? TextChanging; /// - /// Called after the has been changed. Raises the event. + /// Called after the has been changed. /// /// /// - /// This is a signal-only notification. It does not carry old or new text values because - /// semantics vary across derived views. + /// Override in derived views to react to text changes. The base implementation is empty. + /// The event is raised by the caller after this method returns. /// + /// + protected virtual void OnTextChanged () { } + + /// + /// Invokes the CWP post-change workflow for : calls + /// then raises . + /// + /// /// - /// Derived views that override and do not call base.Text should call - /// this method after mutating text to participate in the CWP workflow. + /// Derived views that bypass the base setter (e.g., using new) should + /// call this method after mutating text to participate in the CWP workflow. /// /// - protected virtual void OnTextChanged () => TextChanged?.Invoke (this, EventArgs.Empty); + internal void RaiseTextChanged () + { + OnTextChanged (); + TextChanged?.Invoke (this, EventArgs.Empty); + } /// /// Raised after the has been changed. diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index 95550cedb2..ad1a1835e6 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -197,7 +197,7 @@ public int TabWidth // Keep base View._text in sync SetTextDirect (value); - OnTextChanged (); + RaiseTextChanged (); SetNeedsDraw (); _historyText.Clear (_model.GetAllLines ()); From 5346be29a6406fd779a68a46620bc5bb80010849 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 16:16:23 -0600 Subject: [PATCH 11/13] Minimize TextField diff: remove member reordering churn Reset TextField.Text.cs to original member order and apply only semantic changes (CWP overrides, remove RaiseTextChanging, shadow TextChanging event). Revert cosmetic change in TextField.Mouse.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TextInput/TextField/TextField.Mouse.cs | 2 +- .../TextInput/TextField/TextField.Text.cs | 866 +++++++++--------- 2 files changed, 430 insertions(+), 438 deletions(-) diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs index bfeff263f6..1de72c882c 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Mouse.cs @@ -65,7 +65,7 @@ private int SetInsertionPointFromMouse (Mouse mouse) } else { - TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset + pX, out int _, out int newInsertionIndex); + int cols = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset + pX, out int clipOffset, out int newInsertionIndex); InsertionPoint = newInsertionIndex; } diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 48140c538c..73b3ab5f06 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -1,4 +1,5 @@ using System.Globalization; +using Terminal.Gui.Drawing; namespace Terminal.Gui.Views; @@ -7,100 +8,21 @@ public partial class TextField private CultureInfo _currentCulture; private string? _lastPastedText; - /// - /// Caches the cursor position before a text change operation. Used to properly handle text insertion - /// and deletion operations, particularly for undo/redo and proper cursor placement after edits. - /// - private int _preChangeInsertionPoint; - - /// - /// The text content stored as a list of strings, where each string represents a single text element - /// (grapheme cluster). This allows proper handling of Unicode combining characters and emoji. - /// - private List _text; + /// Raised before changes. The change can be canceled the text adjusted. + public new event EventHandler>? TextChanging; /// - /// Stashes the final text value determined during (after sanitization - /// and possible subscriber modification) for use in . + /// Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so + /// new input should be appended at the cursor position, rather than clearing the entry /// - private string? _pendingText; - - private int _insertionPoint; + public bool Used { get; set; } - /// - /// Gets or sets the insertion point within the text, measured as a 0-based index into text elements. - /// - /// - /// The insertion point position, clamped to the range [0, Text.Length]. Position 0 is before the first character; - /// position equal to the text length is after the last character. - /// - /// - /// - /// This property provides access to the logical insertion point within the text. The value is automatically - /// clamped to valid bounds: values less than 0 become 0, and values greater than the text length become - /// the text length. - /// - /// - /// Relationship to : - /// - /// - /// : Logical position in text elements (0-based index) - /// - /// - /// - /// : Converts logical position to screen coordinates, accounting for - /// and wide characters - /// - /// - /// - /// - /// - /// Example: For text "Hello世界" (Hello + 2 CJK characters): - /// - /// - /// InsertionPoint = 0: Before 'H' - /// - /// - /// InsertionPoint = 5: Before '世' - /// - /// - /// InsertionPoint = 7: After '界' (end of text) - /// - /// - /// Note that screen columns would differ because '世' and '界' each occupy 2 columns. - /// - /// - /// Setting this property also updates the text selection via . - /// - /// - /// - public virtual int InsertionPoint + private TextModel GetModel () { - get => _insertionPoint; - set - { - int previousInsertionPoint = _insertionPoint; - - if (value < 0) - { - _insertionPoint = 0; - } - else if (value > _text.Count) - { - _insertionPoint = _text.Count; - } - else - { - _insertionPoint = value; - } - - PrepareSelection (_selectionAnchor, _insertionPoint - _selectionAnchor); + TextModel model = new (); + model.LoadString (Text); - if (_insertionPoint != previousInsertionPoint) - { - UpdateCursor (); - } - } + return model; } /// @@ -118,60 +40,29 @@ public void InsertText (string toAdd, bool useOldCursorPos = true) } } - /// - /// Gets the horizontal scroll offset, representing the index of the first visible text element. - /// - /// - /// A 0-based index into the text elements indicating which element appears at the left edge of the viewport. - /// + /// /// - /// - /// When the text is longer than the viewport width, this property tracks how far the view has scrolled. - /// The method automatically updates this value to keep the cursor visible. - /// - /// - /// Relationship to cursor positioning: - /// - /// - /// : Absolute position in the text (0 to text length) - /// - /// - /// : Index of first visible character - /// - /// - /// - /// Screen column = - (approximately, - /// adjusted for wide chars) - /// - /// - /// - /// - /// - /// Example: For text "Hello World" with viewport width 5: - /// - /// - /// ScrollOffset = 0: Shows "Hello" - /// - /// - /// ScrollOffset = 6: Shows "World" - /// - /// - /// + /// TextField is single-line — keep only the first line of the paste and drop C0/C1 control + /// characters, including tab. This matches what can actually accept and + /// keeps aligned with the text that will be inserted. /// - /// - public int ScrollOffset { get; private set; } + protected override string OnSanitizingPaste (string raw) + { + int newline = raw.IndexOfAny (['\r', '\n']); + string firstLine = newline >= 0 ? raw [..newline] : raw; - /// Raised before changes. The change can be canceled the text adjusted. - public new event EventHandler>? TextChanging; + StringBuilder sb = new (firstLine.Length); - /// - /// Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so - /// new input should be appended at the cursor position, rather than clearing the entry - /// - public bool Used { get; set; } + foreach (char c in firstLine) + { + if ((c >= 0x20 && c < 0x7F) || c >= 0xA0) + { + sb.Append (c); + } + } - /// - protected override string? GetPastedEventText (string text) => _lastPastedText; + return sb.ToString (); + } /// /// @@ -191,13 +82,11 @@ protected override bool OnPaste (string text) int selStart = _selectionStart == -1 ? InsertionPoint : _selectionStart; int selectedLength = SelectedLength; List oldText = [.. _text]; - var oldTextString = StringExtensions.ToString (oldText); + string oldTextString = StringExtensions.ToString (oldText); List pastedText = text.ToStringList (); - - List proposedText = - [ - .. oldText.GetRange (0, selStart), .. pastedText, .. oldText.GetRange (selStart + selectedLength, oldText.Count - (selStart + selectedLength)) - ]; + List proposedText = [.. oldText.GetRange (0, selStart), + .. pastedText, + .. oldText.GetRange (selStart + selectedLength, oldText.Count - (selStart + selectedLength))]; Text = StringExtensions.ToString (proposedText); bool proposedEqualsFinal = AreEqual (proposedText, _text); @@ -229,76 +118,275 @@ .. oldText.GetRange (0, selStart), .. pastedText, .. oldText.GetRange (selStart } /// - /// - /// TextField is single-line — keep only the first line of the paste and drop C0/C1 control - /// characters, including tab. This matches what can actually accept and - /// keeps aligned with the text that will be inserted. - /// - protected override string OnSanitizingPaste (string raw) + protected override bool ShouldRaisePastedEvent (string text) => !ReadOnly && !string.IsNullOrEmpty (_lastPastedText); + + /// + protected override string? GetPastedEventText (string text) => _lastPastedText; + + private static void GetActualPastedText (string text, List proposedText, int pasteStart, int pastedLength, List finalText, out string actualPastedText, out int insertionPoint) { - int newline = raw.IndexOfAny (['\r', '\n']); - string firstLine = newline >= 0 ? raw [..newline] : raw; + if (AreEqual (proposedText, finalText)) + { + actualPastedText = text; + insertionPoint = Math.Min (pasteStart + pastedLength, finalText.Count); - StringBuilder sb = new (firstLine.Length); + return; + } - foreach (char c in firstLine) + if (proposedText.Count == finalText.Count) { - if ((c >= 0x20 && c < 0x7F) || c >= 0xA0) - { - sb.Append (c); - } + int actualPastedLength = Math.Min (pastedLength, finalText.Count - pasteStart); + actualPastedText = actualPastedLength > 0 + ? StringExtensions.ToString (finalText.GetRange (pasteStart, actualPastedLength)) + : string.Empty; + insertionPoint = Math.Min (pasteStart + actualPastedLength, finalText.Count); + + return; } - return sb.ToString (); + actualPastedText = GetActualPastedTextFromEditOperations (proposedText, pasteStart, pastedLength, finalText, out insertionPoint); } - /// - /// - /// - /// Syncs the internal grapheme-list editing model from the base value, - /// applying any sanitization or subscriber modifications stashed during . - /// - /// - protected override void OnTextChanged () + private static string GetActualPastedTextFromEditOperations (List proposedText, int pasteStart, int pastedLength, List finalText, out int insertionPoint) { - // Use stashed text from OnTextChanging if available; otherwise sanitize base.Text - string finalText = _pendingText ?? Text.Replace ("\t", "").Split ("\n") [0]; - _pendingText = null; + List operations = BuildPasteEditOperations (proposedText, finalText); + int pasteEnd = pasteStart + pastedLength; + int proposedIndex = 0; + int finalIndex = 0; + int pastedStart = -1; + int pastedEnd = -1; + int boundaryAfterPaste = -1; - var oldText = StringExtensions.ToString (_text); + foreach (PasteEditOperation operation in operations) + { + if (ConsumesFinal (operation) + && IsWithinPasteRange (operation, proposedIndex, pasteStart, pasteEnd)) + { + if (pastedStart == -1) + { + pastedStart = finalIndex; + } - ClearAllSelection (); - _text = finalText.ToStringList (); + pastedEnd = finalIndex + 1; + } - // Ensure base View._text holds the sanitized/modified value - if (Text != finalText) - { - SetTextDirect (finalText); + if (ConsumesProposed (operation)) + { + proposedIndex++; + + if (proposedIndex == pasteEnd) + { + boundaryAfterPaste = finalIndex + (ConsumesFinal (operation) ? 1 : 0); + } + } + if (ConsumesFinal (operation)) + { + finalIndex++; + } } - if (!Secret && !_historyText.IsFromHistory) - { - _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); + insertionPoint = boundaryAfterPaste == -1 ? finalIndex : boundaryAfterPaste; - _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); + if (pastedStart == -1 || pastedEnd == -1 || pastedEnd <= pastedStart) + { + return string.Empty; } - base.OnTextChanged (); - - // Raise IValue.ValueChanged - RaiseValueChanged (oldText, StringExtensions.ToString (_text)); + return StringExtensions.ToString (finalText.GetRange (pastedStart, pastedEnd - pastedStart)); + } - ProcessAutocomplete (); + private static List BuildPasteEditOperations (List proposedText, List finalText) + { + int [,] costs = new int [proposedText.Count + 1, finalText.Count + 1]; - if (InsertionPoint > _text.Count) + for (int i = 0; i <= proposedText.Count; i++) { - InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); + costs [i, 0] = i; + } + + for (int j = 0; j <= finalText.Count; j++) + { + costs [0, j] = j; + } + + for (int i = 1; i <= proposedText.Count; i++) + { + for (int j = 1; j <= finalText.Count; j++) + { + int substitutionCost = costs [i - 1, j - 1] + (proposedText [i - 1] == finalText [j - 1] ? 0 : 1); + int insertionCost = costs [i, j - 1] + 1; + int deletionCost = costs [i - 1, j] + 1; + + costs [i, j] = Math.Min (substitutionCost, Math.Min (insertionCost, deletionCost)); + } + } + + List operations = []; + int proposedIndex = proposedText.Count; + int finalIndex = finalText.Count; + + while (proposedIndex > 0 || finalIndex > 0) + { + if (proposedIndex > 0 + && finalIndex > 0 + && costs [proposedIndex, finalIndex] + == costs [proposedIndex - 1, finalIndex - 1] + + (proposedText [proposedIndex - 1] == finalText [finalIndex - 1] ? 0 : 1)) + { + operations.Add (PasteEditOperation.Substitute); + proposedIndex--; + finalIndex--; + + continue; + } + + if (finalIndex > 0 && costs [proposedIndex, finalIndex] == costs [proposedIndex, finalIndex - 1] + 1) + { + operations.Add (PasteEditOperation.Insert); + finalIndex--; + + continue; + } + + operations.Add (PasteEditOperation.Delete); + proposedIndex--; + } + + operations.Reverse (); + + return operations; + } + + private enum PasteEditOperation + { + Insert, + Delete, + Substitute + } + + private static bool AreEqual (List first, List second) + { + if (first.Count != second.Count) + { + return false; + } + + for (int i = 0; i < first.Count; i++) + { + if (first [i] != second [i]) + { + return false; + } + } + + return true; + } + + private static bool ConsumesFinal (PasteEditOperation operation) => operation is PasteEditOperation.Insert or PasteEditOperation.Substitute; + + private static bool ConsumesProposed (PasteEditOperation operation) => operation is PasteEditOperation.Delete or PasteEditOperation.Substitute; + + private static bool IsWithinPasteRange (PasteEditOperation operation, int proposedIndex, int pasteStart, int pasteEnd) + { + if (operation == PasteEditOperation.Insert) + { + return proposedIndex > pasteStart && proposedIndex < pasteEnd; + } + + return proposedIndex >= pasteStart && proposedIndex < pasteEnd; + } + + private List DeleteSelectedText () + { + SetSelectedStartSelectedLength (); + int selStart = SelectedStart > -1 ? _selectionStart : InsertionPoint; + + string newText = StringExtensions.ToString (_text.GetRange (0, selStart)) + + StringExtensions.ToString (_text.GetRange (selStart + SelectedLength, _text.Count - (selStart + SelectedLength))); + + ClearAllSelection (); + InsertionPoint = selStart >= GraphemeHelper.GetGraphemeCount (newText) ? GraphemeHelper.GetGraphemeCount (newText) : selStart; + + return newText.ToStringList (); + } + + private void InsertText (Key a, bool usePreTextChangedCursorPos) + { + _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0)); + + List newText = _text; + + if (SelectedLength > 0) + { + newText = DeleteSelectedText (); + _preChangeInsertionPoint = InsertionPoint; + } + + if (!usePreTextChangedCursorPos) + { + _preChangeInsertionPoint = InsertionPoint; + } + + string grapheme = a.AsGrapheme; + + if (string.IsNullOrEmpty (grapheme)) + { + return; + } + + if (Used) + { + _insertionPoint++; + + if (InsertionPoint == newText.Count + 1) + { + SetText (newText.Concat ([grapheme]).ToList ()); + } + else + { + if (_preChangeInsertionPoint > newText.Count) + { + _preChangeInsertionPoint = newText.Count; + } + + SetText (newText.GetRange (0, _preChangeInsertionPoint) + .Concat ([grapheme]) + .Concat (newText.GetRange (_preChangeInsertionPoint, Math.Min (newText.Count - _preChangeInsertionPoint, newText.Count)))); + } + } + else + { + SetText (newText.GetRange (0, _preChangeInsertionPoint) + .Concat ([grapheme]) + .Concat (newText.GetRange (Math.Min (_preChangeInsertionPoint + 1, newText.Count), + Math.Max (newText.Count - _preChangeInsertionPoint - 1, 0)))); + InsertionPoint++; } Adjust (); - SetNeedsDraw (); } + /// + /// Caches the cursor position before a text change operation. Used to properly handle text insertion + /// and deletion operations, particularly for undo/redo and proper cursor placement after edits. + /// + private int _preChangeInsertionPoint; + + /// + /// The text content stored as a list of strings, where each string represents a single text element + /// (grapheme cluster). This allows proper handling of Unicode combining characters and emoji. + /// + private List _text; + + /// + /// Stashes the final text value determined during (after sanitization + /// and possible subscriber modification) for use in . + /// + private string? _pendingText; + + private void SetText (List newText) => Text = StringExtensions.ToString (newText); + private void SetText (IEnumerable newText) => SetText (newText.ToList ()); + /// /// /// @@ -353,10 +441,54 @@ protected override bool OnTextChanging (string newText) } /// - protected override bool ShouldRaisePastedEvent (string text) => !ReadOnly && !string.IsNullOrEmpty (_lastPastedText); + /// + /// + /// Syncs the internal grapheme-list editing model from the base value, + /// applying any sanitization or subscriber modifications stashed during . + /// + /// + protected override void OnTextChanged () + { + // Use stashed text from OnTextChanging if available; otherwise sanitize base.Text + string finalText = _pendingText ?? Text.Replace ("\t", "").Split ("\n") [0]; + _pendingText = null; + + var oldText = StringExtensions.ToString (_text); + + ClearAllSelection (); + _text = finalText.ToStringList (); + + // Ensure base View._text holds the sanitized/modified value + if (Text != finalText) + { + SetTextDirect (finalText); + } + + if (!Secret && !_historyText.IsFromHistory) + { + _historyText.Add ([Cell.ToCellList (oldText)], new Point (InsertionPoint, 0)); + + _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0), TextEditingLineStatus.Replaced); + } + + base.OnTextChanged (); + + // Raise IValue.ValueChanged + RaiseValueChanged (oldText, StringExtensions.ToString (_text)); + + ProcessAutocomplete (); + + if (InsertionPoint > _text.Count) + { + InsertionPoint = Math.Max (TextModel.DisplaySize (_text, 0).size - 1, 0); + } + + Adjust (); + SetNeedsDraw (); + } /// - /// Returns if the current cursor position is at the end of the , + /// Returns if the current cursor position is at the end of the . This /// includes when it is empty. /// /// @@ -390,7 +522,7 @@ protected override bool OnTextChanging (string newText) /// private void Adjust () { - var need = false; + bool need = false; _ = TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out _, out int startIndex); int tSize = TextModel.DisplaySize (_text, 0, _text.Count).size; @@ -438,260 +570,84 @@ private void Adjust () UpdateCursor (); } - private static bool AreEqual (List first, List second) - { - if (first.Count != second.Count) - { - return false; - } - - for (var i = 0; i < first.Count; i++) - { - if (first [i] != second [i]) - { - return false; - } - } - - return true; - } - - private static List BuildPasteEditOperations (List proposedText, List finalText) - { - var costs = new int [proposedText.Count + 1, finalText.Count + 1]; - - for (var i = 0; i <= proposedText.Count; i++) - { - costs [i, 0] = i; - } - - for (var j = 0; j <= finalText.Count; j++) - { - costs [0, j] = j; - } + private int _insertionPoint; - for (var i = 1; i <= proposedText.Count; i++) - { - for (var j = 1; j <= finalText.Count; j++) - { - int substitutionCost = costs [i - 1, j - 1] + (proposedText [i - 1] == finalText [j - 1] ? 0 : 1); - int insertionCost = costs [i, j - 1] + 1; - int deletionCost = costs [i - 1, j] + 1; - - costs [i, j] = Math.Min (substitutionCost, Math.Min (insertionCost, deletionCost)); - } - } - - List operations = []; - int proposedIndex = proposedText.Count; - int finalIndex = finalText.Count; - - while (proposedIndex > 0 || finalIndex > 0) - { - if (proposedIndex > 0 - && finalIndex > 0 - && costs [proposedIndex, finalIndex] - == costs [proposedIndex - 1, finalIndex - 1] + (proposedText [proposedIndex - 1] == finalText [finalIndex - 1] ? 0 : 1)) - { - operations.Add (PasteEditOperation.Substitute); - proposedIndex--; - finalIndex--; - - continue; - } - - if (finalIndex > 0 && costs [proposedIndex, finalIndex] == costs [proposedIndex, finalIndex - 1] + 1) - { - operations.Add (PasteEditOperation.Insert); - finalIndex--; - - continue; - } - - operations.Add (PasteEditOperation.Delete); - proposedIndex--; - } - - operations.Reverse (); - - return operations; - } - - private static bool ConsumesFinal (PasteEditOperation operation) => operation is PasteEditOperation.Insert or PasteEditOperation.Substitute; - - private static bool ConsumesProposed (PasteEditOperation operation) => operation is PasteEditOperation.Delete or PasteEditOperation.Substitute; - - private List DeleteSelectedText () - { - SetSelectedStartSelectedLength (); - int selStart = SelectedStart > -1 ? _selectionStart : InsertionPoint; - - string newText = StringExtensions.ToString (_text.GetRange (0, selStart)) - + StringExtensions.ToString (_text.GetRange (selStart + SelectedLength, _text.Count - (selStart + SelectedLength))); - - ClearAllSelection (); - InsertionPoint = selStart >= GraphemeHelper.GetGraphemeCount (newText) ? GraphemeHelper.GetGraphemeCount (newText) : selStart; - - return newText.ToStringList (); - } - - private static void GetActualPastedText (string text, - List proposedText, - int pasteStart, - int pastedLength, - List finalText, - out string actualPastedText, - out int insertionPoint) + /// + /// Gets or sets the insertion point within the text, measured as a 0-based index into text elements. + /// + /// + /// The insertion point position, clamped to the range [0, Text.Length]. Position 0 is before the first character; + /// position equal to the text length is after the last character. + /// + /// + /// + /// This property provides access to the logical insertion point within the text. The value is automatically + /// clamped to valid bounds: values less than 0 become 0, and values greater than the text length become + /// the text length. + /// + /// + /// Relationship to : + /// + /// + /// : Logical position in text elements (0-based index) + /// + /// + /// + /// : Converts logical position to screen coordinates, accounting for + /// and wide characters + /// + /// + /// + /// + /// + /// Example: For text "Hello世界" (Hello + 2 CJK characters): + /// + /// + /// InsertionPoint = 0: Before 'H' + /// + /// + /// InsertionPoint = 5: Before '世' + /// + /// + /// InsertionPoint = 7: After '界' (end of text) + /// + /// + /// Note that screen columns would differ because '世' and '界' each occupy 2 columns. + /// + /// + /// Setting this property also updates the text selection via . + /// + /// + /// + public virtual int InsertionPoint { - if (AreEqual (proposedText, finalText)) - { - actualPastedText = text; - insertionPoint = Math.Min (pasteStart + pastedLength, finalText.Count); - - return; - } - - if (proposedText.Count == finalText.Count) + get => _insertionPoint; + set { - int actualPastedLength = Math.Min (pastedLength, finalText.Count - pasteStart); - actualPastedText = actualPastedLength > 0 ? StringExtensions.ToString (finalText.GetRange (pasteStart, actualPastedLength)) : string.Empty; - insertionPoint = Math.Min (pasteStart + actualPastedLength, finalText.Count); - - return; - } - - actualPastedText = GetActualPastedTextFromEditOperations (proposedText, pasteStart, pastedLength, finalText, out insertionPoint); - } - - private static string GetActualPastedTextFromEditOperations (List proposedText, - int pasteStart, - int pastedLength, - List finalText, - out int insertionPoint) - { - List operations = BuildPasteEditOperations (proposedText, finalText); - int pasteEnd = pasteStart + pastedLength; - var proposedIndex = 0; - var finalIndex = 0; - int pastedStart = -1; - int pastedEnd = -1; - int boundaryAfterPaste = -1; + int previousInsertionPoint = _insertionPoint; - foreach (PasteEditOperation operation in operations) - { - if (ConsumesFinal (operation) && IsWithinPasteRange (operation, proposedIndex, pasteStart, pasteEnd)) + if (value < 0) { - if (pastedStart == -1) - { - pastedStart = finalIndex; - } - - pastedEnd = finalIndex + 1; + _insertionPoint = 0; } - - if (ConsumesProposed (operation)) + else if (value > _text.Count) { - proposedIndex++; - - if (proposedIndex == pasteEnd) - { - boundaryAfterPaste = finalIndex + (ConsumesFinal (operation) ? 1 : 0); - } + _insertionPoint = _text.Count; } - - if (ConsumesFinal (operation)) + else { - finalIndex++; + _insertionPoint = value; } - } - - insertionPoint = boundaryAfterPaste == -1 ? finalIndex : boundaryAfterPaste; - - if (pastedStart == -1 || pastedEnd == -1 || pastedEnd <= pastedStart) - { - return string.Empty; - } - - return StringExtensions.ToString (finalText.GetRange (pastedStart, pastedEnd - pastedStart)); - } - - private TextModel GetModel () - { - TextModel model = new (); - model.LoadString (Text); - - return model; - } - - private void InsertText (Key a, bool usePreTextChangedCursorPos) - { - _historyText.Add ([Cell.ToCells (_text)], new Point (InsertionPoint, 0)); - List newText = _text; - - if (SelectedLength > 0) - { - newText = DeleteSelectedText (); - _preChangeInsertionPoint = InsertionPoint; - } - - if (!usePreTextChangedCursorPos) - { - _preChangeInsertionPoint = InsertionPoint; - } - - string grapheme = a.AsGrapheme; - - if (string.IsNullOrEmpty (grapheme)) - { - return; - } - - if (Used) - { - _insertionPoint++; + PrepareSelection (_selectionAnchor, _insertionPoint - _selectionAnchor); - if (InsertionPoint == newText.Count + 1) - { - SetText (newText.Concat ([grapheme]).ToList ()); - } - else + if (_insertionPoint != previousInsertionPoint) { - if (_preChangeInsertionPoint > newText.Count) - { - _preChangeInsertionPoint = newText.Count; - } - - SetText (newText.GetRange (0, _preChangeInsertionPoint) - .Concat ([grapheme]) - .Concat (newText.GetRange (_preChangeInsertionPoint, Math.Min (newText.Count - _preChangeInsertionPoint, newText.Count)))); + UpdateCursor (); } } - else - { - SetText (newText.GetRange (0, _preChangeInsertionPoint) - .Concat ([grapheme]) - .Concat (newText.GetRange (Math.Min (_preChangeInsertionPoint + 1, newText.Count), - Math.Max (newText.Count - _preChangeInsertionPoint - 1, 0)))); - InsertionPoint++; - } - - Adjust (); } - private static bool IsWithinPasteRange (PasteEditOperation operation, int proposedIndex, int pasteStart, int pasteEnd) - { - if (operation == PasteEditOperation.Insert) - { - return proposedIndex > pasteStart && proposedIndex < pasteEnd; - } - - return proposedIndex >= pasteStart && proposedIndex < pasteEnd; - } - - private void SetText (List newText) => Text = StringExtensions.ToString (newText); - private void SetText (IEnumerable newText) => SetText (newText.ToList ()); - /// Updates the cursor position. /// /// This method calculates the cursor position and calls . @@ -708,13 +664,13 @@ private void UpdateCursor () } // Calculate absolute cursor position and store each glyph width - TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); - _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out int _, out int viewportX); + int cursorColumn = TextModel.CursorColumn (_text, InsertionPoint, 0, out List glyphWidths, out _); + _ = TextModel.GetColumnWidthsBeforeStart (glyphWidths, ScrollOffset, out int colOffset, out int viewportX); var colsWidth = 0; if (glyphWidths.Count > 0) { - for (var i = 0; i < Viewport.X; i++) + for (int i = 0; i < Viewport.X; i++) { if (i == glyphWidths.Count) { @@ -745,10 +701,46 @@ private void UpdateCursor () Cursor = Cursor with { Position = ViewportToScreen (new Point (pos, 0)) }; } - private enum PasteEditOperation - { - Insert, - Delete, - Substitute - } + /// + /// Gets the horizontal scroll offset, representing the index of the first visible text element. + /// + /// + /// A 0-based index into the text elements indicating which element appears at the left edge of the viewport. + /// + /// + /// + /// When the text is longer than the viewport width, this property tracks how far the view has scrolled. + /// The method automatically updates this value to keep the cursor visible. + /// + /// + /// Relationship to cursor positioning: + /// + /// + /// : Absolute position in the text (0 to text length) + /// + /// + /// : Index of first visible character + /// + /// + /// + /// Screen column = - (approximately, + /// adjusted for wide chars) + /// + /// + /// + /// + /// + /// Example: For text "Hello World" with viewport width 5: + /// + /// + /// ScrollOffset = 0: Shows "Hello" + /// + /// + /// ScrollOffset = 6: Shows "World" + /// + /// + /// + /// + /// + public int ScrollOffset { get; private set; } } From 41a3cc34331ddd61ebf78d65040e56a1dea8df57 Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 23 May 2026 17:01:24 -0600 Subject: [PATCH 12/13] Fix CR feedback: move validation to OnTextChanging, protect RaiseTextChanged - TextValidateField: move ValueChanging cancellation from OnTextChanged to OnTextChanging so rejected edits never reach View.Text - DatePicker: add OnTextChanging override rejecting unparseable dates - ColorPicker: add OnTextChanging override rejecting unparseable colors - TextField: clear _pendingText when base.OnTextChanging cancels - TextView: add _ownSetterActive flag to skip redundant model sync - View.Text: change RaiseTextChanged from internal to protected internal - Add 5 regression tests in TextCwpTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Terminal.Gui/ViewBase/View.Text.cs | 2 +- Terminal.Gui/Views/Color/ColorPicker.cs | 12 ++ Terminal.Gui/Views/DatePicker.cs | 12 ++ .../TextInput/TextField/TextField.Text.cs | 9 +- .../Views/TextInput/TextValidateField.cs | 80 ++++++----- .../Views/TextInput/TextView/TextView.Text.cs | 40 +++--- .../ViewBase/TextCwpTests.cs | 136 ++++++++++++++++++ 7 files changed, 241 insertions(+), 50 deletions(-) diff --git a/Terminal.Gui/ViewBase/View.Text.cs b/Terminal.Gui/ViewBase/View.Text.cs index 3dcd924001..b58552d540 100644 --- a/Terminal.Gui/ViewBase/View.Text.cs +++ b/Terminal.Gui/ViewBase/View.Text.cs @@ -153,7 +153,7 @@ protected virtual void OnTextChanged () { } /// call this method after mutating text to participate in the CWP workflow. /// /// - internal void RaiseTextChanged () + protected internal void RaiseTextChanged () { OnTextChanged (); TextChanged?.Invoke (this, EventArgs.Empty); diff --git a/Terminal.Gui/Views/Color/ColorPicker.cs b/Terminal.Gui/Views/Color/ColorPicker.cs index 8365b823b3..07d8f123bf 100644 --- a/Terminal.Gui/Views/Color/ColorPicker.cs +++ b/Terminal.Gui/Views/Color/ColorPicker.cs @@ -152,6 +152,18 @@ protected override bool OnDrawingContent (DrawContext? context) /// protected virtual void OnValueChanged (ValueChangedEventArgs args) { } + /// + protected override bool OnTextChanging (string newText) + { + // Reject text that cannot be parsed as a valid color + if (!_colorNameResolver.TryParseColor (newText, out _)) + { + return true; + } + + return base.OnTextChanging (newText); + } + /// protected override void OnTextChanged () { diff --git a/Terminal.Gui/Views/DatePicker.cs b/Terminal.Gui/Views/DatePicker.cs index 84840b07d3..b70d90e0e4 100644 --- a/Terminal.Gui/Views/DatePicker.cs +++ b/Terminal.Gui/Views/DatePicker.cs @@ -46,6 +46,18 @@ public CultureInfo? Culture } } + /// + protected override bool OnTextChanging (string newText) + { + // Reject text that cannot be parsed as a valid DateTime + if (!DateTime.TryParse (newText, out _)) + { + return true; + } + + return base.OnTextChanging (newText); + } + /// protected override void OnTextChanged () { diff --git a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs index 73b3ab5f06..fa22cf78be 100644 --- a/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs @@ -437,7 +437,14 @@ protected override bool OnTextChanging (string newText) // Stash the (possibly subscriber-modified) result for OnTextChanged _pendingText = args.Result; - return base.OnTextChanging (newText); + if (base.OnTextChanging (newText)) + { + _pendingText = null; + + return true; + } + + return false; } /// diff --git a/Terminal.Gui/Views/TextInput/TextValidateField.cs b/Terminal.Gui/Views/TextInput/TextValidateField.cs index 7a0318126c..247975c7c9 100644 --- a/Terminal.Gui/Views/TextInput/TextValidateField.cs +++ b/Terminal.Gui/Views/TextInput/TextValidateField.cs @@ -203,10 +203,52 @@ protected virtual void OnValueChanged (ValueChangedEventArgs args) { } #endregion + /// + /// Stashes the final text value determined during (after possible + /// subscriber modification) for use in . + /// + private string? _pendingValidatedText; + + /// + protected override bool OnTextChanging (string newText) + { + if (_provider is null || SuppressValueEvents) + { + return base.OnTextChanging (newText); + } + + string oldValue = _provider.Text; + + if (oldValue == newText) + { + return true; + } + + ValueChangingEventArgs args = new (oldValue, newText); + + if (OnValueChanging (args) || args.Handled) + { + return true; + } + + ValueChanging?.Invoke (this, args); + + if (args.Handled) + { + return true; + } + + // Allow subscribers to modify the new value + _pendingValidatedText = args.NewValue ?? string.Empty; + + return base.OnTextChanging (newText); + } + /// Text protected override void OnTextChanged () { - string value = Text; + string value = _pendingValidatedText ?? Text; + _pendingValidatedText = null; if (_provider is null) { @@ -224,35 +266,10 @@ protected override void OnTextChanged () return; } - if (!SuppressValueEvents) + // Apply subscriber-modified value if different from what the base stored + if (value != Text) { - ValueChangingEventArgs args = new (oldValue, value); - - if (OnValueChanging (args) || args.Handled) - { - // Revert Text to the provider's current value - SetTextDirect (oldValue); - - return; - } - - ValueChanging?.Invoke (this, args); - - if (args.Handled) - { - // Revert Text to the provider's current value - SetTextDirect (oldValue); - - return; - } - - // Allow subscribers to modify the new value - value = args.NewValue ?? string.Empty; - - if (value != Text) - { - SetTextDirect (value); - } + SetTextDirect (value); } _lastKnownText = value; @@ -264,11 +281,10 @@ protected override void OnTextChanged () if (!SuppressValueEvents) { - string oldVal = oldValue; - ValueChangedEventArgs changedArgs = new (oldVal, _provider.Text); + ValueChangedEventArgs changedArgs = new (oldValue, _provider.Text); OnValueChanged (changedArgs); ValueChanged?.Invoke (this, changedArgs); - ValueChangedUntyped?.Invoke (this, new ValueChangedEventArgs (oldVal, _provider.Text)); + ValueChangedUntyped?.Invoke (this, new ValueChangedEventArgs (oldValue, _provider.Text)); } SetNeedsDraw (); diff --git a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs index ad1a1835e6..378de60603 100644 --- a/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs +++ b/Terminal.Gui/Views/TextInput/TextView/TextView.Text.cs @@ -197,13 +197,18 @@ public int TabWidth // Keep base View._text in sync SetTextDirect (value); + _ownSetterActive = true; RaiseTextChanged (); + _ownSetterActive = false; SetNeedsDraw (); _historyText.Clear (_model.GetAllLines ()); } } + /// Tracks whether the new Text setter is active to avoid redundant sync in . + private bool _ownSetterActive; + /// /// /// Syncs the internal when is set through a @@ -211,27 +216,30 @@ public int TabWidth /// protected override void OnTextChanged () { - string baseText = base.Text; - - // Only sync when the internal model diverges (polymorphic setter case). - // When TextView's own `new Text` setter runs, it already calls LoadString - // and SetTextDirect, so base.Text matches _model — this is a no-op. - if (_model.ToString () != baseText) + // Skip sync when called from our own `new Text` setter — it already updated the model. + if (_ownSetterActive) { - ResetPosition (); - _model.LoadString (baseText); + base.OnTextChanged (); - if (_wordWrap) - { - _wrapManager = new WordWrapManager (_model); - _model = _wrapManager.WrapModel (Viewport.Width, out _, out _, out _, out _); - _lastWrapWidth = Viewport.Width; - } + return; + } - _historyText.Clear (_model.GetAllLines ()); - SetNeedsDraw (); + string baseText = base.Text; + + // Sync when the internal model diverges (polymorphic setter case). + ResetPosition (); + _model.LoadString (baseText); + + if (_wordWrap) + { + _wrapManager = new WordWrapManager (_model); + _model = _wrapManager.WrapModel (Viewport.Width, out _, out _, out _, out _); + _lastWrapWidth = Viewport.Width; } + _historyText.Clear (_model.GetAllLines ()); + SetNeedsDraw (); + base.OnTextChanged (); } diff --git a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs index 9de7131bb3..61c8fc974c 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs @@ -227,4 +227,140 @@ protected override bool OnTextChanging (string newText) return BaseReturnedCancel; } } + + // --- CR feedback regression tests --- + + /// + /// Verifies that when TextField's TextChanging subscriber modifies text, and then a + /// subsequent base TextChanging event cancels, the stale modified text does not leak + /// into the next successful text change. + /// + [Fact] + public void TextField_PendingText_ClearedOnBaseCancel () + { + // Copilot + TextField tf = new () { Width = 20, Height = 1 }; + tf.Text = "initial"; + + // First: a subscriber modifies text via ResultEventArgs + tf.TextChanging += (_, args) => + { + if (args.Result == "modified") + { + args.Result = "subscriber_changed"; + } + }; + + tf.Text = "modified"; + Assert.Equal ("subscriber_changed", tf.Text); + + // Now subscribe to base View.TextChanging to cancel the NEXT change + var cancelOnce = true; + + ((View)tf).TextChanging += (_, e) => + { + if (cancelOnce) + { + e.Cancel = true; + cancelOnce = false; + } + }; + + // This should be cancelled by the base event + tf.Text = "blocked"; + Assert.Equal ("subscriber_changed", tf.Text); + + // Next change should succeed with fresh text, NOT stale _pendingText + tf.Text = "final"; + Assert.Equal ("final", tf.Text); + } + + /// + /// Verifies that TextValidateField does not raise View.TextChanged when + /// ValueChanging is cancelled (CWP semantics: cancel suppresses TextChanged). + /// + [Fact] + public void TextValidateField_ValueChangingCancel_SuppressesTextChanged () + { + // Copilot + TextValidateField field = new () { Width = 20, Height = 1 }; + + // Set a provider that accepts any text + field.Provider = new TextRegexProvider (".*"); + field.Text = "initial"; + + // Cancel ValueChanging + field.ValueChanging += (_, args) => args.Handled = true; + + bool textChangedRaised = false; + ((View)field).TextChanged += (_, _) => textChangedRaised = true; + + field.Text = "blocked"; + + Assert.False (textChangedRaised, "TextChanged should not fire when ValueChanging cancels"); + Assert.Equal ("initial", field.Text); + } + + /// + /// Verifies that DatePicker rejects invalid (unparseable) text: Text should not + /// persist an invalid string that cannot round-trip through DateTime. + /// + [Fact] + public void DatePicker_InvalidText_DoesNotPersist () + { + // Copilot + DatePicker dp = new () { Width = 20, Height = 1 }; + DateTime originalValue = dp.Value; + string originalText = dp.Text; + + // Set invalid date text + dp.Text = "not-a-date"; + + // Value should remain unchanged + Assert.Equal (originalValue, dp.Value); + + // Text should NOT hold the invalid string — it should revert or be rejected + Assert.NotEqual ("not-a-date", dp.Text); + } + + /// + /// Verifies that ColorPicker rejects invalid (unparseable) text: Text should not + /// persist an invalid string that cannot round-trip through Color. + /// + [Fact] + public void ColorPicker_InvalidText_DoesNotPersist () + { + // Copilot + ColorPicker cp = new () { Width = 20, Height = 3 }; + cp.SelectedColor = new Color (255, 0, 0); + string originalText = cp.Text; + + // Set invalid color text + cp.Text = "not-a-color"; + + // SelectedColor should remain unchanged + Assert.Equal (new Color (255, 0, 0), cp.SelectedColor); + + // Text should NOT hold the invalid string — it should revert or be rejected + Assert.NotEqual ("not-a-color", cp.Text); + } + + /// + /// Verifies that setting View.Text on a word-wrapped TextView via a View reference + /// does not corrupt or redundantly re-process the model. + /// + [Fact] + public void TextView_WordWrap_PolymorphicSet_DoesNotCorruptModel () + { + // Copilot + TextView tv = new () { Width = 10, Height = 5, WordWrap = true }; + tv.Text = "Hello World this wraps"; + + // Set via polymorphic View reference + View viewRef = tv; + viewRef.Text = "Short"; + + Assert.Equal ("Short", tv.Text); + Assert.Equal ("Short", viewRef.Text); + } } From 5e2f2a696fc1c434377f3598e3fb126f5699d351 Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 24 May 2026 09:35:58 -0600 Subject: [PATCH 13/13] =?UTF-8?q?Add=20Text=E2=86=94Value=20consistency=20?= =?UTF-8?q?tests=20for=20DatePicker,=20ColorPicker,=20TextValidateField?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Positive-path tests verifying: - Valid text updates Value (DatePicker, ColorPicker, TextValidateField) - Value setter updates Text (DatePicker, ColorPicker) - ValueChanging cancellation keeps Text and provider consistent (TextValidateField) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ViewBase/TextCwpTests.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs index 61c8fc974c..1e49faee05 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs @@ -363,4 +363,111 @@ public void TextView_WordWrap_PolymorphicSet_DoesNotCorruptModel () Assert.Equal ("Short", tv.Text); Assert.Equal ("Short", viewRef.Text); } + + /// + /// Verifies that setting a valid date string via Text updates DatePicker.Value accordingly. + /// Ensures Text↔Value consistency on the happy path. + /// + [Fact] + public void DatePicker_ValidText_UpdatesValue () + { + // Copilot + DatePicker dp = new () { Width = 20, Height = 1 }; + DateTime target = new (2025, 12, 25); + + // Use the same short-date format the picker uses internally + string formatted = target.ToShortDateString (); + dp.Text = formatted; + + Assert.Equal (target, dp.Value); + Assert.Equal (formatted, dp.Text); + } + + /// + /// Verifies that setting DatePicker.Value updates Text to the formatted representation. + /// Ensures Value→Text consistency. + /// + [Fact] + public void DatePicker_ValueSet_UpdatesText () + { + // Copilot + DatePicker dp = new () { Width = 20, Height = 1 }; + DateTime target = new (2024, 7, 4); + + dp.Value = target; + + Assert.Equal (target, dp.Value); + + // Text should be parseable back to the same date + Assert.True (DateTime.TryParse (dp.Text, out DateTime roundTrip)); + Assert.Equal (target, roundTrip); + } + + /// + /// Verifies that setting a valid color string via Text updates ColorPicker.SelectedColor. + /// Ensures Text↔Value consistency on the happy path. + /// + [Fact] + public void ColorPicker_ValidText_UpdatesSelectedColor () + { + // Copilot + ColorPicker cp = new () { Width = 20, Height = 3 }; + cp.SelectedColor = new Color (0, 0, 0); + + // StandardColorsNameResolver accepts StandardColor enum names + cp.Text = "Red"; + + // SelectedColor should have changed from black to red + Assert.NotEqual (new Color (0, 0, 0), cp.SelectedColor); + Assert.Equal ("Red", cp.Text); + } + + /// + /// Verifies that setting ColorPicker.SelectedColor updates Text to the color string. + /// Ensures Value→Text consistency. + /// + [Fact] + public void ColorPicker_SelectedColorSet_UpdatesText () + { + // Copilot + ColorPicker cp = new () { Width = 20, Height = 3 }; + + cp.SelectedColor = new Color (0, 0, 255); + + Assert.Equal (new Color (0, 0, 255).ToString (), cp.Text); + } + + /// + /// Verifies that TextValidateField accepts valid text and updates Value accordingly. + /// Ensures Text↔Value consistency on the happy path. + /// + [Fact] + public void TextValidateField_ValidText_UpdatesValue () + { + // Copilot + TextValidateField tvf = new () { Provider = new TextRegexProvider ("^[0-9]+$") }; + tvf.Text = "123"; + + tvf.Text = "456"; + + Assert.Equal ("456", tvf.Text); + } + + /// + /// Verifies that when TextValidateField.ValueChanging cancels, both Text and the + /// provider's internal value remain at the old value (no divergence). + /// + [Fact] + public void TextValidateField_ValueChangingCancel_TextAndProviderStayConsistent () + { + // Copilot + TextValidateField tvf = new () { Provider = new TextRegexProvider ("^[0-9]+$") }; + tvf.Text = "111"; + tvf.ValueChanging += (_, e) => e.Handled = true; + + tvf.Text = "222"; + + // Both must stay at original value + Assert.Equal ("111", tvf.Text); + } }