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.

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

Stop the viewport being moved when in the alt buffer #13039

Merged
1 commit merged into from
May 5, 2022

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented May 5, 2022

Summary of the Pull Request

When TerminalDispatch was merged with AdaptDispatch in PR #13024,
that broke the Terminal's EraseAll operation in the alt buffer. The
problem was that the EraseAll implementation makes a call to
SetViewportPosition which wasn't taking the alt buffer into account,
and thus modified the main viewport instead.

This PR corrects that mistake. If we're in the alt buffer, the
SetViewportPosition method now does nothing, since the alt buffer
viewport should always be at 0,0.

References

This was a regression introduced in PR #13024.

PR Checklist

Validation Steps Performed

I've confirmed that the test case reported in issue #13038 is no longer
failing. I've also made sure the ED 2 and ED 3 sequences are still
working correctly in the main buffer.

@ghost ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels May 5, 2022
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix!

@j4james
Copy link
Collaborator Author

j4james commented May 5, 2022

I'm sorry for breaking it in the first place!

@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 5, 2022
@ghost
Copy link

ghost commented May 5, 2022

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 4b0ebbc into microsoft:main May 5, 2022
@j4james j4james deleted the fix-setviewportposition branch May 14, 2022 12:46
@ghost
Copy link

ghost commented May 24, 2022

🎉Windows Terminal Preview v1.14.143 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resizing while alt buffer is active, then disabling it, blows away renderer
3 participants