search + find-and-replace: lift AvaloniaEdit engine, wire Editor + ted (R9 amendment)#76
Merged
Conversation
Bring ISearchStrategy, RegexSearchStrategy / SearchResult, and SearchStrategyFactory into src/Terminal.Gui.Text/Search/ per the AvaloniaEdit fork policy. Pinned commit d7a6b63. The lift provides what the bespoke string.IndexOf engine in Editor.FindReplace.cs cannot: - Regex search (RegexOptions.Multiline + IgnoreCase). - Whole-word matching via TextUtilities.GetNextCaretPosition. - Anchored results: SearchResult inherits TextSegment, so attaching results to a TextSegmentCollection<TextSegment>(document) makes offsets track edits automatically. - Range-restricted FindAll for incremental / viewport-scoped search. - A factory abstraction the find dialog can target instead of hand- written matching code. Per-call perf is roughly equivalent to the existing IndexOf path — RegexSearchStrategy still calls document.Text once per FindAll, same as the bespoke engine's IndexOf. The win is structural (one materialization per search instead of N per ReplaceAll) and capability-shaped (regex, whole-word, anchored results); a rope- walking matcher would be a separate, follow-on optimization. Modifications to lifted source are minimal: namespace transform, "Adapted for Terminal.Gui from AvaloniaEdit d7a6b63" header, and a single #nullable disable on RegexSearchStrategy.cs (upstream predates nullable reference types and the IEquatable<T>.Equals override, SearchResult.Data auto-property, and FindAll().FirstOrDefault() pattern all trip CS warnings under nullable enable). Folder-local .editorconfig marks the lift as generated_code so dotnet format preserves upstream formatting on re-syncs. All logged in UPSTREAM.md. 12 tests in tests/Terminal.Gui.Text.Tests/Search/ cover the six spec scenarios (case sensitive/insensitive, whole-word, regex, cross-line, anchored tracking via TextSegmentCollection) plus wildcard mode, replacement backreferences, factory equality, invalid-regex exception, and range-restricted FindAll. Editor + ted are not yet wired to the new engine — that ships with the find-and-replace PR, in line with the new constitution R9 amendment landing in the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strengthen constitution R9 to forbid pure-plumbing PRs: every AvaloniaEdit lift must include the Terminal.Gui.Editor integration and examples/ted wiring that exercises the lifted surface, in the same change. Split work horizontally (smaller user-visible slices), not vertically (model first, view later). The principle: a customer must be able to *experience* the new capability the moment the PR merges. Plan.md restructured accordingly. Lift-only feature rows (search, indentation, folding, syntax-highlighting) collapse into the end-to-end features that ship them to the user (find-and-replace, auto-indent, folding-ui, syntax-colorizer). The "Bundles" column makes the composition explicit so the lift step is visible in review but does not appear as a separate Ready item. The just-landed search lift exposes this gap — the AvaloniaEdit search engine is in place but Editor.FindReplace.cs still uses string.IndexOf and ted's FindReplaceDialog has no toggles for the new regex / whole-word capabilities. The next PR (find-and-replace) is the first one held to the amended R9. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace Editor.FindReplace.cs's bespoke document.Text.IndexOf engine with the lifted ISearchStrategy seam from PR #76. The string-based overloads (FindNext/FindPrevious/ReplaceNext/ReplaceAll taking a search-text string) remain as convenience wrappers: each builds a SearchMode.Normal strategy from its arguments and delegates to the no-argument property-driven overloads. Callers that want regex, whole-word, or wildcard mode assign their own ISearchStrategy to Editor.SearchStrategy (typically constructed via SearchStrategyFactory.Create) and then call the no-arg overloads. Why the engine swap is worth doing alone: - ReplaceAll on N matches now does ONE document.Text materialization (via SearchStrategy.FindAll) instead of N (one per IndexOf call in the legacy loop). Replacements iterate in reverse so offsets stay valid as later matches shrink/grow the document. Both paths still use Document.RunUpdate() for single-step undo (R5). - ReplaceNext routes through ISearchResult.ReplaceWith, picking up regex $1/$2 backreference substitution for free. - FindPrevious is now real backwards search via FindAll over [0, end) and LastOrDefault, replacing the LastIndexOf-with-clamped-start approach that mis-handled cursor-inside-a-match. Quick-find Stopwatch microbench (benchmarks/.../Program.cs --quick-find, DocSize=100KB): Matches New (ms) Old (ms) Speedup Memory ratio 10 0.53 0.58 ~equal 4.5x less 100 0.73 2.52 3.5x faster 32x less 1000 3.55 14.84 4.2x faster 103x less FindNext is roughly equal between engines (both materialize once); the win is concentrated in ReplaceAll where the legacy path's per- iteration rope materialization dominates GC pressure. specs/find-and-replace/spec.md updated to mark FR-001/002/004 done and FR-003 (SearchHitRenderer) / FR-005 (F3 / Ctrl+F / Ctrl+H keybindings) / FR-006 (hit-highlight invalidation) explicitly deferred to a follow-up slice. specs/public-api.md change log records Editor.SearchStrategy landing. 10 new tests in EditorFindReplaceTests.cs cover: property round-trip, FindNext returning false when strategy not set, regex match through Editor, whole-word match through Editor, FindPrevious finds the rightmost match before the caret, FindPrevious wraparound, regex backreference substitution via ReplaceNext, ReplaceAll via regex strategy, R5 single-step undo with regex, reverse-iteration safety when replacement length differs from match length. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add three CheckBox controls above the Find/Replace tab pane that together build the Editor.SearchStrategy for each Find / Replace action. The handlers route through a single TryBuildStrategy helper that constructs SearchStrategyFactory.Create from the toggle state, assigns to editor.SearchStrategy, and catches SearchPatternException to surface invalid-regex errors on a status label rather than silently failing. This is the ted-facing payoff for the engine swap in the previous commit and the R9 amendment in PR #76: a customer (the user of ted) can now exercise the new ISearchStrategy capabilities — regex \d{3}, whole-word "cat", case-sensitive match — without writing code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new benchmark paths in the benchmarks project: - FindBenchmarks.cs: BenchmarkDotNet [ShortRunJob] [MemoryDiagnoser] comparison of FindNext (new vs legacy) and ReplaceAll (new vs legacy) for the engine itself — no Editor wrapper, no event handlers, no visual-line caches. The Editor's per-edit notification cost is real but separate, and would apply to both engines once landed (PR #77 already fixes Editor's all-lines-walk on Document swap incrementally). - Program.cs --quick-find: lightweight Stopwatch microbench that prints the new-vs-old comparison in ~10 seconds. BDN's full statistical rigor is overkill for "is the new path faster or slower?" and adds 5-10 minutes per run due to sub-process spawns and pilot phases for the slow ReplaceAll cases. Sample output (DocSize=100KB, 20 iterations): Matches New (ms) Old (ms) Speedup Memory ratio 10 0.53 0.58 ~equal 4.5x less 100 0.73 2.52 3.5x faster 32x less 1000 3.55 14.84 4.2x faster 103x less The wart this lift fixed: the legacy engine materialized document.Text once per IndexOf call inside the ReplaceAll loop (N rope materializations at 100KB each = 191MB allocated for 1000 matches). The new engine calls FindAll once, materializes once, and iterates the cached result list in reverse. Per-call FindNext cost is roughly equal between engines — the incremental-search per- keystroke materialization remains and would need a rope-walking matcher to truly fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Lifts AvaloniaEdit’s search-engine abstractions into Terminal.Gui.Text to serve as the future seam for Terminal.Gui.Editor find/replace (regex, wildcard, whole-word), and updates project specs/logs to reflect the lift and the revised “end-to-end feature per PR” policy.
Changes:
- Added
Terminal.Gui.Text.Searchsearch engine surface (ISearchStrategy,RegexSearchStrategy/SearchResult,SearchStrategyFactory) plus folder-local.editorconfigto treat the lift as generated code. - Added
SearchStrategyTestscovering core scenarios (case sensitivity, whole-word, regex, wildcard, range, edit-tracking viaTextSegmentCollection). - Updated
third_party/AvaloniaEdit/UPSTREAM.mdand amendedspecs/plan.md/specs/constitution.md(R9 composition rule).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| third_party/AvaloniaEdit/UPSTREAM.md | Records the new Search/ lift and what was intentionally skipped. |
| src/Terminal.Gui.Text/Search/.editorconfig | Marks lifted search code as generated and suppresses analyzers/style. |
| src/Terminal.Gui.Text/Search/ISearchStrategy.cs | Introduces search interfaces, modes, and SearchPatternException. |
| src/Terminal.Gui.Text/Search/RegexSearchStrategy.cs | Implements regex-based searching + SearchResult. |
| src/Terminal.Gui.Text/Search/SearchStrategyFactory.cs | Adds factory for Normal/Wildcard/RegEx strategies. |
| tests/Terminal.Gui.Text.Tests/Search/SearchStrategyTests.cs | Adds unit tests for the lifted search surface. |
| specs/plan.md | Restructures plan around end-to-end user-visible features and bundles lifts. |
| specs/constitution.md | Amends R9 to require lifts ship with Editor + ted wiring in the same PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two correctness deviations from AvaloniaEdit d7a6b63, both surfaced by Copilot review on PR #76. Logged in third_party/AvaloniaEdit/UPSTREAM.md. 1. RegexSearchStrategy.Equals omitted _matchWholeWords. Two strategies that differ only by whole-word matching compared equal, which would break consumer caching / dedup (e.g. an Editor wanting to skip a re-search when the strategy hasn't actually changed). Add it to the chain. One-line change. 2. SearchStrategyFactory.Create accepted an empty pattern, which then compiles to a regex matching at every position — FindAll returns TextLength+1 zero-length results and ReplaceAll DoS's on it. ted's FindReplaceDialog already short-circuits on IsNullOrEmpty before calling Create, so the in-repo consumer was safe, but the factory is public and a third-party caller could trip it. Reject empty patterns with ArgumentException. Whitespace patterns remain legitimate (literal-space search in Normal mode, space-char match in Regex mode). Both deviations pinned by tests in SearchStrategyTests so a future re-sync from upstream that drops them will fail loudly. Issue #1 from the Copilot review (FindAll re-scanning the document prefix from offset 0 each call) is left to address upstream or as a separate fork-optimization issue — that one is a perf, not correctness, change to the matching algorithm itself and belongs out of this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tig
changed the title
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Originally the search lift alone; folded together with #79 (find-and-replace consumer migration) per CR feedback that R9 — as amended in this PR — forbids ship-the-lift-without-the-consumer. PR #79 was auto-merged into this branch when its commits became reachable here.
End-to-end feature slice:
ISearchStrategy,RegexSearchStrategy/SearchResult,SearchStrategyFactorylifted from AvaloniaEditd7a6b63intosrc/Terminal.Gui.Text/Search/. Folder-local.editorconfigmarks the lift asgenerated_codesodotnet formatpreserves upstream formatting. Single#nullable disableonRegexSearchStrategy.csfor the upstream pre-nullable shape. All modifications logged inthird_party/AvaloniaEdit/UPSTREAM.md.Editor.FindReplace.csnow drives all find/replace throughISearchStrategy.Editor.SearchStrategyis the seam. String-basedFindNext/FindPrevious/ReplaceNext/ReplaceAlloverloads kept as convenience wrappers (build aSearchMode.Normalstrategy + delegate).ReplaceAllusesFindAll+ reverse iteration under oneRunUpdate ()scope — one rope materialization instead of N, R5 single-step undo preserved.ReplaceNextroutes throughISearchResult.ReplaceWith, picking up regex$1/$2backreference substitution for free.FindReplaceDialogadds Match case / Whole word / Regex checkboxes + a status label that surfaces invalid-regex errors. Builds anISearchStrategyfrom toggle state on every Find / Replace action.specs/constitution.mdstrengthened: AvaloniaEdit lifts must ship with theirTerminal.Gui.Editor+examples/tedwiring in the same feature slice.specs/plan.mdrestructured around end-to-end features instead of lift-as-feature rows.RegexSearchStrategy.Equalsnow includes_matchWholeWords(upstream omitted it, so two strategies differing only by whole-word setting compared equal — breaks consumer caching/dedup); (b)SearchStrategyFactory.Createrejects empty patterns withArgumentException(upstream accepted them, compiling to a regex matching at every position —TextLength+1zero-length results, DoS inFindAll/ReplaceAll). Both pinned by tests.FindBenchmarks.cs(BDN) +Program.cs --quick-find(Stopwatch). NewReplaceAllis ~4× faster and ~100× less allocation than the legacyIndexOf-loop engine at N=1000 matches;FindNextis roughly equal.Quick-find benchmark (DocSize=100 KB, 20 iters)
Run with
dotnet run --project benchmarks/Terminal.Gui.Editor.Benchmarks -c Release -- --quick-find.The win comes from replacing N rope materializations (one per
IndexOfcall in the legacy loop) with 1 (oneFindAll).FindNextper-call cost is unchanged — both engines materializedocument.Textonce per call. The remaining "10 MB per keystroke" concern for incremental search needs a rope-walking matcher and is out of scope.Copilot CR response
FindAllre-scans from offset 0 on each callEqualsomits_matchWholeWordsd0dfa3f). Fork deviation logged inUPSTREAM.md, pinned by test.Createaccepts empty pattern → DoSd0dfa3f). Fork deviation logged inUPSTREAM.md, pinned by test.Validation
dotnet build Terminal.Gui.Text.slnx— 0 warnings, 0 errorsdotnet format --verify-no-changes— cleanTerminal.Gui.Text.Tests— 227 / 0 fail (15 inSearch/, of which 3 new for the fork deviations)Terminal.Gui.Editor.Tests— 88 / 0 fail (10 new inEditorFindReplaceTests.cs)Terminal.Gui.Editor.IntegrationTests— 105 / 0 failTest plan
dotnet run --project examples/ted. Ctrl+F. Search for\d+with Regex on — numbers select one at a time. Toggle Whole word + searchcatagainstcat catalog scatter— only the first selects.() surfaces "Regex error: ..." on the dialog status label.git log specs/find-and-replace/spec.mdshows FR-003 / FR-005 / FR-006 as the next slice (hit highlighting, F3 / Ctrl+F / Ctrl+H keybindings).🤖 Generated with Claude Code