Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Updating DoSrvPrivateReverseLineFeed with a direct call to ScrollRegion? #2702

Closed
zadjii-msft opened this issue Sep 9, 2019 · 1 comment · Fixed by #2987
Closed

Updating DoSrvPrivateReverseLineFeed with a direct call to ScrollRegion? #2702

zadjii-msft opened this issue Sep 9, 2019 · 1 comment · Fixed by #2987
Assignees
@zadjii-msft
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
20H1

Comments

@zadjii-msft
Copy link
Member

Would it not be worthwhile updating DoSrvPrivateReverseLineFeed at the same time, assuming it's just a matter of replacing the ScrollConsoleScreenBufferWImpl call with a direct call to ScrollRegion?

As for the testing, I've added a whole bunch of scrolling tests in PR #2505 which I think could easily be adapted to use 256 colors once this is merged. Although that PR is going to need to be rebased and updated first.

Originally posted by @j4james in #2668 (comment)

We should also make sure to add some tests for this case as well. I'll do it as a separate PR to make sure we get each one in correctly.
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 9, 2019
@zadjii-msft zadjii-msft self-assigned this Sep 9, 2019
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Sep 9, 2019
@zadjii-msft zadjii-msft added this to the 20H1 milestone Sep 9, 2019
@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase labels Sep 9, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 9, 2019
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Sep 9, 2019
zadjii-msft added a commit that referenced this issue Sep 30, 2019
@zadjii-msft
Fixes #2702 and adds a test
8a2a069
@zadjii-msft zadjii-msft mentioned this issue Sep 30, 2019
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Sep 30, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 1, 2019
@ghost
Copy link

ghost commented Oct 4, 2019

🎉This issue was addressed in #2987, which has now been successfully released as Windows Terminal Preview v0.5.2762.0.:tada:

Handy links:

@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 4, 2019
@j4james j4james mentioned this issue Dec 14, 2019
Closed
@zadjii-msft zadjii-msft mentioned this issue Jan 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
20H1
Development

Successfully merging a pull request may close this issue.

When reverse indexing, preserve RGB/256 attributes microsoft/terminal
2 participants
@DHowett-MSFT @zadjii-msft