Repository: microsoft/terminal
File: src/host/screenInfo.cpp (C++)

Code before:
//      that the screen buffer, it will be clamped to the size of the buffer.
// updateBottom: if true, update our virtual bottom. This should be false when
//      called from UX interactions, such as scrolling with the mouse wheel,
//      and true when called from API endpoints, such as SetConsoleWindowInfo
// Return Value:
// - None
void SCREEN_INFORMATION::SetViewport(const Viewport& newViewport,
                                     const bool updateBottom)
{
    // make sure there's something to do
    if (newViewport == _viewport)
    {
        return;
    }

    // do adjustments on a copy that's easily manipulated.
    const til::rect viewportRect{ newViewport.ToExclusive() };
    const til::size coordScreenBufferSize{ GetBufferSize().Dimensions() };

    // MSFT-33471786, GH#13193:
    // newViewport may reside anywhere outside of the valid coordScreenBufferSize.
    // For instance it might be scrolled down more than our text buffer allows to be scrolled.
    const auto cx = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.width(), 1, coordScreenBufferSize.width));
    const auto cy = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.height(), 1, coordScreenBufferSize.height));
    const auto x = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.left, 0, coordScreenBufferSize.width - cx));
    const auto y = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.top, 0, coordScreenBufferSize.height - cy));

    _viewport = Viewport::FromExclusive({x, y, x + cx, y + cy});

    if (updateBottom)
    {
        UpdateBottom();
    }

    Tracing::s_TraceWindowViewport(_viewport);
}

// Method Description:
// - Clear the entire contents of the viewport, except for the cursor's row,
//   which is moved to the top line of the viewport.
// - This is used exclusively by ConPTY to support GH#1193, GH#1882. This allows
//   a terminal to clear the contents of the ConPTY buffer, which is important
//   if the user would like to be able to clear the terminal-side buffer.
// Arguments:
// - <none>
// Return Value:
// - S_OK
[[nodiscard]] HRESULT SCREEN_INFORMATION::ClearBuffer()
{
    const auto oldCursorPos = _textBuffer->GetCursor().GetPosition();
    auto sNewTop = oldCursorPos.Y;

Review comment (suggestion):
This function needs to correct the `newViewport` argument to be within the valid bounds, while retaining the given size as much as possible. If you pass a `newViewport` with the top/bottom coordinates -130 and -100, it should still result in a viewport that's 30 high, but with a top/bottom of 0 and 30.

I feel like this change increases the robustness of the code by splitting the viewport into its size (cx/cy) and origin (x/y). That way we can first ensure that the size doesn't exceed our limits (just like the old code seemingly intended) and _then_ adjust the origin to be within the smaller rectangle of valid coordinates.

The `gsl::narrow_cast<SHORT>`s will be removed once my 32-bit coord PR has been merged, but it shouldn't pose a problem here either, as `GetBufferSize().Dimensions()` is a `COORD` which can't exceed a `SHORT`.

Code after:
//      that the screen buffer, it will be clamped to the size of the buffer.
// updateBottom: if true, update our virtual bottom. This should be false when
//      called from UX interactions, such as scrolling with the mouse wheel,
//      and true when called from API endpoints, such as SetConsoleWindowInfo
// Return Value:
// - None
void SCREEN_INFORMATION::SetViewport(const Viewport& newViewport,
                                     const bool updateBottom)
{
    // make sure there's something to do
    if (newViewport == _viewport)
    {
        return;
    }

    // do adjustments on a copy that's easily manipulated.
    const til::rect viewportRect{ newViewport.ToInclusive() };
    const til::size coordScreenBufferSize{ GetBufferSize().Dimensions() };

    // MSFT-33471786, GH#13193:
    // newViewport may reside anywhere outside of the valid coordScreenBufferSize.
    // For instance it might be scrolled down more than our text buffer allows to be scrolled.
    const auto cx = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.width(), 1, coordScreenBufferSize.width));
    const auto cy = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.height(), 1, coordScreenBufferSize.height));
    const auto x = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.left, 0, coordScreenBufferSize.width - cx));
    const auto y = gsl::narrow_cast<SHORT>(std::clamp(viewportRect.top, 0, coordScreenBufferSize.height - cy));

    _viewport = Viewport::FromExclusive({ x, y, x + cx, y + cy });

    if (updateBottom)
    {
        UpdateBottom();
    }

    Tracing::s_TraceWindowViewport(_viewport);
}

// Method Description:
// - Clear the entire contents of the viewport, except for the cursor's row,
//   which is moved to the top line of the viewport.
// - This is used exclusively by ConPTY to support GH#1193, GH#1882. This allows
//   a terminal to clear the contents of the ConPTY buffer, which is important
//   if the user would like to be able to clear the terminal-side buffer.
// Arguments:
// - <none>
// Return Value:
// - S_OK
[[nodiscard]] HRESULT SCREEN_INFORMATION::ClearBuffer()
{
    const auto oldCursorPos = _textBuffer->GetCursor().GetPosition();
    auto sNewTop = oldCursorPos.Y;