Fixes #4497 - makes replacement char conifgurable #4498
Merged
+270
−54
Conversation
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
Introduced Glyphs.ReplacementChar to allow overriding the Unicode replacement character, defaulting to a space (' '). Updated both config.json and Glyphs.cs with this property, scoped to ThemeScope and documented as an override for Rune.ReplacementChar.
Replaced all uses of Rune.ReplacementChar.ToString() with Glyphs.ReplacementChar.ToString() in OutputBufferImpl and related tests. This ensures consistent use of the replacement character when invalidating or overwriting wide characters in the output buffer.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v2_develop #4498 +/- ##
==============================================
+ Coverage 77.45% 77.46% +0.01%
==============================================
Files 386 386
Lines 44678 44676 -2
Branches 6283 6281 -2
==============================================
+ Hits 34605 34610 +5
+ Misses 8220 8214 -6
+ Partials 1853 1852 -1
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Allows setting custom replacement characters for wide glyphs that cannot fit in the available space via IOutputBuffer.SetReplacementChars. Updated IDriver to expose GetOutputBuffer. All code paths and tests now use the configurable characters, improving testability and flexibility. Tests now use '①' and '②' for clarity instead of the default replacement character.
Collaborator
Author
|
This change is it points out that no existing test covers this code in 
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the replacement character configurable to address issue #4497 and enable parallel test isolation. Previously, the replacement character was hardcoded to Rune.ReplacementChar. Now it can be configured globally via Glyphs.ReplacementChar or per-buffer via SetReplacementChars().
Key changes:
- Added
Glyphs.ReplacementCharas a configurable static property with a default of space character - Added
SetReplacementChars()method toIOutputBufferfor per-buffer replacement character configuration - Added
GetOutputBuffer()method toIDriverinterface to access the output buffer - Updated all tests to use custom replacement characters (①, ②) for parallel test isolation
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Terminal.Gui/Drawing/Glyphs.cs | Adds configurable ReplacementChar property with default space character |
| Terminal.Gui/Drivers/IOutputBuffer.cs | Adds SetReplacementChars() method to interface |
| Terminal.Gui/Drivers/OutputBufferImpl.cs | Implements replacement character configuration with two separate chars for different scenarios |
| Terminal.Gui/Drivers/IDriver.cs | Adds GetOutputBuffer() method with documentation |
| Terminal.Gui/Drivers/DriverImpl.cs | Implements GetOutputBuffer() method |
| Terminal.Gui/ViewBase/View.Mouse.cs | Updates documentation references from Command.Select to Command.Activate (unrelated fix) |
| Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingClippingTests.cs | Updates tests to use custom replacement chars for parallel isolation |
| Tests/UnitTestsParallelizable/Drivers/OutputBufferWideCharTests.cs | Updates tests to use Glyphs.ReplacementChar and removes unused constructor parameter |
| Tests/UnitTestsParallelizable/Drivers/OutputBaseTests.cs | Updates tests to use custom replacement chars |
| Tests/UnitTestsParallelizable/Drivers/DriverTests.cs | Updates tests to use custom replacement chars |
| Tests/UnitTestsParallelizable/Drivers/AddRuneTests.cs | Updates tests to use custom replacement chars |
| Tests/UnitTests/View/Draw/ClipTests.cs | Updates tests to use custom replacement chars |
| Examples/UICatalog/Scenarios/WideGlyphs.cs | Adds null-forgiving operator and reformats code (unrelated cleanup) |
| Examples/UICatalog/Resources/config.json | Adds Glyphs.ReplacementChar configuration entry |
Co-authored-by: Copilot <[email protected]>
Added three unit tests to OutputBufferWideCharTests.cs to verify and document OutputBufferImpl's behavior when wide (double-width) characters are written at the edges of a clipping region. Tests cover cases where the first or second column of a wide character is outside the clip, as well as when both columns are inside. The tests assert correct use of replacement characters, dirty flags, and column advancement, and document that certain code paths are currently unreachable due to IsValidLocation checks.
…2_4497-ReplacementChar
Added comments to mark a rarely executed code path as dead code, noting it is apparently never called. Referenced the related test scenario AddStr_WideChar_FirstColumnOutsideClip_SecondColumnInside_CurrentBehavior for context. No functional changes were made.
BDisp
reviewed
Dec 15, 2025
Tests/UnitTestsParallelizable/ViewBase/Draw/ViewDrawingClippingTests.cs
Outdated
Show resolved
Hide resolved
BDisp
approved these changes
Dec 15, 2025
Collaborator
BDisp
left a comment
BDisp
left a comment
There was a problem hiding this comment.
It's useful for testing purposes, but in a real terminal it would be counterproductive.
BDisp
reviewed
Dec 15, 2025
Removed unreachable code that handled the case where the first column of a wide character is outside the clipping region but the second column is inside. This logic was marked as dead code and never called. Now, only the cases where the second column is outside the clip or both columns are in bounds are handled. This simplifies the code and removes unnecessary checks.
…larify its use for clipped wide glyphs. Updates IOutputBuffer to use SetWideGlyphReplacement (single Rune) instead of SetReplacementChars (two Runes). Refactors OutputBufferImpl and all test code to use the new property and method. Removes second-column replacement logic, simplifying the API and improving consistency. Updates comments and test assertions to match the new naming and behavior.
Collaborator
Author
|
Based on that, I've done some renaming and simplification here. |
Renamed "UI Catalog Theme" to "UI Catalog" and removed the "Glyphs.ReplacementChar" property. Added several new properties to the "UI Catalog" theme, including default shadow, highlight states, button alignment, and separator line style. Also added "Glyphs.WideGlyphReplacement" to the "Hot Dog Stand" theme.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes
Glyphs.ReplacementCharto enable config #4497Proposed Changes/Todos
Glyphs.ReplacementChar, which is a static.