Skip to content

Commit e1086de

Browse files
authored
Fix handling of cursor position reports in passthrough mode (#13109)
## Summary of the Pull Request When the conpty passthrough mode is enabled, it often needs to send `DSR-CPR` queries (cursor position reports) to the client terminal to obtain the current cursor position. However, the code that originally handled the responses to these queries got broken by the refactoring of the `ConGetSet` API. This PR is an attempt to correct that regression. ## References The conpty passthrough mode was introduced in PR #11264. The refactoring that broke the cursor position handling was in PR #12703. ## PR Checklist * [x] Closes #13106 * [x] CLA signed. * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments Prior to the `ConGetSet` refactoring, the code that handled `DSR-CPR` responses (`InteractDispatch::MoveCursor`) would pass the cursor position to `ConGetSet::SetCursorPosition`, which in turn would forward it to the `SetConsoleCursorPositionImpl` API, and from there to the `VtIo` class. After the refactor, all of those intermediate steps were removed - the cursor was simply updated directly in `InteractDispatch::MoveCursor`, and the `VtIo` call was moved from `SetConsoleCursorPositionImpl` to `InteractDispatch` (since that was the only place it was actually required). However, when the conpty passthrough mode was introduced - which happened in parallel - it relied on the `SetConsoleCursorPositionImpl` API being called from `InteractDispatch` in order to handle its own `DSR-CPR` responses, and that's why things stopped working when the two PRs merged. So what I've done now is made `InteractDispatch::MoveCursor` method call `SetConsoleCursorPositionImpl` again (although without the intermediate `ConGetSet` overhead), and moved the `VtIo::SetCursorPosition` call back into `SetConsoleCursorPositionImpl`. This is not ideal, and there are still a bunch of problems with the `DSR-CPR` handling in passthrough mode, but it's at least as good as it was before. ## Validation Steps Performed I've just manually tested various shells with passthrough mode enabled, and confirmed that they're working better now. There are still issues, but nothing that wasn't already a problem in the initial implementation, at least as far as I can tell.
1 parent fa6b066 commit e1086de

File tree

2 files changed

+7
-8
lines changed

2 files changed

+7
-8
lines changed

src/host/getset.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,10 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
719719
position.Y < 0));
720720
// clang-format on
721721

722+
// MSFT: 15813316 - Try to use this SetCursorPosition call to inherit the cursor position.
723+
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
724+
RETURN_IF_FAILED(gci.GetVtIo()->SetCursorPosition(position));
725+
722726
RETURN_IF_NTSTATUS_FAILED(buffer.SetCursorPosition(position, true));
723727

724728
LOG_IF_FAILED(ConsoleImeResizeCompStrView());

src/terminal/adapter/InteractDispatch.cpp

+3-8
Original file line numberDiff line numberDiff line change
@@ -150,15 +150,10 @@ bool InteractDispatch::MoveCursor(const VTInt row, const VTInt col)
150150

151151
const auto coordCursorShort = til::unwrap_coord(coordCursor);
152152

153-
// MSFT: 15813316 - Try to use this MoveCursor call to inherit the cursor position.
154-
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
155-
RETURN_IF_FAILED(gci.GetVtIo()->SetCursorPosition(coordCursorShort));
156-
157153
// Finally, attempt to set the adjusted cursor position back into the console.
158-
auto& cursor = _api.GetTextBuffer().GetCursor();
159-
cursor.SetPosition(coordCursorShort);
160-
cursor.SetHasMoved(true);
161-
return true;
154+
const auto api = gsl::not_null{ ServiceLocator::LocateGlobals().api };
155+
auto& info = ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer();
156+
return SUCCEEDED(api->SetConsoleCursorPositionImpl(info, coordCursorShort));
162157
}
163158

164159
// Routine Description:

0 commit comments

Comments
 (0)