Skip to content

Commit

Permalink
Use the viewport-relative cursor pos for CCore.CursorPosition (#13785)
Browse files Browse the repository at this point in the history
In #13024, we removed `Terminal::GetCursorPosition` from TerminalCore.
This has been widely regarded as a good move.

Now, you might rightly be wondering: why didn't compilation immediately
fail? Well. It turns out that there were _two_ copies of
`GetCursorPosition`. One for `const Terminal`, and one for `Terminal`.
This is important.

`Terminal::GetCursorPosition()` returned the cursor position relative to
the viewport. `Terminal::GetCursorPosition() const`, however, returns
the cursor position in absolute.

We removed the non-`const` one. Fortunately, thanks to the lookup rules
for `const`-qualified members, this didn't matter. Code that called
`GetCursorPosition()` still called `GetCursorPosition()`, and everything
was fine.

Except that part about the relative coordinates. That was not fine.

The TSF control is the _only_ consumer of `ControlCore.CursorPosition`,
and that was the _only_ consumer of relative-`GetCursorPosition()`.

This commit restores equilibrium by introducing a new
`GetViewportRelativeCursorPosition()` member to `Terminal` and switching
over the only consumer of relative cursor position to use it.

Closes #13769.
  • Loading branch information
DHowett authored Aug 22, 2022
1 parent c12987a commit 2dedc9a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1602,7 +1602,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

auto lock = _terminal->LockForReading();
return _terminal->GetCursorPosition().to_core_point();
return _terminal->GetViewportRelativeCursorPosition().to_core_point();
}

// This one's really pushing the boundary of what counts as "encapsulation".
Expand Down
9 changes: 9 additions & 0 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1638,3 +1638,12 @@ til::color Terminal::GetColorForMark(const Microsoft::Console::VirtualTerminal::
}
}
}

// Method Description:
// - Returns the position of the cursor relative to the active viewport
til::point Terminal::GetViewportRelativeCursorPosition() const noexcept
{
const auto absoluteCursorPosition{ GetCursorPosition() };
const auto viewport{ _GetMutableViewport() };
return absoluteCursorPosition - viewport.Origin();
}
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class Microsoft::Terminal::Core::Terminal final :
bool IsXtermBracketedPasteModeEnabled() const;
std::wstring_view GetWorkingDirectory();

til::point GetViewportRelativeCursorPosition() const noexcept;

// Write comes from the PTY and goes to our parser to be stored in the output buffer
void Write(std::wstring_view stringView);

Expand Down

0 comments on commit 2dedc9a

Please sign in to comment.