Fixes #4730. AnsiDriver will only capture Ctrl+Z if we move the mouse in Windows.#4731
Conversation
… mouse in Windows.
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (18.84%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## v2_develop #4731 +/- ##
==============================================
- Coverage 77.63% 77.51% -0.13%
==============================================
Files 464 466 +2
Lines 45822 45946 +124
Branches 6861 6876 +15
==============================================
+ Hits 35576 35613 +37
- Misses 8237 8319 +82
- Partials 2009 2014 +5
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
To test pressing Ctrl+Z to suspend the app using WSL, we have to open the respective distro first and run the app. Debugging WSL through the IDE doesn't work. |
|
Hey @tig, could you run the unit tests on your Mac? I've run them on an Intel Mac and didn't get any errors, but there might be some incompatibility with the ARM64. It could also just be a conditional failure, but I'd appreciate it if you could confirm that, just to be sure. Thanks. |
|
Should this glyph |
SOme recent PR is somehow setting DrawIndicator in a unit test or something. This is the 2nd time i've seen this. |
|
Fixed in #4777. |
|
That's a good reason to remove System.Console from the other drivers, except for DotNetDriver which is based on it. |
…aw_Cross_Platform(driverName: "dotnet") fails on Unix (gui-cs#4777)
|
I'm getting the same error related to the |
|
I think we should make an effort to move and update all unit tests in the UnitTests project to the UnitTests.Parallelizable project. I suspect there's some static configuration that sporadically uses the spinner feature that isn't explicitly disabled, and some unit test is using it even if don't want it to. |
Do you have any idea how much time I've put into moving tests? Of course we should. I noted above that glyph is the drawindicator. Some recent pr is somehow setting View.DrawIndicator or something else is corrupting it. |
|
Found it. 
|
|
When do you think you'll have time to revise this PR? |
Replaces Logging with Trace.Lifecycle for all lifecycle and error logging in AnsiInput, AnsiOutput, UnixTerminalHelper, and WindowsVTInputHelper, standardizing log output. Adds conditional workaround for Ctrl+Z handling in WindowsVTInputHelper to address ReadFile EOF bug. Fixes a bug in AnsiOutput size query parsing. Cleans up code and updates .DotSettings with new technical terms.
Simplifies input event checking by replacing the Peek() implementation with a direct call to GetNumberOfConsoleInputEvents, removing manual memory management and redundant native API usage. Eliminates the use of FlushConsoleInputBuffer during console mode restoration. Updates comments, region organization, and fixes a logging typo for improved clarity and maintainability.
Always synthesize 0x1A byte on zero-byte ReadFile result, removing conditional compilation and TryHandleCtrlZ method. Ensures the Ctrl+Z bug workaround is always active and simplifies input handling. Updated comments for clarity.
|
I'm working on this now. I'll report back once I have all the facts. |
|
While I'm working on this, I suggest you go look at https://github.com/gui-cs/VTTest as well as
The gist is a) it's possible to work around the ctrlz problem without having to resort to using the WIn32 console APIs, and b) it looks like the WT team now has a fix in development. |
The problemIssue #4730: The ANSI driver on Windows won't capture Ctrl+Z unless the mouse is moved first. This is caused by a longstanding Windows bug where Why the original PR approach was fundamentally wrongI built the ANSI driver specifically to avoid native Win32 console APIs. That's its entire reason for existing. The ANSI driver reads input as a VT byte stream — I said as much in the issue discussion. I closed the issue as "won't fix" and explained that if someone really needs Ctrl+Z on Windows, they can use the Windows or DotNet drivers. I explicitly said: "I do not want the Ansi driver contaminated with dotnet Console code. I worked really hard to make it independent." The PR was submitted anyway. And it did exactly what I said not to do. The author replaced
The author said "my change won't break anything" — but it broke the fundamental contract the ANSI driver is built on. Credit where it's dueThat said, the PR also included genuinely useful work that I appreciate and have kept:
These changes align with the ANSI driver's goals and I'm grateful for them. My objection is solely with the What this PR does nowI rewrote the Ctrl+Z fix to be minimal and architecturally correct, while keeping BDisp's other improvements:
The PR keeps Future Work: Replace remaining P/Invokes with .NET StreamEven with the fix above, I attempted this here. It compiled, and mostly worked, but it broke ANSI request/responses at runtime — specifically the screen size query ( |
Trace.Lifecycle should be used for normal lifecycle events (init, shutdown). Logging.Warning should be used for errors and exceptional conditions. Hot-path tracing in the read loop is commented out. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…to v2_4730_ansidriver-ctrl-z-fix
Just so you know, I was very upset with your attitude, because you didn't even give me the opportunity to explain my reasons and simply closed the issue I created when I was sure my idea would work.
On the contrary, I think that besides not breaking anything, I even added another key combination. At least you acknowledged that I completely eliminated System.Console in AnsiDriver. But please see the explanations below.
The statement that I ignore VT Input is incorrect. Since the
This analysis is completely false. Events are sent to
Another completely false statement. Although it uses libraries that are used in WindowsDriver, the processing is completely different. In WindowsDriver, mouse events are captured in
I completely disagree. While it's not possible to suspend a terminal in Windows, Ctrl+Z is widely used to undo entries in all reputable editors, including Windows Terminal.
I still completely disagree, because I didn't break anything and I added at least one more key combination. |
|
You broke the intent. I'm done arguing with you about this. You continue to discount my main point: the ansi driver's purpose is to not use platform APIs as much as possible. The ctrlz issue is now fixed with 3 lines of code. |
I apologize, I only just realized that your intention is to stop using platform APIs as much as possible, and I thought it was just to stop using System.Console. Therefore, the matter is closed and understood.
I've seen it and it was resolved very well. I also believe it's the only combination that isn't captured, and in that case, I think it's safe to assume it's Ctrl+Z. |
2 participants
Fixes
Proposed Changes/Todos
Pull Request checklist:
CTRL-K-Dto automatically reformat your files before committing.dotnet testbefore commit///style comments)