New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[1.21] Selecting commands, output with the context menu is totally in the wrong place #17131
Labels
In-PR
This issue has a related PR
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
Milestone
Comments
microsoft-github-policy-service
bot
added
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
Needs-Tag-Fix
Doesn't match tag requirements
labels
Apr 25, 2024
or prior: Just staring at that in isolation, it's not clear to me what coordinate system the coord params of SelectNewRegion(const til::point coordStart, const til::point coordEnd) are |
diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp
index ec28b59d5..3d989f31f 100644
--- a/src/cascadia/TerminalCore/terminalrenderdata.cpp
+++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp
@@ -199,7 +199,7 @@ til::CoordType Terminal::_ScrollToPoints(const til::point coordStart, const til:
_NotifyScrollEvent();
}
- return _scrollOffset;
+ return _VisibleStartIndex();
}
void Terminal::SelectNewRegion(const til::point coordStart, const til::point coordEnd) this fixes it. Need to write a test |
carlos-zamora
added
the
Issue-Bug
It either shouldn't be doing this or needs an investigation.
label
Apr 25, 2024
carlos-zamora
removed
the
Needs-Triage
It's a new issue that the core contributor team needs to triage at the next triage meeting
label
Apr 25, 2024
zadjii-msft
added a commit
that referenced
this issue
Apr 26, 2024
I think this subtly regressed in `Terminal::SelectNewRegion` is the only thing that uses the return value from `Terminal::_ScrollToPoints`. Before that PR, `_ScrollToPoints` was just a part of `SelectNewRegion`, and it moved the start & end coords by the `_VisibleStartIndex`, not the `_scrollOffset`. Kinda weird there weren't any _other_ tests for `SelectNewRegion`? I also caught a second bug while I was here - If you had a line with an exact wrap, and tried to select that like with selectOutput, we'd explode. Closes #17131
github-merge-queue bot
pushed a commit
that referenced
this issue
May 1, 2024
I think this subtly regressed in #16611. Jump to 90b8bb7#diff-f9112caf8cb75e7a48a7b84987724d754181227385fbfcc2cc09a879b1f97c12L171-L223 `Terminal::SelectNewRegion` is the only thing that uses the return value from `Terminal::_ScrollToPoints`. Before that PR, `_ScrollToPoints` was just a part of `SelectNewRegion`, and it moved the start & end coords by the `_VisibleStartIndex`, not the `_scrollOffset`. Kinda weird there weren't any _other_ tests for `SelectNewRegion`? I also caught a second bug while I was here - If you had a line with an exact wrap, and tried to select that like with selectOutput, we'd explode. Closes #17131
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
In-PR
This issue has a related PR
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
open pwsh, with shell integration enabled
do a
try and right-click select the output of
dir
, and it's... wrong:surely, I'm the murderer, but I could have swore there were tests for this
The text was updated successfully, but these errors were encountered: