Skip to content

Conversation

@BDisp
Copy link
Collaborator

@BDisp BDisp commented Nov 12, 2025

Fixes

Proposed Changes/Todos

  • Use StringInfo with GetTextElementEnumerator to enumerate all graphemes that can be rendered as a single cell which may occupy 1 or 2 columns in the screen.

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@BDisp BDisp requested a review from tig as a code owner November 12, 2025 18:52
@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 84.31373% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.46%. Comparing base (0995148) to head (b6072bb).

Files with missing lines Patch % Lines
Terminal.Gui/Text/TextFormatter.cs 80.79% 12 Missing and 17 partials ⚠️
Terminal.Gui/Views/CharMap/CharMap.cs 43.47% 12 Missing and 1 partial ⚠️
Terminal.Gui/Drivers/OutputBufferImpl.cs 76.19% 5 Missing and 5 partials ⚠️
Terminal.Gui/Views/Slider/Slider.cs 60.00% 8 Missing and 2 partials ⚠️
Terminal.Gui/Views/TextInput/TextModel.cs 90.38% 0 Missing and 5 partials ⚠️
Terminal.Gui/Drawing/Cell.cs 95.00% 1 Missing and 2 partials ⚠️
Terminal.Gui/Views/TreeView/Branch.cs 83.33% 1 Missing and 2 partials ⚠️
.../Views/Autocomplete/AutocompleteFilepathContext.cs 81.81% 1 Missing and 1 partial ⚠️
Terminal.Gui/Views/TextInput/TextView.cs 91.66% 0 Missing and 2 partials ⚠️
Terminal.Gui/ViewBase/View.Drawing.Primitives.cs 83.33% 0 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files
@@              Coverage Diff               @@
##           v2_develop    #4388      +/-   ##
==============================================
- Coverage       74.56%   74.46%   -0.11%     
==============================================
  Files             388      389       +1     
  Lines           46568    46584      +16     
  Branches         6548     6552       +4     
==============================================
- Hits            34723    34687      -36     
- Misses           9993    10035      +42     
- Partials         1852     1862      +10     
Files with missing lines Coverage Δ
Terminal.Gui/App/Application.cs 89.09% <100.00%> (-1.08%) ⬇️
Terminal.Gui/Drawing/GraphemeHelper.cs 100.00% <100.00%> (ø)
Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs 93.82% <100.00%> (-0.02%) ⬇️
Terminal.Gui/Drivers/DriverImpl.cs 62.83% <100.00%> (ø)
Terminal.Gui/Drivers/OutputBase.cs 81.17% <100.00%> (-2.16%) ⬇️
Terminal.Gui/Text/StringExtensions.cs 88.88% <100.00%> (+3.78%) ⬆️
Terminal.Gui/ViewBase/Adornment/ShadowView.cs 92.85% <100.00%> (ø)
Terminal.Gui/ViewBase/View.Drawing.cs 87.59% <100.00%> (+0.24%) ⬆️
...rminal.Gui/Views/Autocomplete/PopupAutocomplete.cs 61.77% <100.00%> (ø)
Terminal.Gui/Views/TableView/TreeTableSource.cs 79.81% <100.00%> (ø)
... and 12 more

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0995148...b6072bb. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 12, 2025

Please note that this feature is not compatible with legacy consoles, such as cmd.exe and conhost.exe, due to known limitations, such as true colors.

image

Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

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

Holy moly! this is great stuff @BDisp.

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 12, 2025

@tig I did an update branch from here and it broken my local branch when I pulled. Do you know what's happened?

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 12, 2025

@tig I did an update branch from here and it broken my local branch when I pulled. Do you know what's happened?

I managed to solve it, but I don't understand how it happened.

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 12, 2025

@tig the below message appear sometimes when any test fail in racing conditions. Do you know it exist any static configuration that can be changed while another unit test is running?

Message: 
Assert.Equal() Failure: Values differ
Expected: Border(){X=0,Y=0,Width=20,Height=10}
Actual:   null

@tig
Copy link
Collaborator

tig commented Nov 13, 2025

@tig the below message appear sometimes when any test fail in racing conditions. Do you know it exist any static configuration that can be changed while another unit test is running?


Message: 

Assert.Equal() Failure: Values differ

Expected: Border(){X=0,Y=0,Width=20,Height=10}

Actual:   null

Parallelizable, right? Might have something to do with GlobalTestSetup.

@BDisp BDisp marked this pull request as draft November 14, 2025 11:42
@BDisp
Copy link
Collaborator Author

BDisp commented Nov 14, 2025

Damn, testhost.exe take 80% CPU resources. I'll only running all tests in the Terminal.Gui CI actions.

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 14, 2025

The CI should show all errors at once.

@BDisp BDisp marked this pull request as ready for review November 14, 2025 21:26
@BDisp
Copy link
Collaborator Author

BDisp commented Nov 14, 2025

Parallelizable, right? Might have something to do with GlobalTestSetup.

I think I fix this issue in b6072bb.

@BDisp
Copy link
Collaborator Author

BDisp commented Nov 14, 2025

After upgrading Wcwidth to version 4.0.0, I had to disable a few unit tests until I see if there's a solution or better explanation, as commented in #4259 (comment). But there are more advantages to using this version than disadvantages because there are many corrections regarding code points that were previously considered narrow and are now correctly considered zero-length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Runes should not be used on a cell, but rather should use a single grapheme rendering 1 or 2 columns

2 participants