[PR #4372] [MERGED] A pair of fixes related to cursor movement in conpty #25723

Open
opened 2026-01-31 09:11:23 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/4372
Author: @zadjii-msft
Created: 1/27/2020
Status: ✅ Merged
Merged: 1/30/2020
Merged by: @undefined

Base: master ← Head: dev/migrie/b/4102-and-2642


📝 Commits (3)

📊 Changes

3 files changed (+75 additions, -3 deletions)

View changed files

📝 src/host/getset.cpp (+8 -1)
📝 src/host/output.cpp (+14 -2)
📝 src/host/ut_host/ScreenBufferTests.cpp (+53 -0)

📄 Description

Summary of the Pull Request

This is a pair of related fixes to conpty. For both of these bugs, the root cause was that the cursor was getting set to Off in conpty. Without the CursorBlinkerTimer, the cursor would remain off, and frames that only had cursor movements would not update the cursor position in the terminal.

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Recall that there's a bunch of cursor state that's hard to parse without looking up:

  • Visibility This controls whether the cursor is visible at all, regardless if it's been blinked on or off
  • Blinking controls whether the blinker timer should do something, or leave the cursor alone.
  • IsOn: When the cursor is blinking, this alternates between true and false.

The trick here is that we only TriggerCursorMoved when the cursor is On, and there are some scenarios where the cursor is manually set to off.

Fundamentally, these two bugs are similar cases, but they are triggered by different things:

  • #2642 was caused by DoSrvPrivateAllowCursorBlinking(false) (^[[?12l) also manually turning the cursor off.
  • #4102 was caused by the client calling SetConsoleScreenBuffer to change the active buffer. win-curses actually uses that API instead of the alt buffer.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/4372 **Author:** [@zadjii-msft](https://github.com/zadjii-msft) **Created:** 1/27/2020 **Status:** ✅ Merged **Merged:** 1/30/2020 **Merged by:** [@undefined](undefined) **Base:** `master` ← **Head:** `dev/migrie/b/4102-and-2642` --- ### 📝 Commits (3) - [`5e3e59e`](https://github.com/microsoft/terminal/commit/5e3e59ed6fac7017536ecd5b7e6419f8cfbc452f) This fixes #2642 - [`ce6e404`](https://github.com/microsoft/terminal/commit/ce6e404904e20a4e2d73b1d8d21b827ffa7d84f0) This fixes #4102 - [`22d017c`](https://github.com/microsoft/terminal/commit/22d017c2f5050857800cfee119a811eddf152bec) add a test ### 📊 Changes **3 files changed** (+75 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/host/getset.cpp` (+8 -1) 📝 `src/host/output.cpp` (+14 -2) 📝 `src/host/ut_host/ScreenBufferTests.cpp` (+53 -0) </details> ### 📄 Description ## Summary of the Pull Request This is a pair of related fixes to conpty. For both of these bugs, the root cause was that the cursor was getting set to Off in conpty. Without the `CursorBlinkerTimer`, the cursor would remain off, and frames that only had cursor movements would not update the cursor position in the terminal. ## References ## PR Checklist * [x] Closes #4102 * [x] Closes #2642 * [x] Closes #2697 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments Recall that there's a bunch of cursor state that's hard to parse without looking up: * `Visibility` This controls whether the cursor is visible _at all_, regardless if it's been blinked on or off * `Blinking` controls whether the blinker timer should do something, or leave the cursor alone. * `IsOn`: When the cursor is blinking, this alternates between true and false. The trick here is that we only `TriggerCursorMoved` when the cursor is `On`, and there are some scenarios where the cursor is manually set to off. Fundamentally, these two bugs are similar cases, but they are triggered by different things: * #2642 was caused by `DoSrvPrivateAllowCursorBlinking(false)` (`^[[?12l`) also manually turning the cursor off. * #4102 was caused by the client calling `SetConsoleScreenBuffer` to change the active buffer. `win-curses` actually uses that API instead of the alt buffer. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:11:23 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#25723