Skip to content

Commit

Permalink
[1.14] Use the viewport-rel. cursor pos for CursorPosition (#13786)
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.
Backport of #13785.
  • Loading branch information
DHowett authored Aug 22, 2022
1 parent a277b56 commit b8b412c
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 @@ -1419,7 +1419,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}

auto lock = _terminal->LockForReading();
return til::point{ _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 @@ -1462,3 +1462,12 @@ void Terminal::_updateUrlDetection()
ClearPatternTree();
}
}

// Method Description:
// - Returns the position of the cursor relative to the active viewport
til::point Terminal::GetViewportRelativeCursorPosition() const noexcept
{
const til::point absoluteCursorPosition{ GetCursorPosition() };
const auto viewport{ _GetMutableViewport() };
return absoluteCursorPosition - til::point{ 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 b8b412c

Please sign in to comment.