Implement preventing auto-scroll on new output

[carlos-zamora]

Summary of the Pull Request

Updates the Terminal's scroll response to new output. The Terminal will not automatically scroll if...

• a selection is active, or
• the viewport is at the bottom of the scroll history

References

#2529 - Spec
#3863 - Implementation

PR Checklist

• Closes Feature Request: Pause output or scrolling on click (and make it a setting) #980
• Closes Feature Request: Preserve scroll position while not "at bottom" #3863
• Tests added/passed
• Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

Updates the _scrollOffset value properly in TerminalCore when the cursor moves. We calculate a new _scrollOffset based on if we are circling the buffer and how far below the mutable bottom is.

We specifically check for if a selection is active and if the viewport is at the bottom, then use that as a condition for deciding if we should update _scrollOffset to the new calculated value or 0 (the bottom of the scroll history).

Validation Steps Performed

Manual testing. Though I should add automated tests.

• new output
• new output when circling
• new output when circling and viewport is at the top

[johnburnett]

This is probably coming in way to late, but having just found this - can we call this "scrollOnOutput"? The only reason I found this PR is because it's linked through other issues that reference "scroll" in their name. "snap" feels very obscure compared to what this setting actually controls (the scroll position).

[carlos-zamora]

This is probably coming in way to late, but having just found this - can we call this "scrollOnOutput"? The only reason I found this PR is because it's linked through other issues that reference "scroll" in their name. "snap" feels very obscure compared to what this setting actually controls (the scroll position).

Fair request. Copying this comment over to #2529 for discussion there.

EDIT: wait, it's already there hahaha

[DHowett]

clever

[zadjii-msft]

Add a test?

[carlos-zamora]

Add a test?

I think we don't have scrolling-related tests, but I'll double check. So nobody come along and automerge this plz!

[zadjii-msft]

block for a test

[carlos-zamora]

block for a test

I hate that you're in the right for doing this... hahaha

[carlos-zamora]

Ok. I spent most of today trying to add a test to ScrollTest. I copied it below.

That should totally work. All I'm doing is writing lines of text to the buffer, and comparing the scroll offset to what it should be.

It doesn't though because we need a callback to the scrollback in TermControl. Spoke with Dustin and this just doesn't feel right. Like, scrolling should be pretty self sustained.

So, I created #6842 to track this better.

Test Code

void ScrollTest::TestConditionalAutoscrolling()
{
    // When new output is generated, we should only scroll to it when...
    // - no selection is active, and
    // - the viewport is already at the bottom of the scroll history

    Log::Comment(L"Watch out - this test takes a while to run, and won't "
                 L"output anything unless in encounters an error. This is expected.");

    auto& termTb = *_term->_buffer;

    const auto totalBufferSize = termTb.GetSize().Height();

    // We're outputting like 18000 lines of text here, so emitting 18000*4 lines
    // of output to the console is actually quite unnecessary
    WEX::TestExecution::SetVerifyOutput settings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);

    // Emit a bunch of newlines to test scrolling.
    auto scrollAmount = 0;
    for (auto currentRow = 0; currentRow < totalBufferSize * 2; currentRow++)
    {
        *_scrollBarNotification = std::nullopt;
        _renderTarget->Reset();

        _term->_WriteBuffer(L"X\r\n");

        if (currentRow < TerminalViewHeight)
        {
            // no scrolling is expected
            VERIFY_ARE_EQUAL(0, _term->_scrollOffset);
        }
        else if (currentRow < TerminalViewHeight * 2)
        {
            // keep track of scrolling
            ++scrollAmount;
            VERIFY_ARE_EQUAL(scrollAmount, _term->_scrollOffset);
        }
        else if (currentRow == TerminalViewHeight * 2)
        {
            // create a selection
            Log::Comment(L"Create a selection to pause scrolling");
            _term->SelectNewRegion({ 0, static_cast<SHORT>(currentRow) }, { 5, static_cast<SHORT>(currentRow) });

            VERIFY_ARE_EQUAL(scrollAmount, _term->_scrollOffset);
        }
        else if (currentRow < totalBufferSize)
        {
            // scrolling is unchanged because of selection
            VERIFY_ARE_EQUAL(scrollAmount, _term->_scrollOffset);
        }
        else if (scrollAmount > 0)
        {
            // circling the buffer
            // we should be scrolling up to keep the same content in the viewport
            --scrollAmount;
            VERIFY_ARE_EQUAL(scrollAmount, _term->_scrollOffset);
        }
        else
        {
            // still circling the buffer
            // we can't scroll up anymore
            VERIFY_ARE_EQUAL(0, _term->_scrollOffset);
        }
    }
}

[zadjii-msft]

good enough for me

[ghost]

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]

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

Handy links:

• Release Notes
• Store Download

[brandon-kohn]

I'm using 1.17.1023 and when I'm compiling and scroll back to see an error after a few moments it jumps back down to the bottom of buffer. This happens whether I select some text or not.

[zadjii-msft]

@brandon-kohn Interesting - mind filing a new issue? This should have shipped nearly 3 years ago now, so whatever you're seeing is probably unrelated to this original PR. I want to make sure we don't lose track of it.

I'd also check the snapOnInput setting (some mysterious keys that aren't letters sometimes still trigger the snapping). More details about your environment might be helpful too, what tools, WSL or not, tmux or not, etc, etc.

Thanks!