[1.14] Use the viewport-rel. cursor pos for CursorPosition#13786
Merged
DHowett merged 1 commit intorelease-1.14from Aug 22, 2022
Merged
[1.14] Use the viewport-rel. cursor pos for CursorPosition#13786DHowett merged 1 commit intorelease-1.14from
DHowett merged 1 commit intorelease-1.14from
Conversation
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.
ghost
added
Area-Input
carlos-zamora
approved these changes
Aug 19, 2022
zadjii-msft
approved these changes
Aug 22, 2022
DHowett
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #13024, we removed
Terminal::GetCursorPositionfrom 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 forconst Terminal, and one forTerminal.This is important.
Terminal::GetCursorPosition()returned the cursor position relative tothe viewport.
Terminal::GetCursorPosition() const, however, returnsthe cursor position in absolute.
We removed the non-
constone. Fortunately, thanks to the lookup rulesfor
const-qualified members, this didn't matter. Code that calledGetCursorPosition()still calledGetCursorPosition(), and everythingwas 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 toTerminaland switchingover the only consumer of relative cursor position to use it.
Closes #13769.
Backport of #13785.